Skip to content

Commit

Permalink
fix(core): afterRender hooks now only run on ApplicationRef.tick
Browse files Browse the repository at this point in the history
The `afterRender` hooks currently run after `ApplicationRef.tick` but
also run after any call to `ChangeDetectorRef.detectChanges`. This is
problematic because code which uses `afterRender` cannot expect the
component it's registered from to be rendered when the callback
executes. If there is a call to `ChangeDetectorRef.detectChanges` before
the global change detection, that will cause the hooks to run earlier
than expected.

This behavior is somewhat of a blocker for the zoneless project. There
is plenty of application code that do things like `setTimeout(() =>
doSomethingThatExpectsComponentToBeRendered())`, `NgZone.onStable(() =>
...)` or `ApplicationRef.onStable...`. `ApplicationRef.onStable` is a
should likely work similarly, but all of these are really wanting an API
that is `afterRender` with the requirement that the hook runs after the
global render, not an individual CDRef instance.

This change updates the `afterRender` hooks to only run when
`ApplicationRef.tick` happens.

fixes #53232
  • Loading branch information
atscott committed Dec 13, 2023
1 parent b660e05 commit 97dd35f
Show file tree
Hide file tree
Showing 16 changed files with 74 additions and 115 deletions.
9 changes: 3 additions & 6 deletions packages/core/src/application/application_ref.ts
Expand Up @@ -29,6 +29,7 @@ import {ComponentFactoryResolver} from '../linker/component_factory_resolver';
import {NgModuleFactory, NgModuleRef} from '../linker/ng_module_factory';
import {ViewRef} from '../linker/view_ref';
import {isComponentResourceResolutionQueueEmpty, resolveComponentResources} from '../metadata/resource_loading';
import {AfterRenderEventManager} from '../render3/after_render_hooks';
import {assertNgModuleType} from '../render3/assert';
import {ComponentFactory as R3ComponentFactory} from '../render3/component_ref';
import {isStandalone} from '../render3/definition';
Expand All @@ -41,9 +42,6 @@ import {isPromise} from '../util/lang';
import {NgZone, ZONE_IS_STABLE_OBSERVABLE} from '../zone/ng_zone';

import {ApplicationInitStatus} from './application_init';
import { AfterRenderEventManager } from '../render3/after_render_hooks';
import { checkNoChangesInternal, detectChangesInternal } from '../render3/instructions/change_detection';
import { FLAGS, LViewFlags } from '../render3/interfaces/view';

/**
* A [DI token](guide/glossary#di-token "DI token definition") that provides a set of callbacks to
Expand Down Expand Up @@ -559,12 +557,11 @@ export class ApplicationRef {
this.afterRenderEffectManager?.begin();

for (let view of this._views) {
view._lView[FLAGS] |= LViewFlags.RefreshView;
detectChangesInternal(view._lView, view.notifyErrorHandler);
view.detectChanges();
}
if (typeof ngDevMode === 'undefined' || ngDevMode) {
for (let view of this._views) {
checkNoChangesInternal(view._lView, view.notifyErrorHandler);
view.checkNoChanges();
}
}
} catch (e) {
Expand Down
11 changes: 0 additions & 11 deletions packages/core/src/render3/instructions/change_detection.ts
Expand Up @@ -92,17 +92,6 @@ export function checkNoChangesInternal(lView: LView, notifyErrorHandler = true)
}


export function detectChangesAndRunAfterRenderHooks(lView: LView, notifyErrorHandler = true) {
const environment = lView[ENVIRONMENT];
const afterRenderEventManager = environment.afterRenderEventManager;
afterRenderEventManager?.begin();
try {
detectChangesInternal(lView, notifyErrorHandler);
} finally {
afterRenderEventManager?.end();
}
}

/**
* Different modes of traversing the logical view tree during change detection.
*
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/render3/util/change_detection_utils.ts
Expand Up @@ -8,7 +8,7 @@

import {assertDefined} from '../../util/assert';
import {getComponentViewByInstance} from '../context_discovery';
import {detectChangesAndRunAfterRenderHooks, detectChangesInternal} from '../instructions/change_detection';
import {detectChangesInternal} from '../instructions/change_detection';
import {markViewDirty} from '../instructions/mark_view_dirty';
import {FLAGS, LViewFlags} from '../interfaces/view';

Expand Down Expand Up @@ -39,5 +39,5 @@ export function applyChanges(component: {}): void {
function detectChanges(component: {}): void {
const view = getComponentViewByInstance(component);
view[FLAGS] |= LViewFlags.RefreshView;
detectChangesAndRunAfterRenderHooks(view);
detectChangesInternal(view);
}
6 changes: 3 additions & 3 deletions packages/core/src/render3/view_ref.ts
Expand Up @@ -13,7 +13,7 @@ import {removeFromArray} from '../util/array_utils';
import {assertEqual} from '../util/assert';

import {collectNativeNodes} from './collect_native_nodes';
import {checkNoChangesInternal, detectChangesAndRunAfterRenderHooks} from './instructions/change_detection';
import {checkNoChangesInternal, detectChangesInternal} from './instructions/change_detection';
import {markViewDirty} from './instructions/mark_view_dirty';
import {CONTAINER_HEADER_OFFSET, VIEW_REFS} from './interfaces/container';
import {isLContainer} from './interfaces/type_checks';
Expand Down Expand Up @@ -57,7 +57,7 @@ export class ViewRef<T> implements EmbeddedViewRef<T>, ChangeDetectorRefInterfac
*
* This may be different from `_lView` if the `_cdRefInjectingView` is an embedded view.
*/
private _cdRefInjectingView?: LView, readonly notifyErrorHandler = true) {}
private _cdRefInjectingView?: LView, private readonly notifyErrorHandler = true) {}

get context(): T {
return this._lView[CONTEXT] as unknown as T;
Expand Down Expand Up @@ -290,7 +290,7 @@ export class ViewRef<T> implements EmbeddedViewRef<T>, ChangeDetectorRefInterfac
// until the end of the refresh. Using `RefreshView` prevents creating a potential difference
// in the state of the LViewFlags during template execution.
this._lView[FLAGS] |= LViewFlags.RefreshView;
detectChangesAndRunAfterRenderHooks(this._lView, this.notifyErrorHandler);
detectChangesInternal(this._lView, this.notifyErrorHandler);
}

/**
Expand Down

0 comments on commit 97dd35f

Please sign in to comment.