From 8fa627c9e4ca58f41e50a1ce9950177b555f18cc Mon Sep 17 00:00:00 2001 From: Andrew Scott Date: Wed, 1 May 2024 15:08:05 -0700 Subject: [PATCH] fix(core): afterRender hooks registered outside change detection can mark views dirty This commit fixes an error in the looping logic of `ApplicationRef.tick` when the tick skips straight to render hooks. In this case, if a render hook makes a state update that requires a view refresh, we would never actually refresh the view and just loop until we hit the loop limit. --- .../core/src/application/application_ref.ts | 6 ++- .../test/acceptance/after_render_hook_spec.ts | 46 ++++++++++++++++++- 2 files changed, 48 insertions(+), 4 deletions(-) diff --git a/packages/core/src/application/application_ref.ts b/packages/core/src/application/application_ref.ts index 5a1cfe65e25555..4bd6491310a008 100644 --- a/packages/core/src/application/application_ref.ts +++ b/packages/core/src/application/application_ref.ts @@ -576,8 +576,10 @@ export class ApplicationRef { let runs = 0; const afterRenderEffectManager = this.afterRenderEffectManager; while (runs < MAXIMUM_REFRESH_RERUNS) { - if (refreshViews) { - const isFirstPass = runs === 0; + const isFirstPass = runs === 0; + // Some notifications to run a `tick` will only trigger render hooks. so we skip refreshing views the first time through. + // After the we execute render hooks in the first pass, we loop while views are marked dirty and should refresh them. + if (refreshViews || !isFirstPass) { this.beforeRender.next(isFirstPass); for (let {_lView, notifyErrorHandler} of this._views) { detectChangesInViewIfRequired( diff --git a/packages/core/test/acceptance/after_render_hook_spec.ts b/packages/core/test/acceptance/after_render_hook_spec.ts index 68dcc5037851e8..946c9c803ec218 100644 --- a/packages/core/test/acceptance/after_render_hook_spec.ts +++ b/packages/core/test/acceptance/after_render_hook_spec.ts @@ -37,6 +37,8 @@ import {withBody} from '@angular/private/testing'; import {destroyPlatform} from '../../src/core'; import {EnvironmentInjector} from '../../src/di'; +import {firstValueFrom} from 'rxjs'; +import {filter} from 'rxjs/operators'; function createAndAttachComponent(component: Type) { const componentRef = createComponent(component, { @@ -1097,7 +1099,7 @@ describe('after render hooks', () => { expect(fixture.nativeElement.innerText).toBe('1'); }); - it('allows updating state and calling markForCheck in afterRender', () => { + it('allows updating state and calling markForCheck in afterRender', async () => { @Component({ selector: 'test-component', standalone: true, @@ -1124,7 +1126,47 @@ describe('after render hooks', () => { const fixture = TestBed.createComponent(TestCmp); const appRef = TestBed.inject(ApplicationRef); appRef.attachView(fixture.componentRef.hostView); - appRef.tick(); + await firstValueFrom(appRef.isStable.pipe(filter((stable) => stable))); + expect(fixture.nativeElement.innerText).toBe('1'); + }); + + fit('allows updating state and calling markForCheck in afterRender, outside of change detection', async () => { + const counter = signal(0); + @Component({ + selector: 'test-component', + standalone: true, + template: `{{counter()}}`, + }) + class TestCmp { + injector = inject(EnvironmentInjector); + async ngOnInit() { + // push the render hook to a time outside of change detection + await new Promise((resolve) => setTimeout(resolve)); + afterNextRender( + () => { + counter.set(1); + }, + {injector: this.injector}, + ); + } + } + TestBed.configureTestingModule({ + providers: [{provide: PLATFORM_ID, useValue: PLATFORM_BROWSER_ID}], + }); + + const fixture = TestBed.createComponent(TestCmp); + const appRef = TestBed.inject(ApplicationRef); + appRef.attachView(fixture.componentRef.hostView); + await new Promise((resolve) => { + TestBed.runInInjectionContext(() => { + effect(() => { + if (counter() === 1) { + resolve(); + } + }); + }); + }); + await firstValueFrom(appRef.isStable.pipe(filter((stable) => stable))); expect(fixture.nativeElement.innerText).toBe('1'); });