skills/angular__angular
1
0
Fork 0
mirror of https://github.com/angular/angular.git synced 2026-09-14 13:54:52 +08:00
Code Issues Packages Projects Releases Wiki Activity

refactor(core): run internal work outside of public afterRender phases (#52145)

Browse Source 
Public afterRender phases have specific API guarantees which can be invalidated if the internal framework is implemented using them. Instead, the framework should use dedicated internal functions.

PR Close #52145
This commit is contained in:
Gerald Monaco
2023-10-10 17:52:40 +00:00
committed by Pawel Kozlowski
parent 450f360bd7
commit 6ae68f39b9
4 changed files with 150 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/core/src/core_render3_private_export.ts
+1 -1
View File
@@ -313,7 +313,7 @@ export {
export {
noSideEffects as ɵnoSideEffects,
} from './util/closure';
export { AfterRenderEventManager as ɵAfterRenderEventManager } from './render3/after_render_hooks';
export { AfterRenderEventManager as ɵAfterRenderEventManager, internalAfterNextRender as ɵinternalAfterNextRender } from './render3/after_render_hooks';
export {depsTracker as ɵdepsTracker, USE_RUNTIME_DEPS_TRACKER_FOR_JIT as ɵUSE_RUNTIME_DEPS_TRACKER_FOR_JIT} from './render3/deps_tracker/deps_tracker';
export {generateStandaloneInDeclarationsError as ɵgenerateStandaloneInDeclarationsError} from './render3/jit/module';
export {getAsyncClassMetadata as ɵgetAsyncClassMetadata} from './render3/metadata';
packages/core/src/defer/dom_triggers.ts
+27 -25
View File
@@ -7,11 +7,12 @@
*/
import type {Injector} from '../di';
import {afterRender} from '../render3/after_render_hooks';
import {internalAfterNextRender} from '../render3/after_render_hooks';
import {assertLContainer, assertLView} from '../render3/assert';
import {CONTAINER_HEADER_OFFSET} from '../render3/interfaces/container';
import {TNode} from '../render3/interfaces/node';
import {FLAGS, HEADER_OFFSET, INJECTOR, LView, LViewFlags} from '../render3/interfaces/view';
import {isDestroyed} from '../render3/interfaces/type_checks';
import {HEADER_OFFSET, INJECTOR, LView} from '../render3/interfaces/view';
import {getNativeByIndex, removeLViewOnDestroy, storeLViewOnDestroy, walkUpViews} from '../render3/util/view_utils';
import {assertElement, assertEqual} from '../util/assert';
import {NgZone} from '../zone';
@@ -83,13 +84,12 @@ export function onInteraction(
entry = new DeferEventEntry();
interactionTriggers.set(trigger, entry);
// Ensure that the handler runs in the NgZone since it gets
// registered in `afterRender` which runs outside.
injector.get(NgZone).run(() => {
for (const name of interactionEventNames) {
trigger.addEventListener(name, entry!.listener, eventListenerOptions);
}
});
// Ensure that the handler runs in the NgZone
ngDevMode && NgZone.assertInAngularZone();
for (const name of interactionEventNames) {
trigger.addEventListener(name, entry!.listener, eventListenerOptions);
}
}
entry.callbacks.add(callback);
@@ -122,13 +122,13 @@ export function onHover(
if (!entry) {
entry = new DeferEventEntry();
hoverTriggers.set(trigger, entry);
// Ensure that the handler runs in the NgZone since it gets
// registered in `afterRender` which runs outside.
injector.get(NgZone).run(() => {
for (const name of hoverEventNames) {
trigger.addEventListener(name, entry!.listener, eventListenerOptions);
}
});
// Ensure that the handler runs in the NgZone
ngDevMode && NgZone.assertInAngularZone();
for (const name of hoverEventNames) {
trigger.addEventListener(name, entry!.listener, eventListenerOptions);
}
}
entry.callbacks.add(callback);
@@ -262,31 +262,31 @@ export function registerDomTrigger(
registerFn: (element: Element, callback: VoidFunction, injector: Injector) => VoidFunction,
callback: VoidFunction) {
const injector = initialLView[INJECTOR]!;
function pollDomTrigger() {
// If the initial view was destroyed, we don't need to do anything.
if (isDestroyed(initialLView)) {
return;
}
// Assumption: the `afterRender` reference should be destroyed
// automatically so we don't need to keep track of it.
const afterRenderRef = afterRender(() => {
const lDetails = getLDeferBlockDetails(initialLView, tNode);
const renderedState = lDetails[DEFER_BLOCK_STATE];
// If the block was loaded before the trigger was resolved, we don't need to do anything.
if (renderedState !== DeferBlockInternalState.Initial &&
renderedState !== DeferBlockState.Placeholder) {
afterRenderRef.destroy();
return;
}
const triggerLView = getTriggerLView(initialLView, tNode, walkUpTimes);
// Keep polling until we resolve the trigger's LView.
// `afterRender` should stop automatically if the view is destroyed.
if (!triggerLView) {
internalAfterNextRender(pollDomTrigger, {injector});
return;
}
// It's possible that the trigger's view was destroyed before we resolved the trigger element.
if (triggerLView[FLAGS] & LViewFlags.Destroyed) {
afterRenderRef.destroy();
if (isDestroyed(triggerLView)) {
return;
}
@@ -301,7 +301,6 @@ export function registerDomTrigger(
cleanup();
}, injector);
afterRenderRef.destroy();
storeLViewOnDestroy(triggerLView, cleanup);
// Since the trigger and deferred block might be in different
@@ -309,5 +308,8 @@ export function registerDomTrigger(
if (initialLView !== triggerLView) {
storeLViewOnDestroy(initialLView, cleanup);
}
}, {injector});
}
// Begin polling for the trigger.
internalAfterNextRender(pollDomTrigger, {injector});
}
packages/core/src/render3/after_render_hooks.ts
+45
View File
@@ -118,6 +118,40 @@ export interface AfterRenderRef {
destroy(): void;
}
/**
* Options passed to `internalAfterNextRender`.
*/
export interface InternalAfterNextRenderOptions {
/**
* The `Injector` to use during creation.
*
* If this is not provided, the current injection context will be used instead (via `inject`).
*/
injector?: Injector;
}
/**
* Register a callback to run once before any userspace `afterRender` or
* `afterNextRender` callbacks.
*
* This function should almost always be used instead of `afterRender` or
* `afterNextRender` for implementing framework functionality. Consider:
*
* 1.) `AfterRenderPhase.EarlyRead` is intended to be used for implementing
* custom layout. If the framework itself mutates the DOM after *any*
* `AfterRenderPhase.EarlyRead` callbacks are run, the phase can no
* longer reliably serve its purpose.
*
* 2.) Importing `afterRender` in the framework can reduce the ability for it
* to be tree-shaken, and the framework shouldn't need much of the behavior.
*/
export function internalAfterNextRender(
callback: VoidFunction, options?: InternalAfterNextRenderOptions) {
const injector = options?.injector ?? inject(Injector);
const afterRenderEventManager = injector.get(AfterRenderEventManager);
afterRenderEventManager.internalCallbacks.push(callback);
}
/**
* Register a callback to be invoked each time the application
* finishes rendering.
@@ -398,6 +432,9 @@ export class AfterRenderEventManager {
/* @internal */
handler: AfterRenderCallbackHandler|null = null;
/* @internal */
internalCallbacks: VoidFunction[] = [];
/**
* Mark the beginning of a render operation (i.e. CD cycle).
* Throws if called while executing callbacks.
@@ -416,6 +453,13 @@ export class AfterRenderEventManager {
this.renderDepth--;
if (this.renderDepth === 0) {
// Note: internal callbacks power `internalAfterNextRender`. Since internal callbacks
// are fairly trivial, they are kept separate so that `AfterRenderCallbackHandlerImpl`
// can still be tree-shaken unless used by the application.
for (const callback of this.internalCallbacks) {
callback();
}
this.internalCallbacks.length = 0;
this.handler?.execute();
}
}
@@ -423,6 +467,7 @@ export class AfterRenderEventManager {
ngOnDestroy() {
this.handler?.destroy();
this.handler = null;
this.internalCallbacks.length = 0;
}
/** @nocollapse */
packages/core/test/acceptance/after_render_hook_spec.ts
+77 -1
View File
@@ -7,7 +7,7 @@
*/
import {PLATFORM_BROWSER_ID, PLATFORM_SERVER_ID} from '@angular/common/src/platform_id';
import {afterNextRender, afterRender, AfterRenderPhase, AfterRenderRef, ChangeDetectorRef, Component, computed, effect, ErrorHandler, inject, Injector, NgZone, PLATFORM_ID, ViewContainerRef} from '@angular/core';
import {afterNextRender, afterRender, AfterRenderPhase, AfterRenderRef, ChangeDetectorRef, Component, computed, effect, ErrorHandler, inject, Injector, NgZone, PLATFORM_ID, ViewContainerRef, ɵinternalAfterNextRender as internalAfterNextRender} from '@angular/core';
import {TestBed} from '@angular/core/testing';
describe('after render hooks', () => {
@@ -16,6 +16,82 @@ describe('after render hooks', () => {
providers: [{provide: PLATFORM_ID, useValue: PLATFORM_BROWSER_ID}]
};
describe('internalAfterNextRender', () => {
it('should run with the expected timing', () => {
const log: string[] = [];
@Component({selector: 'comp'})
class Comp {
constructor() {
// Helper to register into each phase
function forEachPhase(fn: (phase: AfterRenderPhase) => void) {
for (const phase in AfterRenderPhase) {
const val = AfterRenderPhase[phase];
if (typeof val === 'number') {
fn(val);
}
}
}
internalAfterNextRender(() => {
log.push('internalAfterNextRender #1');
});
forEachPhase(phase => afterRender(() => {
log.push(`afterRender (${AfterRenderPhase[phase]})`);
}, {phase}));
internalAfterNextRender(() => {
log.push('internalAfterNextRender #2');
});
forEachPhase(phase => afterNextRender(() => {
log.push(`afterNextRender (${AfterRenderPhase[phase]})`);
}, {phase}));
internalAfterNextRender(() => {
log.push('internalAfterNextRender #3');
});
}
}
TestBed.configureTestingModule({
declarations: [Comp],
...COMMON_CONFIGURATION,
});
const fixture = TestBed.createComponent(Comp);
// It hasn't run at all
expect(log).toEqual([]);
// Running change detection once
fixture.detectChanges();
expect(log).toEqual([
'internalAfterNextRender #1',
'internalAfterNextRender #2',
'internalAfterNextRender #3',
'afterRender (EarlyRead)',
'afterNextRender (EarlyRead)',
'afterRender (Write)',
'afterNextRender (Write)',
'afterRender (MixedReadWrite)',
'afterNextRender (MixedReadWrite)',
'afterRender (Read)',
'afterNextRender (Read)',
]);
// Running change detection again
log.length = 0;
fixture.detectChanges();
expect(log).toEqual([
'afterRender (EarlyRead)',
'afterRender (Write)',
'afterRender (MixedReadWrite)',
'afterRender (Read)',
]);
});
});
describe('afterRender', () => {
it('should run with the correct timing', () => {
@Component({selector: 'dynamic-comp'})