fix(core): afterRender hooks registered outside change detection can mark views dirty (#55623)

atscott · AndrewKushnir · commit d5edfde6ee3d · 2024-05-02T10:58:29.000-07:00
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. PR Close #55623
diff --git 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
@@ -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<T>(component: Type<T>) {
   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,48 @@ 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');
+    });
+
+    it('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);
+        counter = counter;
+        async ngOnInit() {
+          // push the render hook to a time outside of change detection
+          await new Promise<void>((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<void>((resolve) => {
+        TestBed.runInInjectionContext(() => {
+          effect(() => {
+            if (counter() === 1) {
+              resolve();
+            }
+          });
+        });
+      });
+      await firstValueFrom(appRef.isStable.pipe(filter((stable) => stable)));
       expect(fixture.nativeElement.innerText).toBe('1');
     });
 
