refactor(core): run effects during change detection (#49641)

This commit updates the `effect` primitive and significantly changes the
timing of effect execution.

Previously, effects were scheduled via the microtask queue. This commit
changes effects to run throughout the change detection process instead.
Running effects this way avoids needing additional rounds of change
detection to resolve effects, with the tradeoff that they're harder to use
for model-to-model synchronization (which can be seen as a good thing).

PR Close #49641

packages/core/src/render3/component_ref.ts

@@ -42,6 +42,7 @@ import {CONTEXT, HEADER_OFFSET, INJECTOR, LView, LViewEnvironment, LViewFlags, T
 import {MATH_ML_NAMESPACE, SVG_NAMESPACE} from './namespaces';
 import {createElementNode, setupStaticAttributes, writeDirectClass} from './node_manipulation';
 import {extractAttrsAndClassesFromSelector, stringifyCSSSelectorList} from './node_selector_matcher';
+import {EffectManager} from './reactivity/effect';
 import {enterView, getCurrentTNode, getLView, leaveView} from './state';
 import {computeStaticStyling} from './styling/static_styling';
 import {mergeHostAttrs, setUpAttributes} from './util/attrs_utils';
@@ -165,9 +166,12 @@ export class ComponentFactory<T> extends AbstractComponentFactory<T> {
     }
     const sanitizer = rootViewInjector.get(Sanitizer, null);
 
+    const effectManager = rootViewInjector.get(EffectManager, null);
+
     const environment: LViewEnvironment = {
       rendererFactory,
       sanitizer,
+      effectManager,
     };
 
     const hostRenderer = rendererFactory.createRenderer(null, this.componentDef);

packages/core/src/render3/instructions/shared.ts

@@ -365,10 +365,14 @@ export function refreshView<T>(
   ngDevMode && assertEqual(isCreationMode(lView), false, 'Should be run in update mode');
   const flags = lView[FLAGS];
   if ((flags & LViewFlags.Destroyed) === LViewFlags.Destroyed) return;
-  enterView(lView);
+
   // Check no changes mode is a dev only mode used to verify that bindings have not changed
   // since they were assigned. We do not want to execute lifecycle hooks in that mode.
   const isInCheckNoChangesPass = ngDevMode && isInCheckNoChangesMode();
+
+  !isInCheckNoChangesPass && lView[ENVIRONMENT].effectManager?.flush();
+
+  enterView(lView);
   try {
     resetPreOrderHookFlags(lView);
 
@@ -1819,6 +1823,10 @@ export function detectChangesInternal<T>(
     throw error;
   } finally {
     if (!checkNoChangesMode && rendererFactory.end) rendererFactory.end();
+
+    // One final flush of the effects queue to catch any effects created in `ngAfterViewInit` or
+    // other post-order hooks.
+    !checkNoChangesMode && lView[ENVIRONMENT].effectManager?.flush();
   }
 }
 

packages/core/src/render3/interfaces/view.ts

@@ -12,6 +12,7 @@ import {DehydratedView} from '../../hydration/interfaces';
 import {SchemaMetadata} from '../../metadata/schema';
 import {Sanitizer} from '../../sanitization/sanitizer';
 import type {ReactiveLViewConsumer} from '../reactive_lview_consumer';
+import type {EffectManager} from '../reactivity/effect';
 
 import {LContainer} from './container';
 import {ComponentDef, ComponentTemplate, DirectiveDef, DirectiveDefList, HostBindingsFunction, PipeDef, PipeDefList, ViewQueriesFunction} from './definition';
@@ -367,6 +368,9 @@ export interface LViewEnvironment {
 
   /** An optional custom sanitizer. */
   sanitizer: Sanitizer|null;
+
+  /** Container for reactivity system `effect`s. */
+  effectManager: EffectManager|null;
 }
 
 /** Flags associated with an LView (saved in LView[FLAGS]) */

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

@@ -9,6 +9,7 @@
 import {assertInInjectionContext} from '../../di/contextual';
 import {Injector} from '../../di/injector';
 import {inject} from '../../di/injector_compatibility';
+import {ɵɵdefineInjectable} from '../../di/interface/defs';
 import {DestroyRef} from '../../linker/destroy_ref';
 import {Watch} from '../../signals';
 
@@ -21,10 +22,66 @@ import {Watch} from '../../signals';
  */
 export type EffectCleanupFn = () => void;
 
-const globalWatches = new Set<Watch>();
-const queuedWatches = new Map<Watch, Zone>();
+/**
+ * Tracks all effects registered within a given application and runs them via `flush`.
+ */
+export class EffectManager {
+  private all = new Set<Watch>();
+  private queue = new Map<Watch, Zone>();
+
+  create(effectFn: () => void, destroyRef: DestroyRef|null): EffectRef {
+    const zone = Zone.current;
+    const watch = new Watch(effectFn, (watch) => {
+      if (!this.all.has(watch)) {
+        return;
+      }
+
+      this.queue.set(watch, zone);
+    });
+
+    this.all.add(watch);
+
+    // Effects start dirty.
+    watch.notify();
+
+    let unregisterOnDestroy: (() => void)|undefined;
+
+    const destroy = () => {
+      watch.cleanup();
+      unregisterOnDestroy?.();
+      this.all.delete(watch);
+      this.queue.delete(watch);
+    };
+
+    unregisterOnDestroy = destroyRef?.onDestroy(destroy);
+
+    return {
+      destroy,
+    };
+  }
+
+  flush(): void {
+    if (this.queue.size === 0) {
+      return;
+    }
+
+    for (const [watch, zone] of this.queue) {
+      this.queue.delete(watch);
+      zone.run(() => watch.run());
+    }
+  }
 
-let watchQueuePromise: {promise: Promise<void>; resolveFn: () => void;}|null = null;
+  get isQueueEmpty(): boolean {
+    return this.queue.size === 0;
+  }
+
+  /** @nocollapse */
+  static ɵprov = /** @pureOrBreakMyCode */ ɵɵdefineInjectable({
+    token: EffectManager,
+    providedIn: 'root',
+    factory: () => new EffectManager(),
+  });
+}
 
 /**
  * A global reactive effect, which can be manually destroyed.
@@ -68,62 +125,8 @@ export interface CreateEffectOptions {
 export function effect(
     effectFn: () => EffectCleanupFn | void, options?: CreateEffectOptions): EffectRef {
   !options?.injector && assertInInjectionContext(effect);
-
-  const zone = Zone.current;
-  const watch = new Watch(effectFn, (watch) => queueWatch(watch, zone));
-
   const injector = options?.injector ?? inject(Injector);
+  const effectManager = injector.get(EffectManager);
   const destroyRef = options?.manualCleanup !== true ? injector.get(DestroyRef) : null;
-
-  globalWatches.add(watch);
-
-  // Effects start dirty.
-  watch.notify();
-
-  let unregisterOnDestroy: (() => void)|undefined;
-
-  const destroy = () => {
-    watch.cleanup();
-    unregisterOnDestroy?.();
-    queuedWatches.delete(watch);
-    globalWatches.delete(watch);
-  };
-
-  unregisterOnDestroy = destroyRef?.onDestroy(destroy);
-
-  return {
-    destroy,
-  };
-}
-
-function queueWatch(watch: Watch, zone: Zone): void {
-  if (queuedWatches.has(watch) || !globalWatches.has(watch)) {
-    return;
-  }
-
-  queuedWatches.set(watch, zone);
-
-  if (watchQueuePromise === null) {
-    Promise.resolve().then(runWatchQueue);
-
-    let resolveFn!: () => void;
-    const promise = new Promise<void>((resolve) => {
-      resolveFn = resolve;
-    });
-
-    watchQueuePromise = {
-      promise,
-      resolveFn,
-    };
-  }
-}
-
-function runWatchQueue(): void {
-  for (const [watch, zone] of queuedWatches) {
-    queuedWatches.delete(watch);
-    zone.run(() => watch.run());
-  }
-
-  watchQueuePromise!.resolveFn();
-  watchQueuePromise = null;
+  return effectManager.create(effectFn, destroyRef);
 }

packages/core/test/bundling/animations/bundle.golden_symbols.json

@@ -224,6 +224,9 @@
   {
     "name": "EVENT_MANAGER_PLUGINS"
   },
+  {
+    "name": "EffectManager"
+  },
   {
     "name": "ElementInstructionMap"
   },
@@ -365,6 +368,9 @@
   {
     "name": "NG_TOKEN_PATH"
   },
+  {
+    "name": "NOOP_CLEANUP_FN"
+  },
   {
     "name": "NOT_FOUND"
   },
@@ -602,6 +608,9 @@
   {
     "name": "ViewRef"
   },
+  {
+    "name": "Watch"
+  },
   {
     "name": "WeakRefImpl"
   },

packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json

@@ -131,6 +131,9 @@
   {
     "name": "EVENT_MANAGER_PLUGINS"
   },
+  {
+    "name": "EffectManager"
+  },
   {
     "name": "ElementRef"
   },
@@ -266,6 +269,9 @@
   {
     "name": "NG_TOKEN_PATH"
   },
+  {
+    "name": "NOOP_CLEANUP_FN"
+  },
   {
     "name": "NOT_FOUND"
   },
@@ -455,6 +461,9 @@
   {
     "name": "ViewRef"
   },
+  {
+    "name": "Watch"
+  },
   {
     "name": "WeakRefImpl"
   },

packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json

@@ -176,6 +176,9 @@
   {
     "name": "EVENT_MANAGER_PLUGINS"
   },
+  {
+    "name": "EffectManager"
+  },
   {
     "name": "ElementRef"
   },
@@ -356,6 +359,9 @@
   {
     "name": "NG_VALUE_ACCESSOR"
   },
+  {
+    "name": "NOOP_CLEANUP_FN"
+  },
   {
     "name": "NOT_FOUND"
   },
@@ -614,6 +620,9 @@
   {
     "name": "ViewRef"
   },
+  {
+    "name": "Watch"
+  },
   {
     "name": "WeakRefImpl"
   },

packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json

@@ -179,6 +179,9 @@
   {
     "name": "EVENT_MANAGER_PLUGINS"
   },
+  {
+    "name": "EffectManager"
+  },
   {
     "name": "ElementRef"
   },
@@ -338,6 +341,9 @@
   {
     "name": "NG_VALUE_ACCESSOR"
   },
+  {
+    "name": "NOOP_CLEANUP_FN"
+  },
   {
     "name": "NOT_FOUND"
   },
@@ -605,6 +611,9 @@
   {
     "name": "ViewRef"
   },
+  {
+    "name": "Watch"
+  },
   {
     "name": "WeakRefImpl"
   },

packages/core/test/bundling/hello_world/bundle.golden_symbols.json

@@ -80,6 +80,9 @@
   {
     "name": "ERROR_ORIGINAL_ERROR"
   },
+  {
+    "name": "EffectManager"
+  },
   {
     "name": "ElementRef"
   },
@@ -188,6 +191,9 @@
   {
     "name": "NG_TOKEN_PATH"
   },
+  {
+    "name": "NOOP_CLEANUP_FN"
+  },
   {
     "name": "NOT_FOUND"
   },
@@ -347,6 +353,9 @@
   {
     "name": "ViewRef"
   },
+  {
+    "name": "Watch"
+  },
   {
     "name": "WeakRefImpl"
   },

packages/core/test/bundling/router/bundle.golden_symbols.json

@@ -203,6 +203,9 @@
   {
     "name": "EVENT_MANAGER_PLUGINS"
   },
+  {
+    "name": "EffectManager"
+  },
   {
     "name": "ElementRef"
   },
@@ -404,6 +407,9 @@
   {
     "name": "NONE"
   },
+  {
+    "name": "NOOP_CLEANUP_FN"
+  },
   {
     "name": "NOT_FOUND"
   },
@@ -803,6 +809,9 @@
   {
     "name": "ViewRef"
   },
+  {
+    "name": "Watch"
+  },
   {
     "name": "WeakRefImpl"
   },