Signal.effect is not triggered when value is accessed in ui.access

[abdullahtellioglu]

Description of the bug

https://github.com/user-attachments/assets/e24f9913-4c4e-4c4e-93fb-41add12ad208

Expected behavior

Signal.effect should be invoked regardless of where the value is obtained

Minimal reproducible example

This is the example from the video:

    public MainView() {
        TextField textField = new TextField();
        textField.setValueChangeMode(ValueChangeMode.EAGER);
        textField.getElement().bindProperty("value", AppState.INSTANCE.getNameSignal());
        textField.addValueChangeListener(e -> AppState.INSTANCE.getNameSignal().value(e.getValue()));
        add(textField);
        Span mySpan = new Span();
        mySpan.setText(AppState.INSTANCE.getNameSignal().value());
        add(mySpan);
        Signal.effect(() -> {
            ValueSignal<String> nameSignal = AppState.INSTANCE.getNameSignal();
            System.out.println("Signal effect");
            getUI().ifPresent(ui -> ui.access(() -> {
                mySpan.setText(nameSignal.value());
            }));

        });
    }

@Getter
@Setter
public class AppState {
    public static final AppState INSTANCE = new AppState();
    private final ValueSignal<String> nameSignal = SignalFactory.IN_MEMORY_SHARED.value("name", String.class, "");
}

Versions

Flow: 25.0.0-beta9 Vaadin: 25.0.0-beta9 Spring Boot: 4.0.0 Spring: 7.0.1 Spring Data JPA: Copilot: 25.0.0-beta7 Frontend Hotswap: Disabled ⋅ Pre-built Bundle OS: aarch64 Mac OS X 14.7.1 Java: JetBrains s.r.o. 21.0.6 Browser: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/142.0.0.0 Safari/537.36 Java Hotswap: Java Hotswap is enabled IDE Plugin: 1.5.6 IntelliJ

[Legioth]

We can't make asynchronous callbacks from effects work in the general case since there's no way for us to know that an asynchronous task has been submitted or that it will complete in a reasonable time. We could support it specifically with UI::access since that's under our control but I'm not sure if it would make any sense or just be more confusing that some callbacks work and other callbacks don't?

One of the fundamental benefits of signals is that UI::access is automatically handled so that you don't have to be concerned with it. If you want to update UI components in an effect, then it's in all ways better to use ComponenEffect or a built-in bindXyz method. In your example that could be one of these alternatives:

// Alternative 1: component bind method (not yet implemented)
mySpan.bindText(nameSignal);

// Alternative 2: element bind method
mySpan.getElement().bindText(nameSignal);

// Alternative 3: bind helper in ComponentEffect
ComponentEffect.bind(mySpan, nameSignal, Span::setText);

// Alternative 4: ComponentEffect
ComponentEffect.effect(mySpan, () -> mySpan.setText(nameSignal.value())); 

// Alternative 5: read value outside callback
Signal.effect(() -> {
  String value = nameSignal.value();
  ui.access(() -> mySpan.setText(value));
});

The are two remaining questions:

1. Is there some reasonable use case that can't be supported using the mechanisms suggested above?
2. Is there something we could do to make it easier for developers to understand what they're supposed to do? https://github.com/vaadin/flow/issues/22123 comes to mind but it would probably also be quite annoying.

[abdullahtellioglu]

I was not expecting the code above not to work when the value is obtained in ui.access(), but it works when the value is obtained in effect. Those alternatives could be good replacements, but they force developers to write code in an expected way. Once the code is written differently, as in the example, the code suddenly stops working without any warning/exception.

I, personally, would expect Signal.effect to be invoked whether a value is obtained in the effect block or in an async task.

These are my observations, and feel free to close the issue if you would like

[Legioth]

The core issue is that there's no way for us to support it in general. Something like this is just impossible for us to detect:

Signal.effect(() -> {
  Thread.startVirtualThread(() -> System.out.println(someSignal.value()));
});

We could support UI::access but I suspect it's better to just teach developers to not do anything asynchronous rather than teaching which async ways are ok and which aren't.

And that leads to the question of whether it would be better to make Signal::value throw an exception if called from outside an effect or computed callback. That will be annoying in some cases but it will also help developers realize that they're trying to do something that won't work, and thus make it easier for them to realize what they need to change.

[abdullahtellioglu]

The core issue is that there's no way for us to support it in general. Something like this is just impossible for us to detect:

Can't StackTraceElements be used to detect?

I tried a quick test from the example above, and the stacktrace might give where the value() is called. An exception might be thrown on value registration, or a warning can be printed. There might be false positives, though

[Legioth]

The stacktrace shows where in the code value() is run but there's no association back to the effect invocation from which the async job was started.

In this code example, the challenge is that you cannot from within value() find the now or callback values so that you could run the whole effect again when the signal value changes. It doesn't even have access to the Runnable passed to startVirtualThread to be able to run just that part again.

long now = System.currentTimeMillis();
Runnable callback = () -> {
  System.out.println("Signal " + now);
  Thread.startVirtualThread(() -> System.out.println(someSignal.value()));
}
Signal.effect(callback);