Skip to content

Commit

Permalink
fix(core): afterRender hooks registered outside change detection can …
Browse files Browse the repository at this point in the history
…mark views dirty (#55623)

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
  • Loading branch information
atscott authored and AndrewKushnir committed May 2, 2024
1 parent 36130b2 commit d5edfde
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 4 deletions.
6 changes: 4 additions & 2 deletions packages/core/src/application/application_ref.ts
Expand Up @@ -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(
Expand Down
47 changes: 45 additions & 2 deletions packages/core/test/acceptance/after_render_hook_spec.ts
Expand Up @@ -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, {
Expand Down Expand Up @@ -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,
Expand All @@ -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');
});

Expand Down

0 comments on commit d5edfde

Please sign in to comment.