refactor(core): move effect to render3 and add DestroyRef integration (#49529)

The `effect` implemented in the signal library is useful for testing but
does not integrate with Angular. This commit moves that code to the
actual framework package and integrates it with automatic cleanup via
`DestroyRef`. A simpler effect implementation is used in the signal tests to
test the `Watch` primitive.

Further commits will update the scheduling to tie effects together with
change detection.

PR Close #49529

goldens/public-api/core/index.md

@@ -334,6 +334,12 @@ export interface CreateComputedOptions<T> {
     equal?: ValueEqualityFn<T>;
 }
 
+// @public
+export interface CreateEffectOptions {
+    injector?: Injector;
+    manualCleanup?: boolean;
+}
+
 // @public
 export function createEnvironmentInjector(providers: Array<Provider | EnvironmentProviders>, parent: EnvironmentInjector, debugName?: string | null): EnvironmentInjector;
 
@@ -504,7 +510,7 @@ export interface DoCheck {
 }
 
 // @public
-export function effect(effectFn: () => void): EffectRef;
+export function effect(effectFn: () => void, options?: CreateEffectOptions): EffectRef;
 
 // @public
 export interface EffectRef {

packages/core/src/core_reactivity_export_internal.ts

@@ -11,8 +11,6 @@ export {
   computed,
   CreateComputedOptions,
   CreateSignalOptions,
-  effect,
-  EffectRef,
   isSignal,
   Signal,
   signal,
@@ -20,4 +18,9 @@ export {
   ValueEqualityFn,
   WritableSignal,
 } from './signals';
-// clang-format on
+export {
+  CreateEffectOptions,
+  effect,
+  EffectRef,
+} from './render3/reactivity/effect';
+// clang-format on    

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

@@ -6,7 +6,16 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {Watch} from './watch';
+import {assertInInjectionContext} from '../../di/contextual';
+import {Injector} from '../../di/injector';
+import {inject} from '../../di/injector_compatibility';
+import {DestroyRef} from '../../linker/destroy_ref';
+import {Watch} from '../../signals';
+
+const globalWatches = new Set<Watch>();
+const queuedWatches = new Set<Watch>();
+
+let watchQueuePromise: {promise: Promise<void>; resolveFn: () => void;}|null = null;
 
 /**
  * A global reactive effect, which can be manually destroyed.
@@ -20,46 +29,60 @@ export interface EffectRef {
   destroy(): void;
 }
 
+/**
+ * Options passed to the `effect` function.
+ *
+ * @developerPreview
+ */
+export interface CreateEffectOptions {
+  /**
+   * The `Injector` in which to create the effect.
+   *
+   * If this is not provided, the current injection context will be used instead (via `inject`).
+   */
+  injector?: Injector;
+
+  /**
+   * Whether the `effect` should require manual cleanup.
+   *
+   * If this is `false` (the default) the effect will automatically register itself to be cleaned up
+   * with the current `DestroyRef`.
+   */
+  manualCleanup?: boolean;
+}
+
 /**
  * Create a global `Effect` for the given reactive function.
  *
  * @developerPreview
  */
-export function effect(effectFn: () => void): EffectRef {
+export function effect(effectFn: () => void, options?: CreateEffectOptions): EffectRef {
+  !options?.injector && assertInInjectionContext(effect);
+
+  const injector = options?.injector ?? inject(Injector);
+  const destroyRef = options?.manualCleanup !== true ? injector.get(DestroyRef) : null;
+
   const watch = new Watch(effectFn, queueWatch);
   globalWatches.add(watch);
 
   // Effects start dirty.
   watch.notify();
 
+  let unregisterOnDestroy: (() => void)|undefined;
+
+  const destroy = () => {
+    unregisterOnDestroy?.();
+    queuedWatches.delete(watch);
+    globalWatches.delete(watch);
+  };
+
+  unregisterOnDestroy = destroyRef?.onDestroy(destroy);
+
   return {
-    destroy: () => {
-      queuedWatches.delete(watch);
-      globalWatches.delete(watch);
-    },
+    destroy,
   };
 }
 
-/**
- * Get a `Promise` that resolves when any scheduled effects have resolved.
- */
-export function effectsDone(): Promise<void> {
-  return watchQueuePromise?.promise ?? Promise.resolve();
-}
-
-/**
- * Shut down all active effects.
- */
-export function resetEffects(): void {
-  queuedWatches.clear();
-  globalWatches.clear();
-}
-
-const globalWatches = new Set<Watch>();
-const queuedWatches = new Set<Watch>();
-
-let watchQueuePromise: {promise: Promise<void>; resolveFn: () => void;}|null = null;
-
 function queueWatch(watch: Watch): void {
   if (queuedWatches.has(watch) || !globalWatches.has(watch)) {
     return;

packages/core/src/signals/index.ts

@@ -8,7 +8,6 @@
 
 export {isSignal, Signal, ValueEqualityFn} from './src/api';
 export {computed, CreateComputedOptions} from './src/computed';
-export {effect, EffectRef} from './src/effect';
 export {setActiveConsumer} from './src/graph';
 export {CreateSignalOptions, signal, WritableSignal} from './src/signal';
 export {untracked} from './src/untracked';

packages/core/test/render3/reactivity_spec.ts

@@ -0,0 +1,48 @@
+/**
+ * @license
+ * Copyright Google LLC All Rights Reserved.
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.io/license
+ */
+
+import {Component, effect, ViewChild, ViewContainerRef} from '@angular/core';
+import {bootstrapApplication} from '@angular/platform-browser';
+import {withBody} from '@angular/private/testing';
+
+describe('effects', () => {
+  it('should run prior to change detection', withBody('<test-cmp></test-cmp>', async () => {
+       const log: string[] = [];
+       @Component({
+         selector: 'test-cmp',
+         standalone: true,
+         template: '',
+       })
+       class Cmp {
+         constructor() {
+           log.push('B');
+
+           effect(() => {
+             log.push('E');
+           });
+         }
+
+         ngDoCheck() {
+           log.push('C');
+         }
+       }
+
+       await bootstrapApplication(Cmp);
+
+       expect(log).toEqual([
+         // B: component bootstrapped
+         'B',
+         // C: change detection runs -> triggers ngDoCheck
+         'C',
+         // E: effect runs
+         'E',
+         // C: change detection runs after effect runs
+         'C',
+       ]);
+     }));
+});

packages/core/test/signals/effect_util.ts

@@ -0,0 +1,32 @@
+/**
+ * @license
+ * Copyright Google LLC All Rights Reserved.
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.io/license
+ */
+
+import {Watch} from '@angular/core/src/signals';
+
+let queue = new Set<Watch>();
+
+/**
+ * A wrapper around `Watch` that emulates the `effect` API and allows for more streamlined testing.
+ */
+export function testingEffect(effectFn: () => void): void {
+  const watch = new Watch(effectFn, queue.add.bind(queue));
+
+  // Effects start dirty.
+  watch.notify();
+}
+
+export function flushEffects(): void {
+  for (const watch of queue) {
+    queue.delete(watch);
+    watch.run();
+  }
+}
+
+export function resetEffects(): void {
+  queue.clear();
+}

packages/core/test/signals/non_reactive_spec.ts

@@ -7,7 +7,8 @@
  */
 
 import {computed, signal, untracked} from '@angular/core/src/signals';
-import {effect, effectsDone as flush, resetEffects} from '@angular/core/src/signals/src/effect';
+
+import {flushEffects, resetEffects, testingEffect} from './effect_util';
 
 describe('non-reactive reads', () => {
   afterEach(() => {
@@ -43,74 +44,62 @@ describe('non-reactive reads', () => {
     expect(untracked(double)).toEqual(4);
   });
 
-  it('should not make surrounding effect depend on the signal', async () => {
+  it('should not make surrounding effect depend on the signal', () => {
     const s = signal(1);
 
     const runLog: number[] = [];
-    effect(() => {
+    testingEffect(() => {
       runLog.push(untracked(s));
     });
 
     // an effect will run at least once
-    await flush();
+    flushEffects();
     expect(runLog).toEqual([1]);
 
     // subsequent signal changes should not trigger effects as signal is untracked
     s.set(2);
-    await flush();
+    flushEffects();
     expect(runLog).toEqual([1]);
   });
 
-  it('should schedule effects on dependencies (computed) change', async () => {
+  it('should schedule on dependencies (computed) change', () => {
     const count = signal(0);
     const double = computed(() => count() * 2);
 
     let runLog: number[] = [];
-    const effectRef = effect(() => {
+    testingEffect(() => {
       runLog.push(double());
     });
 
-    await flush();
+    flushEffects();
     expect(runLog).toEqual([0]);
 
     count.set(1);
-    await flush();
-    expect(runLog).toEqual([0, 2]);
-
-    effectRef.destroy();
-    count.set(2);
-    await flush();
+    flushEffects();
     expect(runLog).toEqual([0, 2]);
   });
 
-  it('should non-reactively read all signals accessed inside untrack', async () => {
+  it('should non-reactively read all signals accessed inside untrack', () => {
     const first = signal('John');
     const last = signal('Doe');
 
     let runLog: string[] = [];
-    const effectRef = effect(() => {
+    const effectRef = testingEffect(() => {
       untracked(() => runLog.push(`${first()} ${last()}`));
     });
 
     // effects run at least once
-    await flush();
+    flushEffects();
     expect(runLog).toEqual(['John Doe']);
 
     // change one of the signals - should not update as not read reactively
     first.set('Patricia');
-    await flush();
+    flushEffects();
     expect(runLog).toEqual(['John Doe']);
 
     // change one of the signals - should not update as not read reactively
     last.set('Garcia');
-    await flush();
-    expect(runLog).toEqual(['John Doe']);
-
-    // destroy effect, should not respond to changes
-    effectRef.destroy();
-    first.set('Robert');
-    last.set('Smith');
-    await flush();
+    flushEffects();
     expect(runLog).toEqual(['John Doe']);
   });
 });

packages/core/test/signals/watch_spec.ts

@@ -7,49 +7,41 @@
  */
 
 import {computed, signal} from '@angular/core/src/signals';
-import {effect, effectsDone as flush, resetEffects} from '@angular/core/src/signals/src/effect';
 
-describe('effects', () => {
+import {flushEffects, resetEffects, testingEffect} from './effect_util';
+
+describe('watchers', () => {
   afterEach(() => {
     resetEffects();
   });
 
-  it('should create and run once effect without dependencies', async () => {
+  it('should create and run once, even without dependencies', () => {
     let runs = 0;
 
-    const effectRef = effect(() => {
+    testingEffect(() => {
       runs++;
     });
 
-    await flush();
-    expect(runs).toEqual(1);
-
-    effectRef.destroy();
-    await flush();
+    flushEffects();
     expect(runs).toEqual(1);
   });
 
-  it('should schedule effects on dependencies (signal) change', async () => {
+  it('should schedule on dependencies (signal) change', () => {
     const count = signal(0);
     let runLog: number[] = [];
-    const effectRef = effect(() => {
+    const effectRef = testingEffect(() => {
       runLog.push(count());
     });
 
-    await flush();
+    flushEffects();
     expect(runLog).toEqual([0]);
 
     count.set(1);
-    await flush();
-    expect(runLog).toEqual([0, 1]);
-
-    effectRef.destroy();
-    count.set(2);
-    await flush();
+    flushEffects();
     expect(runLog).toEqual([0, 1]);
   });
 
-  it('should not schedule when a previous dependency changes', async () => {
+  it('should not schedule when a previous dependency changes', () => {
     const increment = (value: number) => value + 1;
     const countA = signal(0);
     const countB = signal(100);
@@ -57,54 +49,54 @@ describe('effects', () => {
 
 
     const runLog: number[] = [];
-    effect(() => {
+    testingEffect(() => {
       runLog.push(useCountA() ? countA() : countB());
     });
 
-    await flush();
+    flushEffects();
     expect(runLog).toEqual([0]);
 
     countB.update(increment);
-    await flush();
+    flushEffects();
     // No update expected: updated the wrong signal.
     expect(runLog).toEqual([0]);
 
     countA.update(increment);
-    await flush();
+    flushEffects();
     expect(runLog).toEqual([0, 1]);
 
     useCountA.set(false);
-    await flush();
+    flushEffects();
     expect(runLog).toEqual([0, 1, 101]);
 
     countA.update(increment);
-    await flush();
+    flushEffects();
     // No update expected: updated the wrong signal.
     expect(runLog).toEqual([0, 1, 101]);
   });
 
-  it('should not update dependencies of effects when dependencies don\'t change', async () => {
+  it('should not update dependencies when dependencies don\'t change', () => {
     const source = signal(0);
     const isEven = computed(() => source() % 2 === 0);
     let updateCounter = 0;
-    effect(() => {
+    testingEffect(() => {
       isEven();
       updateCounter++;
     });
 
-    await flush();
+    flushEffects();
     expect(updateCounter).toEqual(1);
 
     source.set(1);
-    await flush();
+    flushEffects();
     expect(updateCounter).toEqual(2);
 
     source.set(3);
-    await flush();
+    flushEffects();
     expect(updateCounter).toEqual(2);
 
     source.set(4);
-    await flush();
+    flushEffects();
     expect(updateCounter).toEqual(3);
   });
 });