Skip to content

Commit 0908eaa

Browse files
authored
feat: apply ElementEffect once when created (#23824) (#23849)
* feat: apply ElementEffect once when created ElementEffect is executed immediately once when created. This affects all Signal.effect calls with an owner component. Immediate execution ensures that exceptions can be thrown eagerly when e.g. bindText(signal) is called, not later when component is attached. Fixes #23813 * Fix Binder * Fix NodeMap and tests * ensure onChange is called immediately for initial runs * Updated Javadoc of bind methods
1 parent 9ab8402 commit 0908eaa

39 files changed

+659
-251
lines changed

flow-data/src/main/java/com/vaadin/flow/data/binder/Binder.java

Lines changed: 28 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1727,22 +1727,37 @@ private FIELDVALUE convertToFieldType(TARGET target) {
17271727
private void initInternalSignalEffectForValidators() {
17281728
if (signalRegistration == null
17291729
&& getField() instanceof Component component) {
1730-
// Constant signal that is only read and never modified.
1731-
// Satisfies the signal usage requirement for bindings
1732-
// without signal-using validators in the chain.
1733-
var usageGuard = new ValueSignal<Void>(null);
1734-
signalRegistration = Signal.effect(component, ctx -> {
1735-
usageGuard.get();
1736-
Result<TARGET> result = executeConversionChain();
1737-
if (!ctx.isInitialRun()) {
1738-
BindingValidationStatus<TARGET> status = toValidationStatus(
1739-
result);
1740-
fireValidationEvents(status);
1741-
}
1742-
});
1730+
if (component.isAttached()) {
1731+
initInternalSignalEffectForValidators(component);
1732+
} else {
1733+
component.addAttachListener(event -> {
1734+
if (event.isInitialAttach()) {
1735+
event.unregisterListener();
1736+
}
1737+
initInternalSignalEffectForValidators(
1738+
event.getSource());
1739+
});
1740+
}
17431741
}
17441742
}
17451743

1744+
private void initInternalSignalEffectForValidators(
1745+
Component component) {
1746+
// Constant signal that is only read and never modified.
1747+
// Satisfies the signal usage requirement for bindings
1748+
// without signal-using validators in the chain.
1749+
var usageGuard = new ValueSignal<Void>(null);
1750+
signalRegistration = Signal.effect(component, ctx -> {
1751+
usageGuard.get();
1752+
Result<TARGET> result = executeConversionChain();
1753+
if (!ctx.isInitialRun()) {
1754+
BindingValidationStatus<TARGET> status = toValidationStatus(
1755+
result);
1756+
fireValidationEvents(status);
1757+
}
1758+
});
1759+
}
1760+
17461761
/**
17471762
* Handles the value change triggered by the bound field.
17481763
*

flow-data/src/test/java/com/vaadin/flow/data/binder/BinderSignalTest.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -630,6 +630,7 @@ public void crossFieldValidation_onlyWorksWithAttachedFields() {
630630
assertFalse(firstNameField.isInvalid());
631631
assertTrue(binder.isValid());
632632

633+
lastNameSignal.set("Smith");
633634
// Now attach the fields
634635
UI.getCurrent().add(firstNameField, lastNameField);
635636

flow-html-components/src/test/java/com/vaadin/flow/component/html/AnchorBindTextTest.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,12 +43,12 @@ public void constructor_href_signal_lifecycleAndUpdates() {
4343
// Detached: binding inactive
4444
var signal = new ValueSignal<>("one");
4545
Anchor anchor = new Anchor("/path", signal);
46-
// no propagation while detached
47-
assertEquals("", anchor.getText());
46+
// Initial value is applied
47+
assertEquals("one", anchor.getText());
4848

4949
// Update before attach is ignored
5050
signal.set("two");
51-
assertEquals("", anchor.getText());
51+
assertEquals("one", anchor.getText());
5252

5353
// Attach -> latest value applied
5454
UI.getCurrent().add(anchor);

flow-server/src/main/java/com/vaadin/flow/component/Component.java

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -594,10 +594,11 @@ public static <T extends Component> T from(Element element,
594594

595595
/**
596596
* Binds a {@link Signal}'s value to the <code>visible</code> property of
597-
* this component and keeps property synchronized with the signal value
598-
* while the component is in attached state. When the element is in detached
599-
* state, signal value changes have no effect. <code>null</code> signal
600-
* unbinds the existing binding.
597+
* this component. The visibility is set immediately with the current signal
598+
* value when the binding is created, and is kept synchronized with any
599+
* subsequent signal value changes while the component is in attached state.
600+
* When the element is in detached state, signal value changes have no
601+
* effect. <code>null</code> signal unbinds the existing binding.
601602
* <p>
602603
* While a Signal is bound to a property, any attempt to set the visibility
603604
* manually with {@link #setVisible(boolean)} throws

flow-server/src/main/java/com/vaadin/flow/component/HasEnabled.java

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -102,11 +102,12 @@ default boolean isEnabled() {
102102
}
103103

104104
/**
105-
* Binds a {@link Signal}'s value to the enabled state of this component and
106-
* keeps the state synchronized with the signal value while the element is
107-
* in attached state. When the element is in detached state, signal value
108-
* changes have no effect. <code>null</code> signal unbinds the existing
109-
* binding.
105+
* Binds a {@link Signal}'s value to the enabled state of this component.
106+
* The enabled state is set immediately with the current signal value when
107+
* the binding is created, and is kept synchronized with any subsequent
108+
* signal value changes while the element is in attached state. When the
109+
* element is in detached state, signal value changes have no effect.
110+
* <code>null</code> signal unbinds the existing binding.
110111
* <p>
111112
* While a Signal is bound to an enabled state, any attempt to set the state
112113
* manually with {@link #setEnabled(boolean)} throws

flow-server/src/main/java/com/vaadin/flow/component/HasHelper.java

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,10 @@ default void setHelperText(String helperText) {
7474
}
7575

7676
/**
77-
* Binds a signal's value to the component's helper text so that the helper
78-
* text is updated when the signal's value is updated.
77+
* Binds a signal's value to the component's helper text. The helper text is
78+
* set immediately with the current signal value when the binding is
79+
* created, and is kept synchronized with any subsequent signal value
80+
* changes while the component is in attached state.
7981
* <p>
8082
* Passing {@code null} as the {@code signal} removes any existing binding
8183
* for the given helper text. When unbinding, the current helper text is
@@ -86,9 +88,8 @@ default void setHelperText(String helperText) {
8688
* {@link com.vaadin.flow.signals.BindingActiveException}. The same happens
8789
* when trying to bind a new Signal while one is already bound.
8890
* <p>
89-
* Bindings are lifecycle-aware and only active while this component is in
90-
* the attached state; they are deactivated while the component is in the
91-
* detached state.
91+
* When the component is in the detached state, signal value changes have no
92+
* effect.
9293
*
9394
* @param helperTextSignal
9495
* the signal to bind, not <code>null</code>

flow-server/src/main/java/com/vaadin/flow/component/HasPlaceholder.java

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,10 @@ default String getPlaceholder() {
5555
}
5656

5757
/**
58-
* Binds a signal's value to the component's placeholder so that the
59-
* placeholder is updated when the signal's value is updated.
58+
* Binds a signal's value to the component's placeholder. The placeholder is
59+
* set immediately with the current signal value when the binding is
60+
* created, and is kept synchronized with any subsequent signal value
61+
* changes while the component is in attached state.
6062
* <p>
6163
* Passing {@code null} as the {@code signal} removes any existing binding
6264
* for the given placeholder. When unbinding, the current placeholder is
@@ -67,9 +69,8 @@ default String getPlaceholder() {
6769
* {@link com.vaadin.flow.signals.BindingActiveException}. The same happens
6870
* when trying to bind a new signal while one is already bound.
6971
* <p>
70-
* Bindings are lifecycle-aware and only active while this component is in
71-
* the attached state; they are deactivated while the component is in the
72-
* detached state.
72+
* When the component is in the detached state, signal value changes have no
73+
* effect.
7374
*
7475
* @param placeholderSignal
7576
* the signal to bind, not <code>null</code>

flow-server/src/main/java/com/vaadin/flow/component/HasSize.java

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -421,11 +421,12 @@ default void setSizeUndefined() {
421421
}
422422

423423
/**
424-
* Binds a {@link Signal}'s value to the width of this component and keeps
425-
* the width synchronized with the signal value while the component is in
426-
* attached state. When the component is in detached state, signal value
427-
* changes have no effect. <code>null</code> signal unbinds the existing
428-
* binding.
424+
* Binds a {@link Signal}'s value to the width of this component. The width
425+
* is set immediately with the current signal value when the binding is
426+
* created, and is kept synchronized with any subsequent signal value
427+
* changes while the component is in attached state. When the component is
428+
* in detached state, signal value changes have no effect. <code>null</code>
429+
* signal unbinds the existing binding.
429430
* <p>
430431
* The width should be in a format understood by the browser, e.g. "100px"
431432
* or "2.5em".
@@ -462,10 +463,11 @@ default SignalBinding<?> bindWidth(Signal<String> widthSignal) {
462463
}
463464

464465
/**
465-
* Binds a {@link Signal}'s value to the height of this component and keeps
466-
* the height synchronized with the signal value while the component is in
467-
* attached state. When the component is in detached state, signal value
468-
* changes have no effect.
466+
* Binds a {@link Signal}'s value to the height of this component. The
467+
* height is set immediately with the current signal value when the binding
468+
* is created, and is kept synchronized with any subsequent signal value
469+
* changes while the component is in attached state. When the component is
470+
* in detached state, signal value changes have no effect.
469471
* <p>
470472
* The height should be in a format understood by the browser, e.g. "100px"
471473
* or "2.5em".

flow-server/src/main/java/com/vaadin/flow/component/HasStyle.java

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -165,12 +165,15 @@ default void addClassNames(String... classNames) {
165165
}
166166

167167
/**
168-
* Binds the presence of the given CSS class name to a {@link Signal}. When
169-
* the signal's value is {@code true}, the class name is added; when
170-
* {@code false}, the class name is removed.
168+
* Binds the presence of the given CSS class name to a {@link Signal}. The
169+
* class name is immediately added or removed based on the current signal
170+
* value when the binding is created. When the signal's value is
171+
* {@code true}, the class name is added; when {@code false}, the class name
172+
* is removed.
171173
* <p>
172-
* The binding is active while the component is attached to a UI. When
173-
* detached, signal value changes have no effect.
174+
* The binding is kept synchronized with any subsequent signal value changes
175+
* while the component is attached to a UI. When detached, signal value
176+
* changes have no effect.
174177
*
175178
* @param className
176179
* the CSS class name to toggle, not {@code null} or blank
@@ -185,9 +188,11 @@ default SignalBinding<Boolean> bindClassName(String className,
185188
}
186189

187190
/**
188-
* Binds the CSS class names of this component to a {@link Signal} so that
189-
* the class list is dynamically updated to match the signal's value. Only
190-
* one group binding is allowed per component.
191+
* Binds the CSS class names of this component to a {@link Signal}. The
192+
* class list is immediately updated to match the current signal value when
193+
* the binding is created, and is kept synchronized with any subsequent
194+
* signal value changes while the component is attached. Only one group
195+
* binding is allowed per component.
191196
* <p>
192197
* The group binding coexists with static values and individual toggle
193198
* bindings.

flow-server/src/main/java/com/vaadin/flow/component/HasText.java

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -167,11 +167,12 @@ default WhiteSpace getWhiteSpace() {
167167
}
168168

169169
/**
170-
* Binds a {@link Signal}'s value to the text content of this component and
171-
* keeps the text content synchronized with the signal value while the
172-
* element is in attached state. When the element is in detached state,
173-
* signal value changes have no effect. <code>null</code> signal unbinds the
174-
* existing binding.
170+
* Binds a {@link Signal}'s value to the text content of this component. The
171+
* text content is set immediately with the current signal value when the
172+
* binding is created, and is kept synchronized with any subsequent signal
173+
* value changes while the element is in attached state. When the element is
174+
* in detached state, signal value changes have no effect. <code>null</code>
175+
* signal unbinds the existing binding.
175176
* <p>
176177
* While a Signal is bound, any attempt to set the text content manually
177178
* throws {@link com.vaadin.flow.signals.BindingActiveException}. Same

0 commit comments

Comments
 (0)