feat: support named parameters for executeJs#24386
Draft
Artur- wants to merge 4 commits into
Draft
Conversation
|
|
Artur-
commented
May 20, 2026
Artur-
added a commit
that referenced
this pull request
May 20, 2026
Lead the keydown handler body and the listenOn delegate registration JS with `const name = $N;` lines, so the rest of each body reads in terms of `allowedKeys`, `requiredModifiers`, `resetFocus`, `preventDefault` and `locator`, `handler`, `locatorSource` instead of `$0`..`$4`. Once PR #24386 (issue #24385) lands these aliases can be replaced by `JsFunction.withParameter(name, value)` directly.
Artur-
force-pushed
the
claude/fix-vaadin-flow-24385-TESgu
branch
from
7185220 to
ba38bc1
Compare
Add `withParameter(name, value)` to a new `PendingJavaScriptExecution`
subtype returned from `Element.executeJs` and `Page.executeJs`, so the
JavaScript expression can read values by name instead of `$0`, `$1`,
…:
element.executeJs("doSomething(foo)")
.withParameter("foo", "Some value");
The mechanism is purely server-side: each `withParameter` call appends
the value to the invocation's parameter list and prepends
`let <name>=$N;` to the expression. Positional `$0`/`$1`/... parameters
keep their meaning, and parameters cannot be added after the execution
has been sent to the browser. The same `withParameter(name, value)`
helper is added to `JsFunction`, where it captures the value and aliases
it in the body the same way.
`callJsFunction` keeps returning `PendingJavaScriptResult` so it
doesn't expose `withParameter`, matching the issue's request to scope
the feature to `executeJs` only.
Fixes #24385
Mockito.when(page.executeJs(...)).thenReturn(...) now requires PendingJavaScriptExecution instead of PendingJavaScriptResult since that's the new return type of executeJs.
Validation runs again when the invocation is serialised via JacksonCodec, so the explicit dry-run on every withParameter / addNamedParameter call just doubles the work without adding signal.
Changing `Element.executeJs` / `Page.executeJs` to return the new
`PendingJavaScriptExecution` subtype broke binary compatibility for any
pre-built component (e.g. `Upload`) whose bytecode references the
original `PendingJavaScriptResult Element.executeJs(String, Object[])`
signature: the JVM method descriptor includes the return type, so
covariant narrowing on the same class is not link-compatible. The
spring-flow IT job surfaced this as a flood of
`NoSuchMethodError: PendingJavaScriptResult Element.executeJs(...)`
when the constructor of `PrivateView` instantiated `Upload`.
Drop the `PendingJavaScriptExecution` interface and instead expose
`withParameter(String, Object)` as a default method on
`PendingJavaScriptResult` itself, returning `PendingJavaScriptResult`.
`executeJs` keeps its original return type, the chained
`element.executeJs("doSomething(foo)").withParameter("foo", value)` API
keeps working, and pre-built bytecode resolves the method as before.
The default throws `UnsupportedOperationException`; the real behaviour
lives in `PendingJavaScriptInvocation`.
Artur-
force-pushed
the
claude/fix-vaadin-flow-24385-TESgu
branch
from
ba38bc1 to
762602d
Compare
|
Member
|
I'm not sure this is a good idea after all. See |
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
3 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.
Add
withParameter(name, value)to a newPendingJavaScriptExecutionsubtype returned from
Element.executeJsandPage.executeJs, so theJavaScript expression can read values by name instead of
$0,$1,…:
The mechanism is purely server-side: each
withParametercall appendsthe value to the invocation's parameter list and prepends
let <name>=$N;to the expression. Positional$0/$1/... parameterskeep their meaning, and parameters cannot be added after the execution
has been sent to the browser. The same
withParameter(name, value)helper is added to
JsFunction, where it captures the value and aliasesit in the body the same way.
callJsFunctionkeeps returningPendingJavaScriptResultso itdoesn't expose
withParameter, matching the issue's request to scopethe feature to
executeJsonly.Fixes #24385