Add multi-read, fix a major issue with recomputation.

Previously, `x = a.read(a => signalB)` would redo the computation when
signalB changes. Now it just refreshes the x value, without redoing
the computation, which would be useless since a hasn't changed.

README.md

@@ -43,7 +43,7 @@ Monad in the functional programming community. That said, you don't need
  of implementation.
 
 The whole external interface can be summarized in two interfaces and
-one constructor function:
+two exported functions:
 
 ```ts
 export interface Signal<T> {
@@ -55,11 +55,48 @@ export interface Input<T> extends Signal<T> {
     set value(t: T);
 }
 
-export function input<T>(t: T): Input<T> {...};
+export function input<T>(t: T): Input<T>;
+
+export function read<A, B, R>(a: Signal<A>, b: Signal<B>, f: (a: A, b: B): R|Signal<R>): Signal<R>;
+export function read<A, B, R>(a: Signal<A>, b: Signal<B>, c: Signal<C>, f: (a: A, b: B, c: C): R|Signal<R>): Signal<R>;
+// etc ...
 ```
 
 The only non-trivial method is `.read` which one can think of like `Promise.then`, but with different meaning (can run multiple times for example).
 
+## Why is reading multiple signals needed?
+
+At first it might seem that reading multiple signals is not necessary. Instead of `read(a, b, f)` one
+can write `a.read(a => b.read(b => f(a, b))`. But their semantics are different. In the first case, when
+`a` changes a whole new signal is recreated on each recomputation. In the second case, no new signals are
+created so it is likely cheaper performance-wise.
+
+However, reading multiple signals at once means that necessarily `f` is recomputed on changes in `a` or `b`,
+even if the computation doesn't need one of them.
+
+So
+
+```ts
+read(a, b, (a, b) => {
+    if (a === 0) return 0;
+    return a + b;
+});
+```
+
+would recompute on `b` changes even if `a` is zero. While
+
+```ts
+a.read(a => {
+    if (a === 0) return 0;
+    return b.read(b => {
+        return a + b;
+    });
+});
+```
+
+Will be more efficient in recomputations, at the expense of recreating an inner signal for `b.read` on
+each `a` recomputation.
+
 ## Background
 
 I have been interested in the following three areas:
@@ -153,3 +190,4 @@ I consider this library still a work-in-progress. My next tasks would be:
 - [ ] Make sure there are no mem leaks. I used WeakRef, but didn't test that.
 - [ ] add effects, i.e. computations that are recomputed without an explicit .value read.
 - [ ] some `async/await`-like synthetic sugar to make this acceptable for the JS developer. Or just wait for `do`-notation to land in ECMAScript. Try to use something like [https://github.com/pelotom/burrido](https://github.com/pelotom/burrido) to use generators?
+- [ ] Smarter keeping track of values. `x = 1; x = 2; x = 1` will trigger recomputation of all x dependencies.

caching.test.ts

@@ -91,27 +91,27 @@ test('detached signals do not trigger a recomputation', () => {
 
     x.value = 'x2';
     expect(z.value).toBe('x2');
-    expect(bCount).toBe(2);
+    expect(bCount).toBe(1);
     expect(xCount).toBe(2);
     expect(yCount).toBe(0);
 
     // flip the boolean
     b.value = false;
     expect(z.value).toBe('y2');
-    expect(bCount).toBe(3);
+    expect(bCount).toBe(2);
     expect(xCount).toBe(2);
     expect(yCount).toBe(1);
 
     x.value = 'x3';
     expect(z.value).toBe('y2');
-    expect(bCount).toBe(3);
+    expect(bCount).toBe(2);
     expect(xCount).toBe(2);
     expect(yCount).toBe(1);
 
     y.value = 'y3';
     expect(z.value).toBe('y3');
     // no change, all cached.
-    expect(bCount).toBe(4);
+    expect(bCount).toBe(2);
     expect(xCount).toBe(2);
     expect(yCount).toBe(2);
 });
@@ -125,4 +125,26 @@ test('uncomputed signals are not skipped', () => {
 
     x.value = 2;
     expect(z.value).toBe(2);
+});
+
+test('nested reads are cached', () => {
+    const x = input(0);
+    const y = input(0);
+    let countX = 0;
+    let countY = 0;
+    const sum = x.read(x => {
+        countX++;
+        return y.read(y => {
+            countY++;
+            return x + y;
+        });
+    });
+    expect(sum.value).toBe(0);
+    expect(countX).toBe(1);
+    expect(countY).toBe(1);
+
+    y.value = 1;
+    expect(sum.value).toBe(1);
+    expect(countX).toBe(1);
+    expect(countY).toBe(2);
 });

index.ts

@@ -21,10 +21,7 @@ export function input<T>(t: T, debugName?: string): Input<T> {
 const REFERENTIAL_EQUALITY = <T>(a: T, b: T) => a === b;
 
 enum State {
-    INITIAL,
-    // For optimization each signal keeps track of whether its value is the same as previous value.
-    CLEAN_AND_SAME_VALUE,
-    CLEAN_AND_DIFFERENT_VALUE,
+    CLEAN,
     DIRTY,
 }
 
@@ -45,26 +42,43 @@ let DEBUG = false;
  * - the set of inputs that were read during the computation.
  * - the state of the previous computation.
  */
-type Continuation<T> = (t: T, inputs: Set<InputImpl<any>>, upstreamState: State) => void;
+type Continuation<T> = (t: T|SignalImpl<T>, inputs: Set<InputImpl<any>>, comp: IN_SIGNALS_STATE) => void;
 // In Haskell, this would be a value in the continuation monad, but I can't
 // find a standard way to name it so it is different from the continuation type.
 /**
  * One extra piece of information compared to the continuation type:
  *   - the current state of the signal.
  */
-type ContValue<T> = (ct: Continuation<T>, curState: State) => void;
+type ContValue<T> = (ct: Continuation<T>, lastReadVersions: Map<SignalImpl<any>, number>) => void;
 
 /**
  * Global count of signals created. Used for debugging only.
  */
 let COUNT = 0;
 
+enum IN_SIGNALS_STATE {
+    AT_LEAST_ONE_NEW,
+    SAME,
+}
+
 class SignalImpl<T> implements Signal<T> {
     /**
      * True initially, so that the first read will trigger a computation.
      * After that, false when at least one input in the transitive closure has changed.
      */
-    state = State.INITIAL;
+    state = State.DIRTY;
+
+    /**
+     * Increments if the current value was different from the last value.
+     * Note that it doesn't keep any back-history tracking. So 1->2->1 will be 3 version bumps.
+     */
+    version = 0;
+
+    /**
+     * Mapping between signals read as inputs for the current computation and corresponding
+     * version they were at.
+     */
+    lastRead = new Map<SignalImpl<unknown>, number>();
 
     /**
      * Cache value of the computation. Represents the signal value only if state is CLEAN_.
@@ -100,29 +114,46 @@ class SignalImpl<T> implements Signal<T> {
     toString() {
         return `Signal('${this.name}', ${this.id})`;
     }
+
+    lastSignal: SignalImpl<T>| null = null;
+
     get value(): T {
         this.checkGlobalState();
-        if (this.state === State.CLEAN_AND_DIFFERENT_VALUE ||
-            this.state === State.CLEAN_AND_SAME_VALUE) return this.#cachedValue;
+        if (this.state === State.CLEAN) return this.#cachedValue;
         // during recomputation the readers can change, so we remove them first.
         // TODO: use counters trick to optimize this.
         // https://github.com/angular/angular/tree/a1b4c281f384cfd273d81ce10edc3bb2530f6ecf/packages/core/src/signals#equality-semantics
         for (let i of this.inputs) i.readers.delete(this.#ref);
-        this.ct((x: T, inputs, upstreamState) => {
-            if (upstreamState === State.CLEAN_AND_SAME_VALUE && this.state !== State.INITIAL) {
-                this.state = State.CLEAN_AND_SAME_VALUE;
-                // inputs can't change if the downstream value was the same.
-                return;
+        this.ct((x: T| SignalImpl<T>, inputs, inSignalsState) => {
+
+            if (inSignalsState === IN_SIGNALS_STATE.SAME) {
+                if (this.lastSignal === null) {
+                    this.state = State.CLEAN;
+                    return;
+                }
+                // still need to refresh the cached value
+                // rerun the rest of the code, because lastSignal could have changed.
+                x = this.lastSignal;
+                inputs = this.inputs;
             }
-            if (this.eq(x, this.#cachedValue)) {
-                this.state = State.CLEAN_AND_SAME_VALUE;
-            } else {
-                this.state = State.CLEAN_AND_DIFFERENT_VALUE;
+
+            // Adding auto-wrapping of pure values, akin to JS promises.
+            // This means we can never create Signal<Signal<T>>.
+            // Are we trading off some capabilities for syntactic convenience?
+            if ((x instanceof SignalImpl)) {
+                this.lastSignal = x;
+                x = x.value;
+                inputs = new Set([...inputs, ...this.lastSignal.inputs]);
+            }
+
+            if (!this.eq(x, this.#cachedValue)) {
                 this.#cachedValue = x;
+                this.version += 1;
             }
+            this.state = State.CLEAN;
             // note that inputs can change even if the value is the same.
             this.inputs = inputs;
-        }, this.state);
+        }, this.lastRead);
         for (let i of this.inputs) i.readers.add(this.#ref);
         return this.#cachedValue;
     }
@@ -141,6 +172,7 @@ class InputImpl<T> extends SignalImpl<T> {
     inputs: Set<InputImpl<any>> = new Set([this]);
     // Using WeakRef here to avoid retaining reference to readers.
     readers: Set<WeakRef<SignalImpl<any>>> = new Set();
+    state = State.CLEAN;
 
     constructor(private val: T, name?: string) {
         super(_ => { throw new Error(`error: inputs continuation shouldn't be called`) }, name);
@@ -152,6 +184,7 @@ class InputImpl<T> extends SignalImpl<T> {
     set value(t: T) {
         this.checkGlobalState();
         if (this.eq(this.val, t)) return;
+        this.version += 1;
         this.val = t;
         for (let r of this.readers) {
             let reader = r.deref();
@@ -161,30 +194,32 @@ class InputImpl<T> extends SignalImpl<T> {
 }
 
 function makeCt<T>(signals: SignalImpl<any>[], f: (...args: any[]) => T | SignalImpl<T>): ContValue<T> {
-    return (ct, curState) => {
+    return (ct, lastReadVersions) => {
         let inputs = new Set<InputImpl<any>>();
+
+        // have to call getters first to refresh all values and internal state.
         let values = signals.map(s => s.value);
+
         for (let s of signals) {
             inputs = new Set([...inputs, ...s.inputs]);
         }
 
-        if (signals.map(x => x.state).every(s => s === State.CLEAN_AND_SAME_VALUE) && curState !== State.INITIAL) {
-            // the first two values don't matter, because we are not going to use them.
-            // TODO: find a better way to do this.
-            ct(null as any, null as any, State.CLEAN_AND_SAME_VALUE);
-            return;
+        let sameV = true;
+        for (let s of signals) {
+            if (!lastReadVersions.has(s) || lastReadVersions.get(s) != s.version) {
+                // just update in place since we won't need the last read state any more.
+                lastReadVersions.set(s, s.version);
+                sameV = false;
+            }
+        }
+        if (sameV) {
+            return ct(null as any, null as any, IN_SIGNALS_STATE.SAME);
         }
 
         globalState = GLOBAL_STATE.COMPUTING;
         let res = f(...values);
         globalState = GLOBAL_STATE.READY;
-
-        // Adding auto-wrapping of pure values, akin to JS promises.
-        // This means we can never create Signal<Signal<T>>.
-        // Are we trading off some capabilities for syntactic convenience?
-        if (!(res instanceof SignalImpl)) return ct(res, inputs, State.CLEAN_AND_DIFFERENT_VALUE);
-        let resV = res.value;
-        ct(resV, new Set([...inputs, ...res.inputs]), res.state);
+        ct(res, inputs, IN_SIGNALS_STATE.AT_LEAST_ONE_NEW);
     }
 }
 
@@ -194,7 +229,5 @@ export function read<A, B, C, R>(a: Signal<A>, b: Signal<B>, c: Signal<C>, f: (a
 export function read<R>(...args: any[]): Signal<R> {
     let f = args[args.length - 1] as (...args: any[]) => R | SignalImpl<R>;
     let signals = args.slice(0, args.length - 1) as SignalImpl<any>[];
-    if (signals.length === 1) return signals[0].read(f);
-
     return new SignalImpl(makeCt(signals, f));
 }