fix(core): drop mutate function from the signals public API (#51821) (#…

…51986)

This change removes the `mutate` method from the `WritableSignal` interface and
completely drops it from the public API surface.

The initial API proposal for Angular signals included the mutate method, allowing
in-place modification of JS objects, without changing their references (identity).
This was based on the reasoning that identity change on modification is not necessary
as we can send the “modified” notification through the signals graph.
Unfortunately the signal-specific change notification is lost as soon as we read
signal value outside of a reactive context (outside of a reactive graph).
In other words - any code outside of the Angular signals library can’t know
that an object is modified.

Secondly, to make the mutate method work, we’ve defaulted the signal value equality function
to the one that considers non-primitive values as always different.
This is unfortunate for people working with immutable data structures
(this is notably the case for the popular state management libraries)
as the default equality function de-optimizes memoization in computed,
making the application less performant.

Given the above reasons we prefer to remove the mutate method in the signals library -
at least for now. There are just too many sharp edges and tradeoffs that we don’t fully
understand yet.

BREAKING CHANGE:

The  `mutate` method was removed from the `WritableSignal` interface and completely
dropped from the public API surface. As an alternative please use the update method and
make immutable changes to the object.

Example before:

```typescript
items.mutate(itemsArray => itemsArray.push(newItem));
```

Example after:

```typescript
items.update(itemsArray => [itemsArray, …newItem]);
```

PR Close #51986

goldens/public-api/core/index.md

@@ -1630,7 +1630,6 @@ export abstract class ViewRef extends ChangeDetectorRef {
 // @public
 export interface WritableSignal<T> extends Signal<T> {
     asReadonly(): Signal<T>;
-    mutate(mutatorFn: (value: T) => void): void;
     set(value: T): void;
     update(updateFn: (value: T) => T): void;
 }

packages/core/primitives/signals/src/equality.ts

@@ -12,17 +12,8 @@
 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.
- *
- * This allows signals to hold non-primitive values (arrays, objects, other collections) and still
- * propagate change notification upon explicit mutation without identity change.
+ * The default equality function used for `signal` and `computed`, which uses referential equality.
  */
 export function defaultEquals<T>(a: T, b: T) {
-  // `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 (a === null || typeof a !== 'object') && Object.is(a, b);
+  return Object.is(a, b);
 }

packages/core/src/render3/reactivity/signal.ts

@@ -25,12 +25,6 @@ export interface WritableSignal<T> extends Signal<T> {
    */
   update(updateFn: (value: T) => T): void;
 
-  /**
-   * Update the current value by mutating it in-place, and
-   * notify any dependents.
-   */
-  mutate(mutatorFn: (value: T) => void): void;
-
   /**
    * Returns a readonly version of this signal. Readonly signals can be accessed to read their value
    * but can't be changed using set, update or mutate methods. The readonly signals do _not_ have
@@ -61,7 +55,6 @@ export function signal<T>(initialValue: T, options?: CreateSignalOptions<T>): Wr
 
   signalFn.set = (newValue: T) => signalSetFn(node, newValue);
   signalFn.update = (updateFn: (value: T) => T) => signalUpdateFn(node, updateFn);
-  signalFn.mutate = (mutateFn: (value: T) => void) => signalMutateFn(node, mutateFn);
   signalFn.asReadonly = signalAsReadonlyFn.bind(signalFn as any) as () => Signal<T>;
 
   return signalFn as WritableSignal<T>;

packages/core/test/signals/signal_spec.ts

@@ -25,18 +25,6 @@ describe('signals', () => {
     expect(counter()).toEqual(1);
   });
 
-  it('should have mutate function for mutable, out of bound updates', () => {
-    const state = signal<string[]>(['a']);
-    const derived = computed(() => state().join(':'));
-
-    expect(derived()).toEqual('a');
-
-    state.mutate((s) => {
-      s.push('b');
-    });
-    expect(derived()).toEqual('a:b');
-  });
-
   it('should not update signal when new value is equal to the previous one', () => {
     const state = signal('aaa', {equal: (a, b) => a.length === b.length});
     expect(state()).toEqual('aaa');
@@ -73,31 +61,32 @@ describe('signals', () => {
     expect(upper()).toEqual('D');
   });
 
-  it('should consider objects as non-equal with the default equality function', () => {
-    let stateValue: unknown = {};
-    const state = signal(stateValue);
-    let computeCount = 0;
-    const derived = computed(() => `${typeof state()}:${++computeCount}`);
-    expect(derived()).toEqual('object:1');
-
-    // reset signal value to the same object instance, expect change notification
-    state.set(stateValue);
-    expect(derived()).toEqual('object:2');
-
-    // reset signal value to a different object instance, expect change notification
-    stateValue = {};
-    state.set(stateValue);
-    expect(derived()).toEqual('object:3');
-
-    // reset signal value to a different object type, expect change notification
-    stateValue = [];
-    state.set(stateValue);
-    expect(derived()).toEqual('object:4');
-
-    // reset signal value to the same array instance, expect change notification
-    state.set(stateValue);
-    expect(derived()).toEqual('object:5');
-  });
+  it('should consider objects as equal based on their identity with the default equality function',
+     () => {
+       let stateValue: unknown = {};
+       const state = signal(stateValue);
+       let computeCount = 0;
+       const derived = computed(() => `${typeof state()}:${++computeCount}`);
+       expect(derived()).toEqual('object:1');
+
+       // reset signal value to the same object instance, expect NO change notification
+       state.set(stateValue);
+       expect(derived()).toEqual('object:1');
+
+       // reset signal value to a different object instance, expect change notification
+       stateValue = {};
+       state.set(stateValue);
+       expect(derived()).toEqual('object:2');
+
+       // reset signal value to a different object type, expect change notification
+       stateValue = [];
+       state.set(stateValue);
+       expect(derived()).toEqual('object:3');
+
+       // reset signal value to the same array instance, expect NO change notification
+       state.set(stateValue);
+       expect(derived()).toEqual('object:3');
+     });
 
   it('should allow converting writable signals to their readonly counterpart', () => {
     const counter = signal(0);
@@ -107,8 +96,6 @@ describe('signals', () => {
     expect(readOnlyCounter.set).toBeUndefined();
     // @ts-expect-error
     expect(readOnlyCounter.update).toBeUndefined();
-    // @ts-expect-error
-    expect(readOnlyCounter.mutate).toBeUndefined();
 
     const double = computed(() => readOnlyCounter() * 2);
     expect(double()).toBe(0);
@@ -143,13 +130,6 @@ describe('signals', () => {
       expect(log).toBe(1);
     });
 
-    it('should call the post-signal-set fn when invoking .mutate()', () => {
-      const counter = signal(0);
-
-      counter.mutate(() => {});
-      expect(log).toBe(1);
-    });
-
     it('should not call the post-signal-set fn when the value doesn\'t change', () => {
       const counter = signal(0);