feat!: Split computed into separate cached and direct versions (#23822)

Legioth · web-flow · commit ba8ea41f40fd · 2026-03-11T09:21:53.000+02:00
* Split computed into separate cached and direct versions

Previously, the `computed` method was just one out of many ways of
creating a computed signal with the distinction that this computed
signal was also caching. This made it difficult to understand when to
use the method.

Fix this discrepancy by changing `computed` to create a non-cached
computed signal and introducing a separate `cached` method that creates
a cached signal out of any other signal.

* Address review comments

* Clarify error message when inner signal is computed and reads no other
signals
* Document difference between direct lambda and Signal.computed()

* Format
diff --git a/flow-server/src/main/java/com/vaadin/flow/signals/Signal.java b/flow-server/src/main/java/com/vaadin/flow/signals/Signal.java
@@ -29,7 +29,7 @@
 import com.vaadin.flow.signals.function.SignalMapper;
 import com.vaadin.flow.signals.function.TransactionTask;
 import com.vaadin.flow.signals.function.ValueSupplier;
-import com.vaadin.flow.signals.impl.ComputedSignal;
+import com.vaadin.flow.signals.impl.CachedSignal;
 import com.vaadin.flow.signals.impl.Effect;
 import com.vaadin.flow.signals.impl.Transaction;
 import com.vaadin.flow.signals.impl.Transaction.Type;
@@ -48,8 +48,7 @@
  * effect without registering a dependency.
  * <p>
  * This interface can be used for creating simple computed signals as a lambda
- * function that uses other signals. This kind of signal is more limited than
- * {@link #computed(SignalComputation)} since it doesn't cache its value.
+ * function that uses other signals.
  *
  * @param <T>
  *            the signal value type
@@ -68,11 +67,11 @@ public interface Signal<T extends @Nullable Object> extends Serializable {
      * signal value is changed concurrently.
      * <p>
      * Reading the value inside an {@link #unboundEffect(EffectAction)} or
-     * {@link #computed(SignalComputation)} callback sets up that effect or
-     * computed signal to depend on the signal.
+     * {@link #cached(Signal)} callback sets up that effect or cached signal to
+     * depend on the signal.
      * <p>
      * This method must only be called within a reactive context such as an
-     * effect, a computed signal, an explicit {@link #untracked(ValueSupplier)}
+     * effect, cached signal, an explicit {@link #untracked(ValueSupplier)}
      * block, or a {@link #runInTransaction(TransactionTask) transaction}.
      * Calling it outside such a context throws an
      * {@link IllegalStateException}. Use {@link #peek()} for one-time reads
@@ -87,7 +86,7 @@ public interface Signal<T extends @Nullable Object> extends Serializable {
     /**
      * Reads the value without setting up any dependencies. This method returns
      * the same value as {@link #get()} but without creating a dependency when
-     * used inside a transaction, effect or computed signal.
+     * used inside a transaction, effect or cached signal.
      * <p>
      * Unlike {@link #get()}, this method can be called outside a reactive
      * context and is the recommended way to read a signal value for one-time
@@ -108,12 +107,6 @@ default T peek() {
      * passed the value of this signal. If the mapper function accesses other
      * signal values, then the computed signal will also depend on those
      * signals.
-     * <p>
-     * The computed signal does not perform any caching but will instead run the
-     * callback every time the signal value is read. Use
-     * {@link #computed(SignalComputation)} to create a computed signal that
-     * caches the result of running the callback until the value of any
-     * dependency changes.
      *
      * @param <C>
      *            the computed signal type
@@ -228,16 +221,22 @@ static Registration unboundEffect(EffectAction action) {
     /**
      * Creates a new computed signal with the given computation callback. A
      * computed signal behaves like a regular signal except that the value is
-     * not directly set but instead computed from other signals. The computed
-     * signal is automatically updated if any of the used signals are updated.
-     * The computation is lazy so that it only runs when its value is accessed
-     * and only if the previously computed value might have been invalidated by
-     * dependent signal changes. If the computation callback throws a
-     * {@link RuntimeException}, then that exception will be re-thrown when
-     * accessing the signal value. An {@link Signal#unboundEffect(EffectAction)
-     * effect} or computed signal that uses the value from a computed signal
-     * will not be invalidated if the computation is run again but produces the
-     * same value as before.
+     * not directly set but instead computed from other signals. The value is
+     * computed again every time the value is read. Use {@link #cached(Signal)}
+     * to create a signal that computes a new value only if any of the dependent
+     * signals might have changed.
+     * <p>
+     * A computed signal can also be defined directly as a lambda expression.
+     * Using this method enables type inference in cases where the target type
+     * isn't explicitly defined.
+     * 
+     * <pre>
+     * // Type must be explicitly defined for a direct lambda expression
+     * Signal&lt;Integer&gt; signal = () -&gt; stringSignal.get().length();
+     * 
+     * // Type can be inferred by wrapping in computed()
+     * var signal = Signal.computed(() -&gt; stringSignal.get().length());
+     * </pre>
      *
      * @param <T>
      *            the signal type
@@ -247,7 +246,33 @@ static Registration unboundEffect(EffectAction action) {
      */
     static <T extends @Nullable Object> Signal<T> computed(
             SignalComputation<T> computation) {
-        return new ComputedSignal<>(computation);
+        return Objects.requireNonNull(computation)::compute;
+    }
+
+    /**
+     * Creates a new cached signal based on the given inner signal. The inner
+     * signal is typically a computed signal performing an expensive computation
+     * based on other signal values. When getting the value of this signal, a
+     * previously cached value will be used if available as long as that value
+     * is still valid. The cached value remains valid until the value changes
+     * for any of the inner signals.
+     * <p>
+     * If a {@link RuntimeException} is thrown when getting the value of the
+     * inner signal, then that exception will be re-thrown when accessing the
+     * value of this signal. An {@link Signal#unboundEffect(EffectAction)
+     * effect} or outer cached signal that uses the value from a cached signal
+     * will not be invalidated if the inner signal is invalidated but holds the
+     * same value as before invalidation.
+     * 
+     * @param <T>
+     *            the signal type
+     * @param innerSignal
+     *            the inner signal, not <code>null</code>
+     * @return the cached signal not <code>null</code>
+     */
+    static <T extends @Nullable Object> Signal<T> cached(
+            Signal<T> innerSignal) {
+        return new CachedSignal<>(innerSignal);
     }
 
     /**
diff --git a/flow-server/src/main/java/com/vaadin/flow/signals/function/SignalComputation.java b/flow-server/src/main/java/com/vaadin/flow/signals/function/SignalComputation.java
@@ -27,8 +27,7 @@
  * <p>
  * Dependencies are automatically tracked - any signal whose value is accessed
  * during the computation becomes a dependency. The computation is lazy and only
- * runs when the signal value is accessed and the previous value might have been
- * invalidated by dependent signal changes.
+ * runs when the signal value is accessed.
  *
  * @param <T>
  *            the computed value type
diff --git a/flow-server/src/main/java/com/vaadin/flow/signals/impl/CachedSignal.java b/flow-server/src/main/java/com/vaadin/flow/signals/impl/CachedSignal.java
@@ -30,78 +30,79 @@
 import com.vaadin.flow.signals.Signal;
 import com.vaadin.flow.signals.SignalCommand;
 import com.vaadin.flow.signals.function.EffectAction;
-import com.vaadin.flow.signals.function.SignalComputation;
 import com.vaadin.flow.signals.impl.UsageTracker.Usage;
 import com.vaadin.flow.signals.shared.AbstractSharedSignal;
 import com.vaadin.flow.signals.shared.SharedNodeSignal;
 import com.vaadin.flow.signals.shared.impl.SynchronousSignalTree;
 
 /**
- * A signal with a value that is computed based on the value of other signals.
- * The signal value will be lazily re-computed when needed after the value has
- * changed for any of the signals that were used when computing the previous
- * value. If the computation callback throws a {@link RuntimeException}, then
- * that exception will be re-thrown when accessing the value of this signal. An
- * {@link Signal#unboundEffect(EffectAction) effect} or computed signal that
- * uses the value from a computed signal will not be invalidated if the
- * computation is run again but produces the same value as before.
+ * A signal that caches the value of an inner signal. The inner signal is
+ * typically a computed signal performing an expensive computation that should
+ * be run again only if any of its dependencies has changed.
+ * <p>
+ * If a {@link RuntimeException} is thrown when getting the value of the inner
+ * signal, then that exception will be re-thrown when accessing the value of
+ * this signal. An {@link Signal#unboundEffect(EffectAction) effect} or outer
+ * cached signal that uses the value from a cached signal will not be
+ * invalidated if the computation is run again but produces the same value as
+ * before.
  *
  * @param <T>
  *            the value type
  */
-public class ComputedSignal<T extends @Nullable Object>
+public class CachedSignal<T extends @Nullable Object>
         extends AbstractSharedSignal<T> {
 
     /*
      * This state is never supposed to be synchronized across a cluster or to
      * Hilla clients.
      */
-    private record ComputedState(@Nullable Object value,
+    private record CacheState(@Nullable Object value,
             @Nullable RuntimeException exception,
             Usage dependencies) implements Serializable {
     }
 
-    private final SignalComputation<T> computation;
+    private final Signal<T> inner;
 
     private int dependentCount = 0;
     private @Nullable Registration dependencyRegistration;
 
     /**
-     * Creates a new computed signal with the provided compute callback.
+     * Creates a new cached signal with the provided inner signal.
      *
-     * @param computation
-     *            a callback that returns the computed value, not null
+     * @param inner
+     *            a signal to wrap, not null
      */
-    public ComputedSignal(SignalComputation<T> computation) {
+    public CachedSignal(Signal<T> inner) {
         super(new SynchronousSignalTree(true), Id.ZERO, ANYTHING_GOES);
-        this.computation = Objects.requireNonNull(computation);
+        this.inner = Objects.requireNonNull(inner);
     }
 
     /**
-     * As long as nobody listens to changes to this computed signal, we can just
-     * re-compute on demand every time the value is read. But when there's at
+     * As long as nobody listens to changes to this cached signal, we can just
+     * re-validate on demand every time the value is read. But when there's at
      * least one active listener, we need to actively listen to changes in our
      * dependencies.
      * <p>
      * Whenever a dependency changes, we run {@link #getValidState(Data)}. This
-     * causes the compute callback to run again. If that leads to a new value in
+     * causes the inner signal to be read again. If that leads to a new value in
      * the tree, then the {@link Usage} from the super class will be triggered
      * which in notifies out external listeners.
      * <p>
      * We have just a single set of internal listeners on our dependencies even
-     * if there are multiple external listeners so that the compute callback is
-     * run only once. We keep track of how many active external listeners we
-     * have so that our internal listener is active if and only if there's at
-     * least one active external listener.
+     * if there are multiple external listeners so that the inner signal is read
+     * only once. We keep track of how many active external listeners we have so
+     * that our internal listener is active if and only if there's at least one
+     * active external listener.
      */
     private synchronized void revalidateAndListen() {
         // Clear listeners on old dependencies
         if (dependencyRegistration != null) {
             dependencyRegistration.remove();
         }
 
-        // Run compute callback to get new dependencies
-        ComputedState state = getValidState(data(Transaction.getCurrent()));
+        // Read inner value to get new dependencies
+        CacheState state = getValidState(data(Transaction.getCurrent()));
 
         // avoid lambda to allow proper deserialization
         TransientListener usageListener = new TransientListener() {
@@ -131,7 +132,7 @@ private synchronized Registration countActiveExternalListener() {
 
         return () -> {
             if (!removed.getAndSet(true)) {
-                synchronized (ComputedSignal.this) {
+                synchronized (CachedSignal.this) {
                     /*
                      * Decrease the number of active external listeners and stop
                      * listening to our dependencies if there are no more
@@ -194,38 +195,38 @@ public boolean invoke(boolean immediate) {
         };
     }
 
-    private ComputedState getValidState(@Nullable Data data) {
-        ComputedState state = readState(data);
+    private CacheState getValidState(@Nullable Data data) {
+        CacheState state = readState(data);
 
         if (state == null || state.dependencies.hasChanges()) {
             @Nullable
             Object[] holder = new @Nullable Object[2];
             Usage dependencies = UsageTracker.track(() -> {
                 try {
-                    holder[0] = computation.compute();
+                    holder[0] = inner.get();
                 } catch (RuntimeException e) {
                     holder[1] = e;
                 }
             });
             if (dependencies == UsageTracker.NO_USAGE) {
                 throw new MissingSignalUsageException(
-                        "Signal computation must read at least one signal value.");
+                        "A computing inner signal must read at least one other signal value.");
             }
             @Nullable
             Object value = holder[0];
             @Nullable
             RuntimeException exception = (RuntimeException) holder[1];
 
-            state = new ComputedState(value, exception, dependencies);
+            state = new CacheState(value, exception, dependencies);
 
             submit(new SignalCommand.SetCommand(Id.random(), id(),
-                    new ComputedPOJONode(state)));
+                    new CachedPOJONode(state)));
         }
 
         return state;
     }
 
-    private static @Nullable ComputedState readState(@Nullable Data data) {
+    private static @Nullable CacheState readState(@Nullable Data data) {
         if (data == null) {
             return null;
         }
@@ -238,22 +239,22 @@ private ComputedState getValidState(@Nullable Data data) {
         return extractState(value);
     }
 
-    private static ComputedState extractState(JsonNode json) {
-        ComputedPOJONode pojoNode = (ComputedPOJONode) json;
-        return (ComputedState) pojoNode.getPojo();
+    private static CacheState extractState(JsonNode json) {
+        CachedPOJONode pojoNode = (CachedPOJONode) json;
+        return (CacheState) pojoNode.getPojo();
     }
 
-    private static class ComputedPOJONode extends POJONode
+    private static class CachedPOJONode extends POJONode
             implements Serializable {
 
-        public ComputedPOJONode(Object v) {
+        public CachedPOJONode(Object v) {
             super(v);
         }
     }
 
     @Override
     protected @Nullable T extractValue(@Nullable Data data) {
-        ComputedState state = getValidState(data);
+        CacheState state = getValidState(data);
 
         if (state.exception != null) {
             throw state.exception;
@@ -275,13 +276,12 @@ public ComputedPOJONode(Object v) {
 
     @Override
     public T peekConfirmed() {
-        throw new UnsupportedOperationException(
-                "Cannot peek a computed signal");
+        throw new UnsupportedOperationException("Cannot peek a cached signal");
     }
 
     @Override
     public SharedNodeSignal asNode() {
         throw new UnsupportedOperationException(
-                "Cannot use a computed signal as a node signal");
+                "Cannot use a cached signal as a node signal");
     }
 }
diff --git a/flow-server/src/test/java/com/vaadin/flow/signals/impl/ComputedSignalTest.java b/flow-server/src/test/java/com/vaadin/flow/signals/impl/ComputedSignalTest.java
