Skip to content

feat: identify focus and blur invocations with a command object - #25747

Closed
totally-not-ai[bot] wants to merge 1 commit into
mainfrom
feat/js-command-seam-for-focusable
Closed

totally-not-ai[bot] wants to merge 1 commit into
mainfrom
feat/js-command-seam-for-focusable

Conversation

@totally-not-ai

Copy link
Copy Markdown
Contributor

Part of #25734
Related to vaadin/browserless-test#221

missing API · flow-server · external drivers of the client side, such
as browserless test frameworks

Background — the pending JavaScript queue. Server-initiated client
work is scheduled as executeJs and queued per UI until the end of a
request, when dumpPendingJavaScriptInvocations() hands the batch to the
UIDL writer. A queued invocation carries an expression string and its
parameters, and nothing else.

A driver that plays the browser without running JavaScript can therefore
only recognise the invocations it should act on by substring-matching
generated script — today expression.contains("this.focus("), against a
string that is the framework's script wrapped by executeJs. This
prototype lets an invocation carry a typed description of what it does,
so focus and blur are identified by type instead. What a browser receives
is unchanged.

switch (pending.getInvocation().getCommand()) {
case FocusCommand focus -> focus(Element.get(pending.getOwner()));
case BlurCommand blur -> blur(Element.get(pending.getOwner()));
case null, default -> recordUnhandledJavaScript(pending);
}

Risks:

  • ⚠️ Public API: JsCommand, FocusCommand, BlurCommand,
    Element.executeJs(JsCommand), and on JavaScriptInvocation a new
    constructor plus getCommand().
  • ⚠️ Behaviour change: an Element subclass that overrides
    executeJs(String, Object...) no longer intercepts the script of
    focus() and blur(), which now take the command overload.
  • ❓ Open question: getCommand() returns null for plain JavaScript,
    against the sentinel-over-null convention. A sentinel would allocate
    per executeJs, and case null, default is needed regardless because
    JsCommand cannot be sealed across features.
  • 💾 Serialization: a command is a serializable value held by the queued
    invocation.
  • ✅ Generated expressions and parameters byte for byte as before, no
    protocol, threading or performance change.

Context. Focusable built its script inline, so the only
server-side trace of a focus was the text of that script. The
trigger/action model already describes client-side work as Java values
(Action#toJs); this is the same idea for one-shot executeJs.

  • JsCommand (new) is a typed, immutable description of a client-side
    operation together with the expression and parameters that perform it.
  • Element.executeJs(JsCommand) (new) schedules the command's script and
    keeps the command on the queued invocation, where
    JavaScriptInvocation.getCommand() returns it. The command never
    leaves the server.
  • Focusable.focus(FocusOption...) and blur() schedule FocusCommand
    and BlurCommand; the scripts move into those records unchanged, and
    the options travel as FocusOption values rather than only as JSON.
  • Prototype scope is Focusable only. Page.executeJs, WebStorage,
    Page.fetchCurrentURL, History and scrollIntoView are the
    candidates next in line and need no further API to adopt it.

Focusable.focus() and blur() schedule their JavaScript through the new
Element.executeJs(JsCommand), so the pending invocation carries a typed
FocusCommand or BlurCommand. A driver of the client side that cannot run
JavaScript recognizes the invocation by the type of its command instead
of by matching the text of the generated expression, which is the
framework's script wrapped by executeJs.

The expression and the parameters sent to a browser are unchanged.

Part of #25734
@totally-not-ai

Copy link
Copy Markdown
Contributor Author

Type of change

Prototype, kept as a draft on purpose: it is here to make the shape
discussable, and the open questions below are decisions rather than
details. Public API, additive; nothing is removed or re-signed.

How to test

mvn test -pl flow-server -Dtest=FocusableTest,ElementTest
mvn test -pl flow-server     # whole module: 5284 tests, 0 failures

No integration test is added: the expressions and parameters that reach
the browser are unchanged, so FocusBlurView / FocusBlurIT keep
guarding the browser behaviour. The three generated scripts (focus,
focus with options, blur) were compared before and after the move into
the command records and are byte identical, and the existing expression
assertions in FocusableTest still pass untouched.

API changes

Addition Shape
com.vaadin.flow.dom.JsCommand interface, Serializable; String getExpression(), default List<Object> getParameters()
Element#executeJs(JsCommand) schedules the command's expression and parameters, keeping the command server-side
com.vaadin.flow.component.FocusCommand record FocusCommand(List<FocusOption> options), plus a varargs constructor
com.vaadin.flow.component.BlurCommand record BlurCommand()
UIInternals.JavaScriptInvocation#JavaScriptInvocation(JsCommand, String, Object...) new constructor; the existing one delegates with no command
UIInternals.JavaScriptInvocation#getCommand() the command, or null for plain JavaScript

What a browserless driver gets out of it

  1. Identification by type. instanceof / a pattern switch instead of
    expression.contains("this.focus("). A change to the script cannot
    silently turn a simulation into a no-op.
  2. Typed arguments. FocusCommand#options() gives the
    FocusOption values that were passed, rather than the JSON that was
    built for the browser.
  3. The target, as the owner of the pending invocation:
    Element.get(pending.getOwner()).
  4. Order and the long tail preserved. The queue is still drained
    whole and in order; invocations with no command are plain JavaScript
    the driver can record and report. FocusableTest has that loop as a
    test, over a queue of focus → application executeJs → blur.
  5. Return values need nothing new: a driver already resolves a
    subscribed invocation with complete(...).

Server-initiated focus and blur mark themselves for the client so the
resulting event reports isFromClient() == false; that is documented on
the two commands, since a driver acting on the command has to do the
same.

Relation to the alternatives in #25734

This is ask 3 of that issue, but as a value object rather than a
PendingJavaScriptInvocation subtype: a subtype would have to be
constructed inside Element#scheduleJavaScriptInvocation, which is where
the runWhenAttached / beforeClientResponse / countWhenAttached
plumbing lives, while a command needs no new plumbing at all. It also
replaces ask 1 and ask 2 (public constants, a string tag) with something
that carries the arguments.

A FocusClient-style port along the lines of GeolocationClient stays
possible and is not mutually exclusive, but it is a bigger decision:
focus/blur are element-scoped default methods with no server-side
state for a client to own, and a port does nothing for WebStorage,
Page.fetchCurrentURL, History or scrollIntoView.

Open questions

  1. getCommand() returning null versus a sentinel command for plain
    JavaScript. A sentinel would allocate for every executeJs, and a
    case null, default arm is needed either way because JsCommand
    cannot be sealed across features.
  2. An Element subclass that overrides executeJs(String, Object...)
    no longer intercepts focus() / blur(). Only test code does that
    today, but it is a real behaviour change.
  3. Should Page.executeJs get the same overload here, or with the first
    UI-level command?
  4. Should FocusCommand expose derived focusVisible() /
    preventScroll() accessors instead of the raw option list, so a
    driver does not repeat the last-one-wins resolution?

@Artur- @mcollovati — the queue side of #25734, small enough to judge from
the diff.

@totally-not-ai

totally-not-ai Bot commented Sep 16, 2026

Copy link
Copy Markdown
Contributor Author

The invoker shape from #10759, as an alternative to this one on the same feature: #25749. It replaces the command records with a FocusJs interface whose methods carry their JavaScript as @JsExpression constants, keeping JsCommand and the queue plumbing from here. The two are compared in #25749 (comment).

@sonarqubecloud

Copy link
Copy Markdown

@github-actions

Copy link
Copy Markdown
Contributor

Test Results

 1 452 files  ±0   1 536 suites  ±0   1h 30m 3s ⏱️ - 8m 16s
11 905 tests +5  11 837 ✅ +5  68 💤 ±0  0 ❌ ±0 
12 223 runs  +5  12 155 ✅ +5  68 💤 ±0  0 ❌ ±0 

Results for commit 621c889. ± Comparison against base commit a2e6fca.

@totally-not-ai

Copy link
Copy Markdown
Contributor Author

Dropped in favour of the invoker shape, which is now the single PR: #25749. It carries the queue-side identification this PR was about, plus the build and client sides that make a call need no unsafe-eval.

@totally-not-ai totally-not-ai Bot closed this Sep 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants