Repository navigation
docs: describe signal write callbacks, effect timing and nested bindings - #6526
totally-not-ai[bot] wants to merge 6 commits into
Conversation
Document which field value changes run the write callback of bindValue() and that a value the callback doesn't write is reverted, that effects and bindings run before set() returns on the UI thread, how bindings created inside an effect are tracked, and that the read-signals-unconditionally rule also applies to bindings and computed signals. Fixes #6525
Move the bindings subsection after the note that closes the read-unconditionally section, guard the combo box example against a null selection, and state when effects of detached components and changes inside a transaction run.
…llback-effect-timing # Conflicts: # articles/flow/ui-state/usage-examples/conditional-visibility.adoc
Preview DeploymentThis PR has been deployed for preview. URL: https://docs-preview-pr-6526.fly.dev Changed pagesAdded content is highlighted in green; removed content is marked in red on each page.
Built from d80b486 |
vaadin-review-bot
left a comment
There was a problem hiding this comment.
Reviewed the changes — left 2 comments.
| Finding | |
|---|---|
| "doesn't fire a value change event" is wrong when the write callback writes a different value | |
| 💬 | The Binding Rules list still states that any manual set throws, with the exception only noted 380 lines later |
| } | ||
| ---- | ||
|
|
||
| The callback doesn't have to write the value it receives. After the callback returns, the field compares the signal's value with the new value. If they differ, the field shows the signal's value instead, and doesn't fire a value change event. A callback can therefore reject a value by leaving the signal unchanged: |
There was a problem hiding this comment.
After the callback returns, the field compares the signal's value with the new value. If they differ, the field shows the signal's value instead, and doesn't fire a value change event.
This holds only when the callback leaves the signal unchanged (the rejection case). When the callback writes a different value — the trimming case the section goes on to describe — the signal.set() inside the callback runs the binding effect inline, which calls setValue() again with valueSetFromSignal set. That nested call skips the write-callback block and reaches ComponentUtil.fireEvent(...), so a value change event is fired (old value = the untrimmed input, new value = the trimmed one).
Source: flow-server/src/main/java/com/vaadin/flow/component/internal/AbstractFieldSupport.java:280-304; the comment at SignalBindingFeature.java:234 ("no need to fire event, signal change triggered that") says the same.
Suggested rewrite of the second half: "If they differ, the field shows the signal's value instead. When the callback left the signal unchanged, no value change event is fired at all; when it wrote a different value, the event for that value has already been fired by the signal update."
building-ui.adoc:240 · accuracy
There was a problem hiding this comment.
Good catch, thanks. The "no event" statement now applies only when the callback leaves the signal unchanged (the rejection case). The trimming paragraph now says that when the callback writes a different value, the signal update fires a value change event for the value that the signal holds, since the nested setValue() from the binding reaches fireEvent.
| // Signal changes -> field is updated | ||
| ---- | ||
|
|
||
| Unlike the bindings above, a value binding with a write callback doesn't throw when the value is set manually: [methodname]`setValue()` passes the value to the write callback. See <<building-ui#write-callback,When the Write Callback Runs>> for which changes run the callback and how a value that the callback doesn't write is reverted. |
There was a problem hiding this comment.
💬 The Binding Rules list still states that any manual set throws, with the exception only noted 380 lines later
The new sentence is correct, but Binding Rules at line 31 still says without qualification:
While a signal is bound, any attempt to set the value manually (other than through the signal) throws
BindingActiveException
A reader of a reference page reads that bullet list first and stops there. Add the carve-out to the bullet, for example: "… throws BindingActiveException. A value binding with a write callback is the exception: setValue() passes the value to the callback instead."
element-bindings.adoc:413 · clarity
There was a problem hiding this comment.
Agreed. The Binding Rules bullet now carries the exception: a value binding with a write callback passes the value from setValue() to the callback instead of throwing. The bullet links to "When the Write Callback Runs".
Review risk: two-way door — documentation only, four pages under Flow signals.
Summary
Documents four signal behaviors that application code relies on but that the Flow signals pages didn't describe. Each statement was checked against the Flow 25.4 sources (
AbstractFieldSupport,SignalBindingFeature,ElementEffect,Effect,Signal.computed(),ComboBoxDataController).Fixes #6525
What changed
bindValue()write callback runs for every value change that doesn't come from the signal while the field is attached (user edits,setValue(),clear(), and changes the component makes itself, such asComboBox.setItems()clearing the value), with an example that keeps the selection acrosssetItems(). It also covers rejecting a value: when the signal doesn't end up holding the new value, the field shows the signal's value and fires no value change event. Also a short pointer from Conditional Visibility to the read-unconditionally rule.set()returns. Changes from other threads, UIs or sessions are queued throughUI.access()(isBackgroundChange()istrue), and detached components catch up when they're attached again. New Building UI Components in an Effect section:get()calls made during the run, including those in constructors, are dependencies, while bindings and effects created inside track their own signals. New Bindings and Computed Signals subsection under Read Signals Unconditionally, with the&&short-circuit example and a note that the exception is thrown from the binding call.setValue()goes to the write callback instead of throwing, with a link to the new section.Grid selection bindings currently don't revert a rejected value (vaadin/flow-components#10552), so the revert statement is limited to fields.
Test summary
Working on behalf of Artur