fixup! refactor(core): prototype of signals, a reactive primitive for…

… Angular

packages/core/src/signals/README.md

@@ -21,6 +21,24 @@ counter.set(2);
 counter.update(count => count + 1);
 ```
 
+The signal value can be also updated in-place, using the dedicated `.mutate` method:
+
+```typescript
+const todoList = signal<Todo[])([]);
+
+todoList.mutate(list => {
+    list.push({title: 'One more task', completed: false});
+});
+```
+
+#### Equality
+
+The signal creation function one can, optionally, specify an equality comparator function. The comparator is used to decide whether the new supplied value is the same, or different, as compared to the current signal’s value.
+
+If the equality function determines that 2 values are equal it will:
+* block update of signal’s value;
+* skip change propagation.
+
 ### Declarative derived values: `computed()`
 
 `computed()` creates a memoizing signal, which calculates its value from the values of some number of input signals.
@@ -34,6 +52,8 @@ const isEven = computed(() => counter() % 2 === 0);
 
 Because the calculation function used to create the `computed` is executed in a reactive context, any signals read by that calculation will be tracked as dependencies, and the value of the computed signal recalculated whenever any of those dependencies changes.
 
+Similarly to signals, the `computed` can (optionally) specify an equality comparator function. 
+
 ### Side effects: `effect()`
 
 `effect()` schedules and runs a side-effectful function inside a reactive context. Signal dependencies of this function are captured, and the side effect is re-executed whenever any of its dependencies produces a new value.
@@ -93,7 +113,7 @@ Crucially, during this first phase, no side effects are run, and no recomputatio
 
 Once this change propagation has completed (synchronously), the second phase can begin. In this second phase, signal values may be read by the application or framework, triggering recomputation of any needed derived values which were previously invalidated.
 
-We refer to this as the "push/pull" algorithm: "dirtyness" is eagerly _pushed_ through the graph when a source signal is changed, but recalculation is performed lazily, only when values are _pulled_ by reading their signals.
+We refer to this as the "push/pull" algorithm: "dirtiness" is eagerly _pushed_ through the graph when a source signal is changed, but recalculation is performed lazily, only when values are _pulled_ by reading their signals.
 
 ## Dynamic Dependency Tracking
 
@@ -139,4 +159,4 @@ The `Consumer` can then compare the `valueVersion` of the new value with the one
 
 ## `Watch` primitive
 
-`Watch` is a primitive used to build different types of effects. `Watch`es are `Consumer`s that run side-effectful functions in their reactive context, but where the scheduling of the side effect is delegated to the implementor. The `Watch` will call this scheduling operation when it receives a notification that it's stale.
+`Watch` is a primitive used to build different types of effects. `Watch`es are `Consumer`s that run side-effectful functions in their reactive context, but where the scheduling of the side effect is delegated to the implementor. The `Watch` will call this scheduling operation when it receives a notification that it's stale.

packages/core/src/signals/src/api.ts

@@ -54,14 +54,19 @@ export function markSignal<T, U extends {} = {}>(fn: () => T, extraApi: U = ({}
 export type ValueEqualityFn<T> = (a: T, b: T) => boolean;
 
 /**
- * The default equality function used for `signal` and `computed`, which treats objects
- * and arrays as never equal, and all other primitive values using identity semantics.
+ * The default equality function used for `signal` and `computed`, which treats objects and arrays
+ * as never equal, and all other primitive values using identity semantics.
+ *
+ * This allows signals to hold non-primitive values (arrays, objects, other collections) and still
+ * propagate change notification upon explicit mutation without identity change.
  *
  * @developerPreview
  */
 export function defaultEquals<T>(a: T, b: T) {
-  if (Object.is(a, b)) {
-    return a !== null && typeof a === 'object' ? false : true;
-  }
-  return false;
+  // Object.is compares two values using identity semantics which is desired behavior for primitive
+  // values. If Object.is determines two values to be equal we need to make sure that those don't
+  // represent objects (we want to make sure that 2 objects are always considered "unequal"). The
+  // null check is needed for the special case of JavaScript reporting null values as objects
+  // (typeof null === 'object').
+  return Object.is(a, b) && (a === null || typeof a !== 'object');
 }

packages/core/src/signals/src/computed.ts

@@ -23,18 +23,21 @@ export function computed<T>(
 
 /**
  * A dedicated symbol used before a computed value has been calculated for the first time.
+ * Explicitly typed as `any` so we can use it as signal's value.
  */
 const UNSET: any = Symbol('UNSET');
 
 /**
  * A dedicated symbol used in place of a computed signal value to indicate that a given computation
  * is in progress. Used to detect cycles in computation chains.
+ * Explicitly typed as `any` so we can use it as signal's value.
  */
 const COMPUTING: any = Symbol('COMPUTING');
 
 /**
  * A dedicated symbol used in place of a computed signal value to indicate that a given computation
  * failed. The thrown error is cached until the computation gets dirty again.
+ * Explicitly typed as `any` so we can use it as signal's value.
  */
 const ERRORED: any = Symbol('ERRORED');
 
@@ -44,25 +47,18 @@ const ERRORED: any = Symbol('ERRORED');
  * `Computed`s are both `Producer`s and `Consumer`s of reactivity.
  */
 class ComputedImpl<T> implements Producer, Consumer {
-  readonly id = nextReactiveId();
-  readonly ref = new WeakRef(this);
-  readonly producers = new Map<ProducerId, Edge>();
-  readonly consumers = new Map<ConsumerId, Edge>();
-  trackingVersion = 0;
-  valueVersion = 0;
-
   /**
    * Current value of the computation.
    *
    * This can also be one of the special values `UNSET`, `COMPUTING`, or `ERRORED`.
    */
-  value: T = UNSET;
+  private value: T = UNSET;
 
   /**
    * If `value` is `ERRORED`, the error caught from the last computation attempt which will
    * be re-thrown.
    */
-  error: unknown = null;
+  private error: unknown = null;
 
   /**
    * Flag indicating that the computation is currently stale, meaning that one of the
@@ -71,7 +67,14 @@ class ComputedImpl<T> implements Producer, Consumer {
    * It's possible that no dependency has _actually_ changed, in which case the `stale`
    * state can be resolved without recomputing the value.
    */
-  stale = true;
+  private stale = true;
+
+  readonly id = nextReactiveId();
+  readonly ref = new WeakRef(this);
+  readonly producers = new Map<ProducerId, Edge>();
+  readonly consumers = new Map<ConsumerId, Edge>();
+  trackingVersion = 0;
+  valueVersion = 0;
 
   constructor(private computation: () => T, private equal: (oldValue: T, newValue: T) => boolean) {}
 

packages/core/src/signals/src/internal.ts

@@ -13,10 +13,26 @@ export type ConsumerId = number&{__consumer: true};
 
 let _nextReactiveId: number = 0;
 
+/**
+ * Tracks the currently active reactive context (or `null` if there is no active
+ * context).
+ */
+let activeConsumer: Consumer|null = null;
+
 export function nextReactiveId(): ProducerId&ConsumerId {
   return (_nextReactiveId++ as ProducerId & ConsumerId);
 }
 
+/**
+ * Set `consumer` as the active reactive context, and return the previous `Consumer`
+ * (if any) for later restoration.
+ */
+export function setActiveConsumer(consumer: Consumer|null): Consumer|null {
+  const prevConsumer = activeConsumer;
+  activeConsumer = consumer;
+  return prevConsumer;
+}
+
 export interface Edge {
   readonly consumerRef: WeakRef<Consumer>;
   readonly producerRef: WeakRef<Producer>;
@@ -230,19 +246,3 @@ export function consumerPollValueStatus(consumer: Consumer): boolean {
   // impacted.
   return false;
 }
-
-/**
- * Tracks the currently active reactive context (or `null` if there is no active
- * context).
- */
-let activeConsumer: Consumer|null = null;
-
-/**
- * Set `consumer` as the active reactive context, and return the previous `Consumer`
- * (if any) for later restoration.
- */
-export function setActiveConsumer(consumer: Consumer|null): Consumer|null {
-  const prevConsumer = activeConsumer;
-  activeConsumer = consumer;
-  return prevConsumer;
-}

packages/core/src/signals/src/signal.ts

@@ -78,7 +78,7 @@ class SettableSignalImpl<T> implements Producer {
    * Calls `mutator` on the current value and assumes that it has been mutated.
    */
   mutate(mutator: (value: T) => void): void {
-    // Note: mutate bypasses equality checks as it's by definition changing the value.
+    // Mutate bypasses equality checks as it's by definition changing the value.
     mutator(this.value);
     this.valueVersion++;
     producerNotifyConsumers(this);
@@ -97,11 +97,11 @@ class SettableSignalImpl<T> implements Producer {
  */
 export function signal<T>(
     initialValue: T, equal: ValueEqualityFn<T> = defaultEquals): SettableSignal<T> {
-  const sig = new SettableSignalImpl(initialValue, equal);
-  const res = markSignal(sig.signal.bind(sig), {
-    set: sig.set.bind(sig),
-    update: sig.update.bind(sig),
-    mutate: sig.mutate.bind(sig),
+  const signalNode = new SettableSignalImpl(initialValue, equal);
+  const signalFn = markSignal(signalNode.signal.bind(signalNode), {
+    set: signalNode.set.bind(signalNode),
+    update: signalNode.update.bind(signalNode),
+    mutate: signalNode.mutate.bind(signalNode),
   });
-  return res;
+  return signalFn;
 }

packages/core/src/signals/src/untrack.ts

@@ -16,6 +16,8 @@ import {setActiveConsumer} from './internal';
  */
 export function untrack<T>(nonReactiveReadsFn: () => T): T {
   const prevConsumer = setActiveConsumer(null);
+  // We are not trying to catch any particular errors here, just making sure that the consumers
+  // stack is restored in case of errors.
   try {
     return nonReactiveReadsFn();
   } finally {

packages/core/test/signals/computed_spec.ts

@@ -6,7 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {computed, signal} from '@angular/core/src/signals';
+import {computed, signal, Watch} from '@angular/core/src/signals';
 
 describe('computed', () => {
   it('should create computed', () => {
@@ -130,4 +130,37 @@ describe('computed', () => {
     updateTracker();
     expect(updateCounter).toEqual(3);
   });
+
+  it('should not mark dirty computed signals that are dirty already', () => {
+    const source = signal('a');
+    const derived = computed(() => source().toUpperCase());
+
+    let watchCount = 0;
+    const watch = new Watch(
+        () => {
+          derived();
+        },
+        () => {
+          watchCount++;
+        });
+
+    watch.run();
+    expect(watchCount).toEqual(0);
+
+    // change signal, mark downstream dependencies dirty
+    source.set('b');
+    expect(watchCount).toEqual(1);
+
+    // change signal again, downstream dependencies should be dirty already and not marked again
+    source.set('c');
+    expect(watchCount).toEqual(1);
+
+    // resetting dependencies back to clean
+    watch.run();
+    expect(watchCount).toEqual(1);
+
+    // expecting another notification at this point
+    source.set('d');
+    expect(watchCount).toEqual(2);
+  });
 });

packages/core/test/signals/glitch_free_spec.ts

@@ -28,7 +28,6 @@ describe('glitch-free computations', () => {
     expect(fullRecompute).toEqual(2);
   });
 
-  // https://twitter.com/GrishechkinOleg/status/1572819756574511104
   it('should recompute only once', () => {
     const a = signal('a');
     const b = computed(() => a() + 'b');