Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improvements for Web Components #1444

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@ handling
handle: aRequestContext
"Resume processing of a request. To ensure valid application state restore all registered states."

self states restore.
self restoreState.
self withUnregisteredHandlerDo: [ super handle: aRequestContext ]
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
processing
restoreState

self states restore
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
private
createCache
^ WASingleElementCache new
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
testing
testClear
cache at: 1 put: 'one'.
cache clear.
self assert: (cache at: 1 ifAbsent: [ 'two' ]) equals: 'two'.
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
testing
testKeyAtValue
cache at: 1 put: 'one'.
self assert: (cache keyAtValue: 'one' ifAbsent: [ 2 ]) equals: 1.
self assert: (cache keyAtValue: 'two' ifAbsent: [ 2 ]) equals: 2.
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
testing
testKeysAndValuesDo
| reference readBack |
reference := Dictionary new.

cache at: 1 put: 'one'.
reference at: 1 put: 'one'.

readBack := Dictionary new.
cache keysAndValuesDo: [ :key :value |
readBack at: key put: value ].

self assert: readBack equals: reference
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
testing
testSize
self assert: cache size equals: 0.
cache at: 1 put: 'one'.
self assert: cache size equals: 1.
cache at: 2 put: 'two'.
self assert: cache size equals: 1.
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
testing
testStore
| generator |
generator := WAPrecomputedKeyGenerator keys: #(1 2 3).
WAKeyGenerator
use: generator
during: [
self assert: (cache store: 'key1') equals: 1.
self assert: (cache store: 'key2') equals: 2.
self assert: (cache store: 'key3') equals: 3 ]
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"commentStamp" : "",
"super" : "WACacheTest",
"category" : "Seaside-Tests-WebComponents",
"classinstvars" : [ ],
"pools" : [ ],
"classvars" : [ ],
"instvars" : [ ],
"name" : "WASingleElementCacheTest",
"type" : "normal"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
I am a session that stores only a single continuation. I am usesful for cases where state snapshots are not used.
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
initialization
createContinuationCache
^ WASingleElementCache new

Check warning on line 3 in repository/Seaside-WebComponents-Core.package/WASingleContinuationSession.class/instance/createContinuationCache.st

View check run for this annotation

Codecov / codecov/patch

repository/Seaside-WebComponents-Core.package/WASingleContinuationSession.class/instance/createContinuationCache.st#L1-L3

Added lines #L1 - L3 were not covered by tests
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"commentStamp" : "xxx 7/11/2024 13:25",
"super" : "WASession",
"category" : "Seaside-WebComponents-Core",
"classinstvars" : [ ],
"pools" : [ ],
"classvars" : [ ],
"instvars" : [ ],
"name" : "WASingleContinuationSession",
"type" : "normal"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
I am a cache that contains at most one element.
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
accessing
at: aKey ifAbsent: aBlock
^ key = aKey
ifTrue: [ value ]
ifFalse: [ aBlock value ]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
ifFalse: [ aBlock value ]
ifFalse: aBlock

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I do this I get a code critic warning in Pharo

ifTrue: ifFalse: ifTrue:ifFalse: ifFalse:ifTrue: is used in a way that it can not be optimized.

Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
putting
at: aKey put: anObject
key := aKey.
value := anObject.
^ anObject
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
public
clear
key := nil.
value := nil
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
accessing
keyAtValue: anObject ifAbsent: aBlock
^ value = anObject
ifTrue: [ key ]
ifFalse: [ aBlock value ]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
ifFalse: [ aBlock value ]
ifFalse: aBlock

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some Pharo code critic warning if I do this.

Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
enumerating
keysAndValuesDo: aTwoArgumentBlock
key isNil ifFalse: [
aTwoArgumentBlock value: key value: value ]
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
removing
remove: anObject
value = anObject ifTrue: [
key := nil.
value := nil ]

Check warning on line 5 in repository/Seaside-WebComponents-Core.package/WASingleElementCache.class/instance/remove..st

View check run for this annotation

Codecov / codecov/patch

repository/Seaside-WebComponents-Core.package/WASingleElementCache.class/instance/remove..st#L1-L5

Added lines #L1 - L5 were not covered by tests
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
accessing
size
^ key isNil
ifTrue: [ 0 ]
ifFalse: [ 1 ]
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
putting
store: anObject
key := WAKeyGenerator current keyOfLength: self keySize.
value := anObject.
^ key
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"commentStamp" : "xxx 7/11/2024 10:46",
"super" : "WACache",
"category" : "Seaside-WebComponents-Core",
"classinstvars" : [ ],
"pools" : [ ],
"classvars" : [ ],
"instvars" : [
"key",
"value"
],
"name" : "WASingleElementCache",
"type" : "normal"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
I am a special action continuation with optimizations for web components.

Since web components have no back button and a full page reload results in an entire new page I perform the following optimizations.

- I do not redirect.
- I do not capture the state.
- I do not restore the state.
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
processing
basicPerformAction
super basicPerformAction.
self renderContext callbacks handle: self requestContext

Check warning on line 4 in repository/Seaside-WebComponents-Core.package/WAWebComponentActionContinuation.class/instance/basicPerformAction.st

View check run for this annotation

Codecov / codecov/patch

repository/Seaside-WebComponents-Core.package/WAWebComponentActionContinuation.class/instance/basicPerformAction.st#L1-L4

Added lines #L1 - L4 were not covered by tests
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
processing
continue
"do not capture state"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Side-note: since we do not capture state, do we need the contents of the states instvar in this class?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see what you meant. The question comes down to how much we want to tie WAFragmentRenderPhaseContinuation to WAFragmentRenderPhaseContinuation. Right now the states inst vars are still filled with WASnapshot instances due to two reasons:

  • WAFragmentRenderPhaseContinuation inheritsWARenderLoopContinuation >> #createActionContinuationWithContext: and send #snapshot:renderContext: to the action phase continuation class
  • WAWebComponentActionContinuation >> #handle: still sends ends up in WASessionContinuation >> #handle:

self createRenderContinuation handle: self requestContext

Check warning on line 4 in repository/Seaside-WebComponents-Core.package/WAWebComponentActionContinuation.class/instance/continue.st

View check run for this annotation

Codecov / codecov/patch

repository/Seaside-WebComponents-Core.package/WAWebComponentActionContinuation.class/instance/continue.st#L1-L4

Added lines #L1 - L4 were not covered by tests
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
public
jumpToAnchor: aString
"Intentionally empty for compatibility with WACallbackProcessingActionContinuation"

Check warning on line 3 in repository/Seaside-WebComponents-Core.package/WAWebComponentActionContinuation.class/instance/jumpToAnchor..st

View check run for this annotation

Codecov / codecov/patch

repository/Seaside-WebComponents-Core.package/WAWebComponentActionContinuation.class/instance/jumpToAnchor..st#L1-L3

Added lines #L1 - L3 were not covered by tests
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
processing
restoreState
"don't restore states"

Check warning on line 3 in repository/Seaside-WebComponents-Core.package/WAWebComponentActionContinuation.class/instance/restoreState.st

View check run for this annotation

Codecov / codecov/patch

repository/Seaside-WebComponents-Core.package/WAWebComponentActionContinuation.class/instance/restoreState.st#L1-L3

Added lines #L1 - L3 were not covered by tests
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
private
shouldRedirect
^ false

Check warning on line 3 in repository/Seaside-WebComponents-Core.package/WAWebComponentActionContinuation.class/instance/shouldRedirect.st

View check run for this annotation

Codecov / codecov/patch

repository/Seaside-WebComponents-Core.package/WAWebComponentActionContinuation.class/instance/shouldRedirect.st#L1-L3

Added lines #L1 - L3 were not covered by tests
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"commentStamp" : "xxx 7/11/2024 13:24",
"super" : "WAActionPhaseContinuation",
"category" : "Seaside-WebComponents-Core",
"classinstvars" : [ ],
"pools" : [ ],
"classvars" : [ ],
"instvars" : [ ],
"name" : "WAWebComponentActionContinuation",
"type" : "normal"
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
#load(method, url, data) {
const xhr = new XMLHttpRequest();

xhr.responseType = "text"; // we do not stirp anyhing, just use innerHtml
xhr.responseType = "text"; // we do not strip anyhing, just use innerHtml

Check warning on line 40 in repository/Seaside-WebComponents-Core.package/WAWebComponentsLibrary.class/instance/seasideWebComponentsJs.st

View check run for this annotation

Codecov / codecov/patch

repository/Seaside-WebComponents-Core.package/WAWebComponentsLibrary.class/instance/seasideWebComponentsJs.st#L40

Added line #L40 was not covered by tests
xhr.addEventListener("load", (event) => {
if (xhr.status === 200) {
this.#shadowRoot.innerHTML = xhr.response;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,8 @@ initialize
"register such that we do not have the developer tools"
application := WAAdmin register: WAApplication at: 'examples/headless-counter' in: WAAdmin defaultDispatcher.
application configuration addParent: WARenderLoopConfiguration instance.
application rootClass: self.
application preferenceAt: #renderPhaseContinuationClass put: WAFragmentRenderPhaseContinuation
application
rootClass: self;
sessionClass: WASingleContinuationSession;
preferenceAt: #renderPhaseContinuationClass put: WAFragmentRenderPhaseContinuation;
preferenceAt: #actionPhaseContinuationClass put: WAWebComponentActionContinuation
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
initialization
initialize
(WAAdmin register: self asApplicationAt: 'examples/web-components')
| application |
"register such that we do not have the developer tools"
application := WAAdmin register: WAApplication at: 'examples/web-components' in: WAAdmin defaultDispatcher.
application configuration addParent: WARenderLoopConfiguration instance.
application
rootClass: self;
scriptGeneratorClass: WANullScriptGenerator
Loading