feat: add Element.addJsInitializer for scoped client-side JS lifecycle#24366
Merged
Conversation
Artur-
force-pushed
the
feature/element-js-initializer
branch
4 times, most recently
from
85d4713 to
1a87829
Compare
Application code that wires up client-side listeners on behalf of a server component had two failure modes: re-attach after a real detach produces a fresh DOM node and silently strips installed listeners, and cleanup from a detach listener cannot be sent because the element is leaving the tree. addJsInitializer registers a JavaScript expression that runs each time the element acquires a fresh client-side DOM, and whose returned cleanup function is invoked when that DOM is discarded or the returned Registration is removed. The init is gated on the existing StateNode.isClientSideInitialized flag, so a same-round-trip detach+reattach (where the client never discards its DOM) does not double-install; a cross-round-trip detach+reattach does. The wrapper JS hosts a per-StateNode cleanup map under window.Vaadin.Flow.initializers and drains it via the client-side StateNode unregister listener, keeping the mechanism scoped per element with no new wire protocol. Fixes #23900
Review feedback on Element.addJsInitializer:
- The wrapper no longer awaits the user expression, so returning a
promise that resolves to a function is not supported anymore. The
registry only accepts a function returned synchronously; any other
return value (including a promise) is treated as "no cleanup".
- Javadoc updated to state this explicitly.
- Added unit tests for additional lifecycle scenarios:
- attach + detach + attach within one round trip emits exactly one
init invocation,
- registering while detached defers the init until the element is
attached,
- multiple initializers on the same element get distinct ids,
- removing one of several registrations disposes only that one.
The wrapper code generated by Element.addJsInitializer was wrapped in an
extra IIFE (`return (function() { ... })()`) at the outermost level.
Inside that IIFE, `this` was the global object instead of the Flow
executor that the user expression actually needs, so the first call to
`this.getNode(__element)` threw synchronously and the user expression
never ran. The unit tests only inspected the generated string, so the
issue only surfaced in the browser ITs.
Drop the outer IIFE; the Flow-generated `new Function(...)` body already
provides function scope and keeps `this` bound to the executor context.
The inner IIFE around the user expression is kept so the user's
`return` yields the cleanup function instead of escaping the outer
wrapper.
Also add `addJsInitializer` to the ignore list in
`ElementJacksonTest.publicElementMethodsShouldReturnElement` (it returns
a Registration, mirroring the same ignore already in ElementTest).
Artur-
force-pushed
the
feature/element-js-initializer
branch
from
1a87829 to
f250600
Compare
Legioth
requested changes
May 19, 2026
Addresses comments from @Legioth on PR 24366: - Element.addJsInitializer javadoc: replace "acquires a client-side DOM node" with the clearer "a client-side DOM node is created for this element"; rephrase the return-value contract so a non-function return is documented as an error. - ElementJsInitializerRegistration: drop the inline registry IIFE from the wrapper. The cleanup map now lives in the client bundle (ExecuteJavaScriptElementUtils) and is exposed to the executor context as registerInitializer / disposeInitializer. The wrapper expression is reduced to one straight call into those helpers and logs a console error when the user expression returns a non-function, non-undefined value. - Wrapper params have named bindings at the top of the snippet (`const element = $0; const initializer = $1; ...`) instead of being documented in javadoc. Init and dispose share the same parameter order with element first. - Drop the redundant `runWhenAttached` inside `onAttach` — the node is known to be attached when this runs. - Initializer ids now come from a UI-wide counter on UIInternals (`nextJsInitializerId`), allocated lazily on first emit. The per-node JsInitializerCounter is kept in place by this commit and removed in the next one. - Added a unit test verifying that calling `remove()` twice only sends a single dispose invocation. - Renamed `clickAndWait` to `click` in the IT (the waiting actually happens in `assertCounters`).
addJsInitializer now allocates ids from UIInternals.nextJsInitializerId (see the preceding commit), so the per-node counter is dead code. Remove the class, its NodeFeatures id, and its registry/state-provider/test entries.
Legioth
reviewed
May 20, 2026
| if (existing != null) { | ||
| invokeSafely(existing); | ||
| } | ||
| entry.set(id, cleanup); |
Member
There was a problem hiding this comment.
Set before invoking to avoid corruption in the exceptional case that the callback explicitly disposes or registers
| private static final String INIT_EXPRESSION = """ | ||
| const element = $0; | ||
| const initializer = $1; | ||
| const initializerId = $2; |
Member
There was a problem hiding this comment.
nit: Could further improve consistency with the dispose expression by always having the initializer id as $1 and thus send the initializer expression as $2
github-project-automation
Bot
moved this to 🔎Iteration reviews
in Vaadin Flow | Hilla | Kits ongoing work
- ExecuteJavaScriptElementUtils.registerInitializer: install the new cleanup in the map before invoking the previous one, so a re-entrant register/dispose call from inside the existing callback sees the new state instead of the stale entry. - UIInternals.nextJsInitializerId: call session.checkHasLock() to match the locking convention used by other methods that mutate UI state. - ElementJsInitializerRegistration: reorder init parameters to [element, initializerId, userFunction] so the initializer id is at $1 in both the init and dispose expressions. - Element.addJsInitializer / wrapper: accept null in addition to undefined as an explicit "no cleanup" signal; only non-null, non-undefined, non-function return values are logged as errors. - Test: parameter-order assertions updated for the new layout.
Artur-
added a commit
to vaadin/docs
that referenced
this pull request
May 20, 2026
Reflect two changes made in vaadin/flow#24366 after review: - Reword "acquires a fresh client-side DOM node" to match the new javadoc phrasing - Non-function return values are now logged as a client-side error rather than silently treated as "no cleanup"
|
Legioth
approved these changes
May 20, 2026
github-project-automation
Bot
moved this from 🔎Iteration reviews
to Done
in Vaadin Flow | Hilla | Kits ongoing work
Collaborator
|
This ticket/PR has been released with Vaadin 25.2.0-beta1. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Application code that wires up client-side listeners on behalf of a server component had two failure modes: re-attach after a real detach produces a fresh DOM node and silently strips installed listeners, and cleanup from a detach listener cannot be sent because the element is leaving the tree.
addJsInitializer registers a JavaScript expression that runs each time the element acquires a fresh client-side DOM, and whose returned cleanup function is invoked when that DOM is discarded or the returned Registration is removed. The init is gated on the existing StateNode.isClientSideInitialized flag, so a same-round-trip detach+reattach (where the client never discards its DOM) does not double-install; a cross-round-trip detach+reattach does. The wrapper JS hosts a per-StateNode cleanup map under window.Vaadin.Flow.initializers and drains it via the client-side StateNode unregister listener, keeping the mechanism scoped per element with no new wire protocol.
Fixes #23900