Skip to content

Commit 846c73d

Browse files
crisbetoatscott
crisbeto
authored and
atscott
committed
fix(core): add debugName on afterRenderEffect
Adds a `debugName` to `afterRenderEffect` to match `effect`.
1 parent 4a59cef commit 846c73d

6 files changed

Lines changed: 264 additions & 9 deletions

File tree

‎goldens/public-api/core/index.api.md‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,15 +48,20 @@ export function afterNextRender<E = never, W = never, M = never>(spec: {
4848
export function afterNextRender(callback: VoidFunction, options?: AfterRenderOptions): AfterRenderRef;
4949

5050
// @public
51-
export function afterRenderEffect(callback: (onCleanup: EffectCleanupRegisterFn) => void, options?: AfterRenderOptions): AfterRenderRef;
51+
export function afterRenderEffect(callback: (onCleanup: EffectCleanupRegisterFn) => void, options?: AfterRenderEffectOptions): AfterRenderRef;
5252

5353
// @public
5454
export function afterRenderEffect<E = never, W = never, M = never>(spec: {
5555
earlyRead?: (onCleanup: EffectCleanupRegisterFn) => E;
5656
write?: (...args: [...ɵFirstAvailableSignal<[E]>, EffectCleanupRegisterFn]) => W;
5757
mixedReadWrite?: (...args: [...ɵFirstAvailableSignal<[W, E]>, EffectCleanupRegisterFn]) => M;
5858
read?: (...args: [...ɵFirstAvailableSignal<[M, W, E]>, EffectCleanupRegisterFn]) => void;
59-
}, options?: AfterRenderOptions): AfterRenderRef;
59+
}, options?: AfterRenderEffectOptions): AfterRenderRef;
60+
61+
// @public
62+
export interface AfterRenderEffectOptions extends AfterRenderOptions {
63+
debugName?: string;
64+
}
6065

6166
// @public
6267
export interface AfterRenderOptions {

‎packages/compiler-cli/src/ngtsc/transform/src/implicit_signal_debug_name_transform.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,7 @@ const signalFunctions: ReadonlyMap<string, PackageName> = new Map([
232232
['contentChild', 'core'],
233233
['contentChildren', 'core'],
234234
['effect', 'core'],
235+
['afterRenderEffect', 'core'],
235236
['resource', 'core'],
236237
['httpResource', 'common'],
237238
]);

‎packages/compiler-cli/test/ngtsc/debug_transform_spec.ts‎

Lines changed: 198 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2109,6 +2109,204 @@ runInEachFileSystem(() => {
21092109
});
21102110
});
21112111

2112+
describe('afterRenderEffect', () => {
2113+
it('should not insert debug info into afterRenderEffect function if not imported from angular core', () => {
2114+
env.write(
2115+
'test.ts',
2116+
`
2117+
declare function afterRenderEffect(fn: () => any): any;
2118+
import {Component} from '@angular/core';
2119+
2120+
@Component({
2121+
template: ''
2122+
}) class MyComponent {
2123+
testEffect = afterRenderEffect(() => {});
2124+
}
2125+
`,
2126+
);
2127+
env.driveMain();
2128+
const jsContents = env.getContents('test.js');
2129+
expect(jsContents).not.toContain('debugName');
2130+
});
2131+
2132+
it('should insert debug info into afterRenderEffect function if imported from angular core', () => {
2133+
env.write(
2134+
'test.ts',
2135+
`
2136+
import {signal, afterRenderEffect, Component} from '@angular/core';
2137+
2138+
@Component({
2139+
template: ''
2140+
}) class MyComponent {
2141+
testSignal = signal(123);
2142+
testEffect = afterRenderEffect(() => this.testSignal());
2143+
}
2144+
`,
2145+
);
2146+
env.driveMain();
2147+
const jsContents = env.getContents('test.js');
2148+
expect(cleanNewLines(jsContents)).toContain(
2149+
`afterRenderEffect(() => this.testSignal(), /* @ts-ignore */ ...(ngDevMode ? [{ debugName: "testEffect" }] : /* istanbul ignore next */ []))`,
2150+
);
2151+
});
2152+
2153+
it('should insert debug info into afterRenderEffect function with a phase spec', () => {
2154+
env.write(
2155+
'test.ts',
2156+
`
2157+
import {signal, afterRenderEffect, Component} from '@angular/core';
2158+
2159+
@Component({
2160+
template: ''
2161+
}) class MyComponent {
2162+
testSignal = signal(123);
2163+
testEffect = afterRenderEffect({read: () => this.testSignal()});
2164+
}
2165+
`,
2166+
);
2167+
env.driveMain();
2168+
const jsContents = env.getContents('test.js');
2169+
expect(cleanNewLines(jsContents)).toContain(
2170+
`afterRenderEffect({ read: () => this.testSignal() }, /* @ts-ignore */ ...(ngDevMode ? [{ debugName: "testEffect" }] : /* istanbul ignore next */ []))`,
2171+
);
2172+
});
2173+
2174+
it('should insert debug info into afterRenderEffect function in a property assignment', () => {
2175+
env.write(
2176+
'test.ts',
2177+
`
2178+
import {signal, afterRenderEffect, AfterRenderRef, Component} from '@angular/core';
2179+
2180+
@Component({
2181+
template: ''
2182+
}) class MyComponent {
2183+
testSignal = signal(123);
2184+
testEffect: AfterRenderRef;
2185+
constructor() {
2186+
this.testEffect = afterRenderEffect(() => this.testSignal());
2187+
}
2188+
}
2189+
`,
2190+
);
2191+
env.driveMain();
2192+
const jsContents = env.getContents('test.js');
2193+
expect(cleanNewLines(jsContents)).toContain(
2194+
`afterRenderEffect(() => this.testSignal(), /* @ts-ignore */ ...(ngDevMode ? [{ debugName: "testEffect" }] : /* istanbul ignore next */ []))`,
2195+
);
2196+
});
2197+
2198+
it('should not override an explicit debugName', () => {
2199+
env.write(
2200+
'test.ts',
2201+
`
2202+
import {afterRenderEffect, Component} from '@angular/core';
2203+
2204+
@Component({
2205+
template: ''
2206+
}) class MyComponent {
2207+
testEffect = afterRenderEffect(() => {}, {debugName: 'customName'});
2208+
}
2209+
`,
2210+
);
2211+
env.driveMain();
2212+
const jsContents = env.getContents('test.js');
2213+
expect(jsContents).toContain(`debugName: 'customName'`);
2214+
expect(jsContents).not.toContain('debugName: "testEffect"');
2215+
});
2216+
2217+
it('should tree-shake away debug info if in prod mode', async () => {
2218+
env.write(
2219+
'test.ts',
2220+
`
2221+
import {signal, afterRenderEffect, Component} from '@angular/core';
2222+
@Component({
2223+
template: ''
2224+
}) class MyComponent {
2225+
testSignal = signal(123);
2226+
testEffect = afterRenderEffect(() => this.testSignal());
2227+
}
2228+
`,
2229+
);
2230+
env.driveMain();
2231+
const jsContents = env.getContents('test.js');
2232+
const builtContent = (await esbuild.transform(jsContents, minifiedProdBuildOptions)).code;
2233+
expect(builtContent).not.toContain('debugName');
2234+
expect(cleanNewLines(builtContent)).toContain(
2235+
'afterRenderEffect( () => this.testSignal() )',
2236+
);
2237+
});
2238+
2239+
it('should not tree-shake away debug info if in dev mode', async () => {
2240+
env.write(
2241+
'test.ts',
2242+
`
2243+
import {signal, afterRenderEffect, Component} from '@angular/core';
2244+
2245+
@Component({
2246+
template: ''
2247+
}) class MyComponent {
2248+
testSignal = signal(123);
2249+
testEffect = afterRenderEffect(() => this.testSignal());
2250+
}
2251+
`,
2252+
);
2253+
env.driveMain();
2254+
2255+
const jsContents = env.getContents('test.js');
2256+
const builtContent = (await esbuild.transform(jsContents, minifiedDevBuildOptions)).code;
2257+
expect(cleanNewLines(builtContent)).toContain(
2258+
`afterRenderEffect( () => this.testSignal(), { debugName: "testEffect" } )`,
2259+
);
2260+
});
2261+
2262+
it('should tree-shake away debug info if in prod mode with existing options', async () => {
2263+
env.write(
2264+
'test.ts',
2265+
`
2266+
import {signal, afterRenderEffect, Component} from '@angular/core';
2267+
2268+
@Component({
2269+
template: ''
2270+
}) class MyComponent {
2271+
testSignal = signal(123);
2272+
testEffect = afterRenderEffect(() => this.testSignal(), { manualCleanup: true });
2273+
}
2274+
`,
2275+
);
2276+
env.driveMain();
2277+
2278+
const jsContents = env.getContents('test.js');
2279+
const builtContent = (await esbuild.transform(jsContents, minifiedProdBuildOptions)).code;
2280+
expect(builtContent).not.toContain('debugName');
2281+
expect(cleanNewLines(builtContent)).toContain(
2282+
'afterRenderEffect(() => this.testSignal(), { manualCleanup: !0 })',
2283+
);
2284+
});
2285+
2286+
it('should not tree-shake away debug info if in dev mode with existing options', async () => {
2287+
env.write(
2288+
'test.ts',
2289+
`
2290+
import {signal, afterRenderEffect, Component} from '@angular/core';
2291+
2292+
@Component({
2293+
template: ''
2294+
}) class MyComponent {
2295+
testSignal = signal(123);
2296+
testEffect = afterRenderEffect(() => this.testSignal(), { manualCleanup: true });
2297+
}
2298+
`,
2299+
);
2300+
env.driveMain();
2301+
2302+
const jsContents = env.getContents('test.js');
2303+
const builtContent = (await esbuild.transform(jsContents, minifiedDevBuildOptions)).code;
2304+
expect(cleanNewLines(builtContent)).toContain(
2305+
`afterRenderEffect(() => this.testSignal(), { debugName: "testEffect", manualCleanup: !0 })`,
2306+
);
2307+
});
2308+
});
2309+
21122310
describe('linkedSignal', () => {
21132311
it('should not insert debug info into linkedSignal function if not imported from angular core', () => {
21142312
env.write(

‎packages/core/src/core_reactivity_export_internal.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,5 +26,9 @@ export {
2626
EffectCleanupRegisterFn,
2727
} from './render3/reactivity/effect';
2828
export {EffectScheduler as ɵEffectScheduler} from './render3/reactivity/root_effect_scheduler';
29-
export {afterRenderEffect, ɵFirstAvailableSignal} from './render3/reactivity/after_render_effect';
29+
export {
30+
afterRenderEffect,
31+
AfterRenderEffectOptions,
32+
ɵFirstAvailableSignal,
33+
} from './render3/reactivity/after_render_effect';
3034
export {assertNotInReactiveContext} from './render3/reactivity/asserts';

‎packages/core/src/render3/reactivity/after_render_effect.ts‎

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,18 @@ import {
4646
const NOT_SET = /* @__PURE__ */ Symbol('NOT_SET');
4747
const EMPTY_CLEANUP_SET = /* @__PURE__ */ new Set<() => void>();
4848

49+
/**
50+
* Options passed to `afterRenderEffect`.
51+
*
52+
* @publicApi
53+
*/
54+
export interface AfterRenderEffectOptions extends AfterRenderOptions {
55+
/**
56+
* A debug name for the effect. Used in Angular DevTools to identify the effect.
57+
*/
58+
debugName?: string;
59+
}
60+
4961
/** Callback type for an `afterRenderEffect` phase effect */
5062
type AfterRenderPhaseEffectHook = (
5163
// Either a cleanup function or a pipelined value and a cleanup function
@@ -190,6 +202,7 @@ export class AfterRenderEffectSequence extends AfterRenderSequence {
190202
readonly scheduler: ChangeDetectionScheduler,
191203
injector: Injector,
192204
snapshot: TracingSnapshot | null = null,
205+
debugName?: string,
193206
) {
194207
// Note that we also initialize the underlying `AfterRenderSequence` hooks to `undefined` and
195208
// populate them as we create reactive nodes below.
@@ -228,7 +241,7 @@ export class AfterRenderEffectSequence extends AfterRenderSequence {
228241
this.hooks[phase] = (value) => node.phaseFn(value);
229242

230243
if (ngDevMode) {
231-
setupDebugInfo(node, injector);
244+
setupDebugInfo(node, injector, debugName);
232245
}
233246
}
234247
}
@@ -304,7 +317,7 @@ export type ɵFirstAvailableSignal<T extends unknown[]> = T extends [infer H, ..
304317
*/
305318
export function afterRenderEffect(
306319
callback: (onCleanup: EffectCleanupRegisterFn) => void,
307-
options?: AfterRenderOptions,
320+
options?: AfterRenderEffectOptions,
308321
): AfterRenderRef;
309322
/**
310323
* Register effects that, when triggered, are invoked when the application finishes rendering,
@@ -373,7 +386,7 @@ export function afterRenderEffect<E = never, W = never, M = never>(
373386
mixedReadWrite?: (...args: [...ɵFirstAvailableSignal<[W, E]>, EffectCleanupRegisterFn]) => M;
374387
read?: (...args: [...ɵFirstAvailableSignal<[M, W, E]>, EffectCleanupRegisterFn]) => void;
375388
},
376-
options?: AfterRenderOptions,
389+
options?: AfterRenderEffectOptions,
377390
): AfterRenderRef;
378391

379392
/**
@@ -390,7 +403,7 @@ export function afterRenderEffect<E = never, W = never, M = never>(
390403
) => M;
391404
read?: (...args: [...ɵFirstAvailableSignal<[M, W, E]>, EffectCleanupRegisterFn]) => void;
392405
},
393-
options?: AfterRenderOptions,
406+
options?: AfterRenderEffectOptions,
394407
): AfterRenderRef {
395408
ngDevMode &&
396409
assertNotInReactiveContext(
@@ -427,13 +440,18 @@ export function afterRenderEffect<E = never, W = never, M = never>(
427440
scheduler,
428441
injector,
429442
tracing?.snapshot(null),
443+
ngDevMode ? options?.debugName : undefined,
430444
);
431445
manager.impl.register(sequence);
432446
return sequence;
433447
}
434448

435-
function setupDebugInfo(node: AfterRenderPhaseEffectNode, injector: Injector): void {
436-
node.debugName = `afterRenderEffect - ${phaseDebugName(node.phase)} phase`;
449+
function setupDebugInfo(
450+
node: AfterRenderPhaseEffectNode,
451+
injector: Injector,
452+
debugName: string | undefined,
453+
): void {
454+
node.debugName = `${debugName || 'afterRenderEffect'} - ${phaseDebugName(node.phase)} phase`;
437455
const prevInjectorProfilerContext = setInjectorProfilerContext({injector, token: null});
438456
try {
439457
emitAfterRenderEffectPhaseCreatedEvent(node);

‎packages/core/test/acceptance/signal_debug_spec.ts‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -468,6 +468,35 @@ describe('getSignalGraph', () => {
468468
expect(getFrameworkDIDebugData().resolverToEffects.get(injector)?.length).toBe(0);
469469
});
470470

471+
it('should use the debugName of afterRenderEffect for each phase', async () => {
472+
@Component({template: ''})
473+
class App {
474+
constructor() {
475+
afterRenderEffect(
476+
{
477+
earlyRead: () => {},
478+
write: () => {},
479+
mixedReadWrite: () => {},
480+
read: () => {},
481+
},
482+
{debugName: 'myRenderEffect'},
483+
);
484+
}
485+
}
486+
const fixture = TestBed.createComponent(App);
487+
await fixture.whenStable();
488+
489+
const {nodes} = getSignalGraph(fixture.componentRef.injector);
490+
const labels = nodes.filter((n) => n.kind === 'afterRenderEffectPhase').map((n) => n.label);
491+
492+
expect(labels.sort()).toEqual([
493+
'myRenderEffect - EarlyRead phase',
494+
'myRenderEffect - MixedReadWrite phase',
495+
'myRenderEffect - Read phase',
496+
'myRenderEffect - Write phase',
497+
]);
498+
});
499+
471500
describe('debuggableFn', () => {
472501
it('should expose the computation of a computed', async () => {
473502
const computation = () => 1;

0 commit comments

Comments
 (0)