diff --git a/packages/core/src/render3/instructions/change_detection.ts b/packages/core/src/render3/instructions/change_detection.ts index f5f2ba5ca77..ccdbd4a15ab 100644 --- a/packages/core/src/render3/instructions/change_detection.ts +++ b/packages/core/src/render3/instructions/change_detection.ts @@ -13,7 +13,7 @@ import {getComponentViewByInstance} from '../context_discovery'; import {executeCheckHooks, executeInitAndCheckHooks, incrementInitPhaseFlags} from '../hooks'; import {CONTAINER_HEADER_OFFSET, HAS_CHILD_VIEWS_TO_REFRESH, HAS_TRANSPLANTED_VIEWS, LContainer, MOVED_VIEWS} from '../interfaces/container'; import {ComponentTemplate, RenderFlags} from '../interfaces/definition'; -import {CONTEXT, ENVIRONMENT, FLAGS, InitPhaseState, LView, LViewFlags, PARENT, REACTIVE_TEMPLATE_CONSUMER, TVIEW, TView, TViewType} from '../interfaces/view'; +import {CONTEXT, EFFECTS_TO_SCHEDULE, ENVIRONMENT, FLAGS, InitPhaseState, LView, LViewFlags, PARENT, REACTIVE_TEMPLATE_CONSUMER, TVIEW, TView, TViewType} from '../interfaces/view'; import {enterView, isInCheckNoChangesMode, leaveView, setBindingIndex, setIsInCheckNoChangesMode} from '../state'; import {getFirstLContainer, getNextLContainer} from '../util/view_traversal_utils'; import {getComponentLViewByIndex, isCreationMode, markAncestorsForTraversal, markViewForRefresh, resetPreOrderHookFlags, viewAttachedToChangeDetector} from '../util/view_utils'; @@ -250,6 +250,16 @@ export function refreshView( tView.firstUpdatePass = false; } + // Schedule any effects that are waiting on the update pass of this view. + if (lView[EFFECTS_TO_SCHEDULE]) { + for (const notifyEffect of lView[EFFECTS_TO_SCHEDULE]) { + notifyEffect(); + } + + // Once they've been run, we can drop the array. + lView[EFFECTS_TO_SCHEDULE] = null; + } + // Do not reset the dirty state when running in check no changes mode. We don't want components // to behave differently depending on whether check no changes is enabled or not. For example: // Marking an OnPush component as dirty from within the `ngAfterViewInit` hook in order to diff --git a/packages/core/src/render3/interfaces/view.ts b/packages/core/src/render3/interfaces/view.ts index 1c68aa6074c..dc6048ba7d0 100644 --- a/packages/core/src/render3/interfaces/view.ts +++ b/packages/core/src/render3/interfaces/view.ts @@ -57,6 +57,7 @@ export const QUERIES = 18; export const ID = 19; export const EMBEDDED_VIEW_INJECTOR = 20; export const ON_DESTROY_HOOKS = 21; +export const EFFECTS_TO_SCHEDULE = 22; export const REACTIVE_TEMPLATE_CONSUMER = 23; export const REACTIVE_HOST_BINDING_CONSUMER = 24; @@ -336,6 +337,11 @@ export interface LView extends Array { */ readonly[EMBEDDED_VIEW_INJECTOR]: Injector|null; + /** + * Effect scheduling operations that need to run during this views's update pass. + */ + [EFFECTS_TO_SCHEDULE]: Array<() => void>|null; + /** * A collection of callbacks functions that are executed when a given LView is destroyed. Those * are user defined, LView-specific destroy callbacks that don't have any corresponding TView diff --git a/packages/core/src/render3/reactivity/effect.ts b/packages/core/src/render3/reactivity/effect.ts index 17bd61396f8..991d8800a9e 100644 --- a/packages/core/src/render3/reactivity/effect.ts +++ b/packages/core/src/render3/reactivity/effect.ts @@ -8,13 +8,16 @@ import {createWatch, Watch, WatchCleanupRegisterFn} from '@angular/core/primitives/signals'; +import {ChangeDetectorRef} from '../../change_detection'; import {assertInInjectionContext} from '../../di/contextual'; import {InjectionToken} from '../../di/injection_token'; import {Injector} from '../../di/injector'; import {inject} from '../../di/injector_compatibility'; import {ɵɵdefineInjectable} from '../../di/interface/defs'; import {ErrorHandler} from '../../error_handler'; +import type {InternalViewRef} from '../view_ref'; import {DestroyRef} from '../../linker/destroy_ref'; +import {FLAGS, LViewFlags, EFFECTS_TO_SCHEDULE} from '../interfaces/view'; import {assertNotInReactiveContext} from './asserts'; @@ -170,7 +173,7 @@ export class ZoneAwareMicrotaskScheduler implements EffectScheduler { */ class EffectHandle implements EffectRef, SchedulableEffect { unregisterOnDestroy: (() => void)|undefined; - protected watcher: Watch; + readonly watcher: Watch; constructor( private scheduler: EffectScheduler, @@ -198,10 +201,6 @@ class EffectHandle implements EffectRef, SchedulableEffect { this.scheduler.scheduleEffect(this); } - notify(): void { - this.watcher.notify(); - } - destroy(): void { this.watcher.destroy(); this.unregisterOnDestroy?.(); @@ -272,8 +271,25 @@ export function effect( (typeof Zone === 'undefined') ? null : Zone.current, destroyRef, errorHandler, options?.allowSignalWrites ?? false); - // Effects start dirty. - handle.notify(); + // Effects need to be marked dirty manually to trigger their initial run. The timing of this + // marking matters, because the effects may read signals that track component inputs, which are + // only available after those components have had their first update pass. + // + // We inject `ChangeDetectorRef` optionally, to determine whether this effect is being created in + // the context of a component or not. If it is, then we check whether the component has already + // run its update pass, and defer the effect's initial scheduling until the update pass if it + // hasn't already run. + const cdr = + injector.get(ChangeDetectorRef, null, {optional: true}) as InternalViewRef| null; + if (!cdr || !(cdr._lView[FLAGS] & LViewFlags.FirstLViewPass)) { + // This effect is either not running in a view injector, or the view has already + // undergone its first change detection pass, which is necessary for any required inputs to be + // set. + handle.watcher.notify(); + } else { + // Delay the initialization of the effect until the view is fully initialized. + (cdr._lView[EFFECTS_TO_SCHEDULE] ??= []).push(handle.watcher.notify); + } return handle; } diff --git a/packages/core/test/acceptance/after_render_hook_spec.ts b/packages/core/test/acceptance/after_render_hook_spec.ts index bf8971c7a21..2815d4e07f3 100644 --- a/packages/core/test/acceptance/after_render_hook_spec.ts +++ b/packages/core/test/acceptance/after_render_hook_spec.ts @@ -489,9 +489,9 @@ describe('after render hooks', () => { }, ] }); - TestBed.createComponent(TestCmp); + const fixture = TestBed.createComponent(TestCmp); - expect(() => TestBed.flushEffects()) + expect(() => fixture.detectChanges()) .toThrowError(/afterRender\(\) cannot be called from within a reactive context/); }); }); diff --git a/packages/core/test/render3/reactivity_spec.ts b/packages/core/test/render3/reactivity_spec.ts index d62a781b631..b90174ae5db 100644 --- a/packages/core/test/render3/reactivity_spec.ts +++ b/packages/core/test/render3/reactivity_spec.ts @@ -7,7 +7,7 @@ */ import {AsyncPipe} from '@angular/common'; -import {AfterViewInit, Component, computed, ContentChildren, createComponent, createEnvironmentInjector, destroyPlatform, effect, EnvironmentInjector, ErrorHandler, inject, Injector, Input, NgZone, OnChanges, QueryList, signal, SimpleChanges, ViewChild} from '@angular/core'; +import {AfterViewInit, Component, computed, ContentChildren, createComponent, createEnvironmentInjector, destroyPlatform, effect, EnvironmentInjector, ErrorHandler, inject, Injectable, Injector, Input, NgZone, OnChanges, QueryList, signal, SimpleChanges, ViewChild, ViewContainerRef} from '@angular/core'; import {toObservable} from '@angular/core/rxjs-interop'; import {TestBed} from '@angular/core/testing'; import {bootstrapApplication} from '@angular/platform-browser'; @@ -124,11 +124,6 @@ describe('effects', () => { const fixture = TestBed.createComponent(Cmp); fixture.detectChanges(); - // Effects don't run during change detection. - expect(didRun).toBeFalse(); - - TestBed.flushEffects(); - expect(didRun).toBeTrue(); }); @@ -349,6 +344,174 @@ describe('effects', () => { expect(fixture.nativeElement.textContent).toBe('0'); }); + describe('effects created in components should first run after ngOnInit', () => { + it('when created during bootstrapping', () => { + let log: string[] = []; + @Component({ + standalone: true, + selector: 'test-cmp', + template: '', + }) + class TestCmp { + constructor() { + effect(() => log.push('effect')); + } + + ngOnInit(): void { + log.push('init'); + } + } + + const fixture = TestBed.createComponent(TestCmp); + TestBed.flushEffects(); + expect(log).toEqual([]); + fixture.detectChanges(); + expect(log).toEqual(['init', 'effect']); + }); + + it('when created during change detection', () => { + let log: string[] = []; + + @Component({ + standalone: true, + selector: 'test-cmp', + template: '', + }) + class TestCmp { + ngOnInitRan = false; + constructor() { + effect(() => log.push('effect')); + } + + ngOnInit(): void { + log.push('init'); + } + } + + @Component({ + standalone: true, + selector: 'driver-cmp', + imports: [TestCmp], + template: ` + @if (cond) { + + } + `, + }) + class DriverCmp { + cond = false; + } + + const fixture = TestBed.createComponent(DriverCmp); + fixture.detectChanges(); + expect(log).toEqual([]); + + // Toggle the @if, which should create and run the effect. + fixture.componentInstance.cond = true; + fixture.detectChanges(); + expect(log).toEqual(['init', 'effect']); + }); + + it('when created dynamically', () => { + let log: string[] = []; + @Component({ + standalone: true, + selector: 'test-cmp', + template: '', + }) + class TestCmp { + ngOnInitRan = false; + constructor() { + effect(() => log.push('effect')); + } + + ngOnInit(): void { + log.push('init'); + } + } + + @Component({ + standalone: true, + selector: 'driver-cmp', + template: '', + }) + class DriverCmp { + vcr = inject(ViewContainerRef); + } + + const fixture = TestBed.createComponent(DriverCmp); + fixture.detectChanges(); + + fixture.componentInstance.vcr.createComponent(TestCmp); + + // Verify that simply creating the component didn't schedule the effect. + TestBed.flushEffects(); + expect(log).toEqual([]); + + // Running change detection should schedule and run the effect. + fixture.detectChanges(); + expect(log).toEqual(['init', 'effect']); + }); + + it('when created in a service provided in a component', () => { + let log: string[] = []; + + @Injectable() + class EffectService { + constructor() { + effect(() => log.push('effect')); + } + } + + @Component({ + standalone: true, + selector: 'test-cmp', + template: '', + providers: [EffectService], + }) + class TestCmp { + svc = inject(EffectService); + + ngOnInit(): void { + log.push('init'); + } + } + + const fixture = TestBed.createComponent(TestCmp); + TestBed.flushEffects(); + expect(log).toEqual([]); + fixture.detectChanges(); + expect(log).toEqual(['init', 'effect']); + }); + + it('if multiple effects are created', () => { + let log: string[] = []; + @Component({ + standalone: true, + selector: 'test-cmp', + template: '', + }) + class TestCmp { + constructor() { + effect(() => log.push('effect a')); + effect(() => log.push('effect b')); + effect(() => log.push('effect c')); + } + + ngOnInit(): void { + log.push('init'); + } + } + + const fixture = TestBed.createComponent(TestCmp); + fixture.detectChanges(); + expect(log[0]).toBe('init'); + expect(log).toContain('effect a'); + expect(log).toContain('effect b'); + expect(log).toContain('effect c'); + }); + }); + describe('should disallow creating an effect context', () => { it('inside template effect', () => { @Component({ @@ -400,9 +563,9 @@ describe('effects', () => { }, ] }); - TestBed.createComponent(Cmp); + const fixture = TestBed.createComponent(Cmp); - expect(() => TestBed.flushEffects()) + expect(() => fixture.detectChanges()) .toThrowError(/effect\(\) cannot be called from within a reactive context./); }); }); diff --git a/packages/core/test/test_bed_effect_spec.ts b/packages/core/test/test_bed_effect_spec.ts index efb79909af9..a304fbdc643 100644 --- a/packages/core/test/test_bed_effect_spec.ts +++ b/packages/core/test/test_bed_effect_spec.ts @@ -34,13 +34,19 @@ describe('effects in TestBed', () => { TestBed.createComponent(Cmp).detectChanges(); expect(log).toEqual([ + // The component gets constructed, which creates the effect. Since the effect is created in a + // component, it doesn't get scheduled until the component is first change detected. 'Ctor', - 'Effect', + + // Next, the first change detection (update pass) happens. 'DoCheck', + + // Then the effect runs. + 'Effect', ]); }); - it('created in ngOnInit should not run with detectChanges()', () => { + it('created in ngOnInit should run with detectChanges()', () => { const log: string[] = []; @Component({ selector: 'test-cmp', @@ -68,12 +74,15 @@ describe('effects in TestBed', () => { TestBed.createComponent(Cmp).detectChanges(); expect(log).toEqual([ - // B: component bootstrapped + // The component gets constructed. 'Ctor', - // ngDoCheck runs before ngOnInit - 'DoCheck', - ]); - // effect should not have executed. + // Next, the first change detection (update pass) happens, which creates the effect and + // schedules it for execution. + 'DoCheck', + + // Then the effect runs. + 'Effect', + ]); }); }); diff --git a/packages/core/testing/src/component_fixture.ts b/packages/core/testing/src/component_fixture.ts index 50c94e94b2f..3579b8be41c 100644 --- a/packages/core/testing/src/component_fixture.ts +++ b/packages/core/testing/src/component_fixture.ts @@ -135,6 +135,9 @@ export class ComponentFixture { // Running without zone. Just do the change detection. this._tick(checkNoChanges); } + // Run any effects that were created/dirtied during change detection. Such effects might become + // dirty in response to input signals changing. + this.effectRunner?.flush(); } /**