refactor(core): model() implements OutputRef (#54650)

A model signal is technically an output, at runtime and conceptually.

This commit re-uses the shared output ref logic and ensures the
interfaces match.

PR Close #54650

goldens/public-api/core/index.md

@@ -1112,15 +1112,13 @@ export interface ModelOptions {
 }
 
 // @public
-export interface ModelSignal<T> extends WritableSignal<T> {
+export interface ModelSignal<T> extends WritableSignal<T>, OutputRef<T> {
     // (undocumented)
     [ɵINPUT_SIGNAL_BRAND_READ_TYPE]: T;
     // (undocumented)
     [ɵINPUT_SIGNAL_BRAND_WRITE_TYPE]: T;
     // (undocumented)
     [SIGNAL]: InputSignalNode<T, T>;
-    // @deprecated (undocumented)
-    subscribe(callback: (value: T) => void): () => void;
 }
 
 // @public @deprecated
@@ -1817,7 +1815,7 @@ export abstract class ViewRef extends ChangeDetectorRef {
 // @public
 export interface WritableSignal<T> extends Signal<T> {
     // (undocumented)
-    [WRITABLE_SIGNAL]: T;
+    [ɵWRITABLE_SIGNAL]: T;
     asReadonly(): Signal<T>;
     set(value: T): void;
     update(updateFn: (value: T) => T): void;

packages/core/src/authoring/model/model_signal.ts

@@ -10,9 +10,11 @@ import {producerAccessed, SIGNAL, signalSetFn} from '@angular/core/primitives/si
 
 import {RuntimeError, RuntimeErrorCode} from '../../errors';
 import {Signal} from '../../render3/reactivity/api';
-import {WritableSignal} from '../../render3/reactivity/signal';
+import {WritableSignal, ɵWRITABLE_SIGNAL} from '../../render3/reactivity/signal';
 import {ɵINPUT_SIGNAL_BRAND_READ_TYPE, ɵINPUT_SIGNAL_BRAND_WRITE_TYPE} from '../input/input_signal';
 import {INPUT_SIGNAL_NODE, InputSignalNode, REQUIRED_UNSET_VALUE} from '../input/input_signal_node';
+import {OutputEmitterRef} from '../output/output_emitter_ref';
+import {OutputRef} from '../output/output_ref';
 
 /**
  * @developerPreview
@@ -35,14 +37,10 @@ export interface ModelOptions {
  *
  * @developerPreview
  */
-export interface ModelSignal<T> extends WritableSignal<T> {
+export interface ModelSignal<T> extends WritableSignal<T>, OutputRef<T> {
   [SIGNAL]: InputSignalNode<T, T>;
   [ɵINPUT_SIGNAL_BRAND_READ_TYPE]: T;
   [ɵINPUT_SIGNAL_BRAND_WRITE_TYPE]: T;
-
-  // TODO(crisbeto): either make this a public API or mark as internal pending discussion.
-  /** @deprecated Do not use, will be removed. */
-  subscribe(callback: (value: T) => void): () => void;
 }
 
 /**
@@ -53,8 +51,8 @@ export interface ModelSignal<T> extends WritableSignal<T> {
  * @param options Additional options for the model.
  */
 export function createModelSignal<T>(initialValue: T): ModelSignal<T> {
-  const subscriptions: ((value: T) => void)[] = [];
   const node: InputSignalNode<T, T> = Object.create(INPUT_SIGNAL_NODE);
+  const emitterRef = new OutputEmitterRef<T>();
 
   node.value = initialValue;
 
@@ -64,19 +62,14 @@ export function createModelSignal<T>(initialValue: T): ModelSignal<T> {
     return node.value;
   }
 
-  function notifySubscribers(value: T): void {
-    for (let i = 0; i < subscriptions.length; i++) {
-      subscriptions[i](value);
-    }
-  }
-
-  (getter as any)[SIGNAL] = node;
-  (getter as any).asReadonly = (() => getter()) as Signal<T>;
+  getter[SIGNAL] = node;
+  getter.asReadonly = (() => getter()) as () => Signal<T>;
 
+  // TODO: Should we throw an error when updating a destroyed model?
   getter.set = (newValue: T) => {
     if (!node.equal(node.value, newValue)) {
       signalSetFn(node, newValue);
-      notifySubscribers(newValue);
+      emitterRef.emit(newValue);
     }
   };
 
@@ -85,23 +78,17 @@ export function createModelSignal<T>(initialValue: T): ModelSignal<T> {
     getter.set(updateFn(node.value));
   };
 
-  getter.subscribe = (callback: (value: T) => void) => {
-    subscriptions.push(callback);
-
-    return () => {
-      const index = subscriptions.indexOf(callback);
-
-      if (index > -1) {
-        subscriptions.splice(index, 1);
-      }
-    };
-  };
+  getter.subscribe = emitterRef.subscribe.bind(emitterRef);
+  getter.destroyRef = emitterRef.destroyRef;
 
   if (ngDevMode) {
     getter.toString = () => `[Model Signal: ${getter()}]`;
   }
 
-  return getter as ModelSignal<T>;
+  return getter as (typeof getter&Pick<
+                    ModelSignal<T>,
+                    typeof ɵINPUT_SIGNAL_BRAND_READ_TYPE|typeof ɵINPUT_SIGNAL_BRAND_WRITE_TYPE|
+                    typeof ɵWRITABLE_SIGNAL>);
 }
 
 /** Asserts that a model's value is set. */

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

@@ -13,13 +13,13 @@ import {performanceMarkFeature} from '../../util/performance';
 import {isSignal, Signal, ValueEqualityFn} from './api';
 
 /** Symbol used distinguish `WritableSignal` from other non-writable signals and functions. */
-const WRITABLE_SIGNAL = /* @__PURE__ */ Symbol('WRITABLE_SIGNAL');
+export const ɵWRITABLE_SIGNAL = /* @__PURE__ */ Symbol('WRITABLE_SIGNAL');
 
 /**
  * A `Signal` with a value that can be mutated via a setter interface.
  */
 export interface WritableSignal<T> extends Signal<T> {
-  [WRITABLE_SIGNAL]: T;
+  [ɵWRITABLE_SIGNAL]: T;
 
   /**
    * Directly set the signal to a new value, and notify any dependents.
@@ -44,7 +44,7 @@ export interface WritableSignal<T> extends Signal<T> {
  * Utility function used during template type checking to extract the value from a `WritableSignal`.
  * @codeGenApi
  */
-export function ɵunwrapWritableSignal<T>(value: T|{[WRITABLE_SIGNAL]: T}): T {
+export function ɵunwrapWritableSignal<T>(value: T|{[ɵWRITABLE_SIGNAL]: T}): T {
   // Note: the function uses `WRITABLE_SIGNAL` as a brand instead of `WritableSignal<T>`,
   // because the latter incorrectly unwraps non-signal getter functions.
   return null!;

packages/core/test/acceptance/authoring/model_inputs_spec.ts

@@ -432,7 +432,7 @@ describe('model inputs', () => {
     expect(emittedEvents).toBe(1);
 
     fixture.destroy();
-    modelRef.set(2);
+    expect(() => modelRef.set(2)).toThrowError(/Unexpected emit for destroyed `OutputRef`/);
     expect(emittedEvents).toBe(1);
   });
 

packages/core/test/authoring/model_input_spec.ts

@@ -6,21 +6,12 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {computed} from '@angular/core';
-import {model} from '@angular/core/src/authoring/model/model';
-import {ModelSignal} from '@angular/core/src/authoring/model/model_signal';
-
-/**
- * Utility type representing a signal that can be subscribed to. This is already captured
- * in `ModelSignal`, but it's marked as internal which makes it unavailable in tests.
- */
-type SubscribableSignal<T> = ModelSignal<T>&{
-  subscribe(callback: (value: T) => void): () => void;
-};
+import {computed, model} from '@angular/core';
+import {TestBed} from '@angular/core/testing';
 
 describe('model signal', () => {
   it('should work with computed expressions', () => {
-    const signal = model(0);
+    const signal = TestBed.runInInjectionContext(() => model(0));
     let computedCount = 0;
     const derived = computed(() => (computedCount++, signal() + 1000));
 
@@ -35,7 +26,7 @@ describe('model signal', () => {
   });
 
   it('should allow updates based on the previous value', () => {
-    const signal = model(2);
+    const signal = TestBed.runInInjectionContext(() => model(2));
 
     signal.update(value => value * 3);
     expect(signal()).toBe(6);
@@ -45,7 +36,7 @@ describe('model signal', () => {
   });
 
   it('should emit when the value changes', () => {
-    const signal = model(1) as SubscribableSignal<number>;
+    const signal = TestBed.runInInjectionContext(() => model(1));
     const values: number[] = [];
 
     signal.subscribe(value => values.push(value));
@@ -60,21 +51,33 @@ describe('model signal', () => {
     expect(values).toEqual([2, 4, 5]);
   });
 
+  it('should error when subscribing to a destroyed model', () => {
+    const signal = TestBed.runInInjectionContext(() => model(1));
+    const values: number[] = [];
+
+    signal.subscribe(value => values.push(value));
+
+    TestBed.resetTestingModule();
+
+    expect(() => signal.subscribe(() => {}))
+        .toThrowError(/Unexpected subscription to destroyed `OutputRef`/);
+  });
+
   it('should stop emitting after unsubscribing', () => {
-    const signal = model(0) as SubscribableSignal<number>;
+    const signal = TestBed.runInInjectionContext(() => model(0));
     const values: number[] = [];
     const subscription = signal.subscribe(value => values.push(value));
 
     signal.set(1);
     expect(values).toEqual([1]);
 
-    subscription();
+    subscription.unsubscribe();
     signal.set(2);
     expect(values).toEqual([1]);
   });
 
   it('should not emit if the value does not change', () => {
-    const signal = model(0) as SubscribableSignal<number>;
+    const signal = TestBed.runInInjectionContext(() => model(0));
     const values: number[] = [];
     signal.subscribe(value => values.push(value));
 
@@ -91,8 +94,8 @@ describe('model signal', () => {
 
   it('should not share subscriptions between models', () => {
     let emitCount = 0;
-    const signal1 = model(0) as SubscribableSignal<number>;
-    const signal2 = model(0) as SubscribableSignal<number>;
+    const signal1 = TestBed.runInInjectionContext(() => model(0));
+    const signal2 = TestBed.runInInjectionContext(() => model(0));
     const callback = () => emitCount++;
     const subscription1 = signal1.subscribe(callback);
     const subscription2 = signal2.subscribe(callback);
@@ -103,17 +106,17 @@ describe('model signal', () => {
     signal2.set(1);
     expect(emitCount).toBe(2);
 
-    subscription1();
+    subscription1.unsubscribe();
     signal2.set(2);
     expect(emitCount).toBe(3);
 
-    subscription2();
+    subscription2.unsubscribe();
     signal2.set(3);
     expect(emitCount).toBe(3);
   });
 
   it('should throw if there is no value for required model', () => {
-    const signal = model.required();
+    const signal = TestBed.runInInjectionContext(() => model.required());
 
     expect(() => signal()).toThrowError(/Model is required but no value is available yet\./);
 
@@ -122,7 +125,7 @@ describe('model signal', () => {
   });
 
   it('should throw if a `computed` depends on an uninitialized required model', () => {
-    const signal = model.required<number>();
+    const signal = TestBed.runInInjectionContext(() => model.required<number>());
     const expr = computed(() => signal() + 1000);
 
     expect(() => expr()).toThrowError(/Model is required but no value is available yet\./);
@@ -132,7 +135,7 @@ describe('model signal', () => {
   });
 
   it('should have a toString implementation', () => {
-    const signal = model(0);
+    const signal = TestBed.runInInjectionContext(() => model(0));
     expect(signal + '').toBe('[Model Signal: 0]');
   });
 });