From d45bd2f53d8a3f94452e05025815909893d6c5ce Mon Sep 17 00:00:00 2001 From: Cameron Smick Date: Mon, 20 Jul 2026 13:30:22 -0700 Subject: [PATCH] refactor(core): implement toggleWatchSignal for DevTools signal debugging Implement \`toggleWatchSignal(id)\` in \`signal_debug.ts\` to enable toggling reactive watch listeners on individual signal graph nodes. - Create reactive \`Watch\` instances using \`createWatch\` to log signal value/state changes to the console when active. - Use \`WeakRef\` mapping and \`FinalizationRegistry\` (\`watchCleanupRegistry\`) so active watches do not retain strong references to signal nodes or prevent garbage collection. - Expose \`watched\` status on \`DebugSignalGraphNode\` and publish \`toggleWatchSignal\` onto \`window.ng\` in development mode. - Add acceptance tests covering watch activation, signal mutation logging, manual disposal, GC cleanup, and safe invalid ID handling. --- .../core/primitives/signals/index.api.md | 3 + .../devtools/src/debug_signal_graph.ts | 1 + packages/core/primitives/signals/index.ts | 2 +- .../core/src/render3/util/global_utils.ts | 23 +- .../core/src/render3/util/signal_debug.ts | 196 +++++++++++++++++- .../core/test/acceptance/signal_debug_spec.ts | 183 +++++++++++++++- 6 files changed, 388 insertions(+), 20 deletions(-) diff --git a/goldens/public-api/core/primitives/signals/index.api.md b/goldens/public-api/core/primitives/signals/index.api.md index 1a6ab32eac3..40e8b725134 100644 --- a/goldens/public-api/core/primitives/signals/index.api.md +++ b/goldens/public-api/core/primitives/signals/index.api.md @@ -61,6 +61,9 @@ export function createWatch(fn: (onCleanup: WatchCleanupRegisterFn) => void, sch // @public export function defaultEquals(a: T, b: T): boolean; +// @public +export const ERRORED: any; + // @public export function finalizeConsumerAfterComputation(node: ReactiveNode): void; diff --git a/packages/core/primitives/devtools/src/debug_signal_graph.ts b/packages/core/primitives/devtools/src/debug_signal_graph.ts index 48a5a8856e9..3411b34ccab 100644 --- a/packages/core/primitives/devtools/src/debug_signal_graph.ts +++ b/packages/core/primitives/devtools/src/debug_signal_graph.ts @@ -15,6 +15,7 @@ export interface DebugSignalGraphNode { label?: string; value?: unknown; debuggableFn?: () => unknown; + watched?: boolean; } export interface DebugSignalGraphEdge { diff --git a/packages/core/primitives/signals/index.ts b/packages/core/primitives/signals/index.ts index 7584f1601db..bf96c266902 100644 --- a/packages/core/primitives/signals/index.ts +++ b/packages/core/primitives/signals/index.ts @@ -8,7 +8,7 @@ import {installDevToolsSignalFormatter} from './src/formatter'; -export {ComputedNode, createComputed} from './src/computed'; +export {ComputedNode, createComputed, ERRORED} from './src/computed'; export { ComputationFn, LinkedSignalNode, diff --git a/packages/core/src/render3/util/global_utils.ts b/packages/core/src/render3/util/global_utils.ts index e9b3e1fdf5b..61d6f3bdc62 100644 --- a/packages/core/src/render3/util/global_utils.ts +++ b/packages/core/src/render3/util/global_utils.ts @@ -17,8 +17,6 @@ import {Signal, isSignal} from '../reactivity/api'; import {applyChanges} from './change_detection_utils'; import {getControlFlowBlocks} from './control_flow'; import { - AngularComponentDebugMetadata, - AngularDirectiveDebugMetadata, DirectiveDebugMetadata, Listener, getComponent, @@ -37,7 +35,7 @@ import { getInjectorProviders, getInjectorResolutionPath, } from './injector_discovery_utils'; -import {getSignalGraph} from './signal_debug'; +import {getSignalGraph, toggleWatchSignal} from './signal_debug'; import { enableProfiling, @@ -94,7 +92,9 @@ interface NonCoreGlobalUtils { * Angular. This allows fast iteration on new global utils and only applies Angular's long-lived * versioning constraint when we are ready to accept it. */ -interface InternalCoreGlobalUtils {} +interface InternalCoreGlobalUtils { + toggleWatchSignal(id: string): void; +} /** * The set of external (meaning outside google3) global utils implemented by `@angular/core`. @@ -135,6 +135,10 @@ export interface ExternalCoreGlobalUtils { enableProfiling(): void; } +const internalCoreGlobalUtils: InternalCoreGlobalUtils = { + toggleWatchSignal, +}; + const externalCoreGlobalUtils: ExternalCoreGlobalUtils = { /** * Warning: functions that start with `ɵ` are considered *INTERNAL* and should not be relied upon @@ -185,6 +189,10 @@ export function publishDefaultGlobalUtils() { for (const [methodName, method] of Object.entries(externalCoreGlobalUtils)) { publishGlobalUtil(methodName as keyof ExternalCoreGlobalUtils, method); } + + for (const [methodName, method] of Object.entries(internalCoreGlobalUtils)) { + publishGlobalUtil(methodName as keyof InternalCoreGlobalUtils, method); + } } } @@ -192,10 +200,9 @@ export function publishDefaultGlobalUtils() { * Publishes the given function to `window.ng` so that it can be * used from the browser console when an application is not in production. */ -export function publishGlobalUtil( - name: K, - fn: ExternalCoreGlobalUtils[K], -): void { +export function publishGlobalUtil< + K extends keyof InternalCoreGlobalUtils | keyof ExternalCoreGlobalUtils, +>(name: K, fn: (InternalCoreGlobalUtils & ExternalCoreGlobalUtils)[K]): void { publishUtil(name, fn); } diff --git a/packages/core/src/render3/util/signal_debug.ts b/packages/core/src/render3/util/signal_debug.ts index df7c674d8a5..1519eeabf02 100644 --- a/packages/core/src/render3/util/signal_debug.ts +++ b/packages/core/src/render3/util/signal_debug.ts @@ -12,10 +12,18 @@ import type { } from '../../../primitives/devtools'; import { ComputedNode, + ERRORED, LinkedSignalNode, + REACTIVE_NODE, ReactiveNode, SIGNAL, SignalNode, + consumerAfterComputation, + consumerBeforeComputation, + consumerDestroy, + consumerPollProducersForChange, + producerAccessed, + producerUpdateValueVersion, } from '../../../primitives/signals'; import {Injector} from '../../di/injector'; import {R3Injector} from '../../di/r3_injector'; @@ -70,9 +78,46 @@ function getTemplateConsumer(injector: NodeInjector): ReactiveLViewConsumer | nu return null; } +/** + * Maps a `ReactiveNode` to its generated unique string ID for DevTools. + */ const signalDebugMap = new WeakMap(); + +interface DebugWatchNode extends ReactiveNode { + targetNode: WeakRef; + destroyed: boolean; +} + +/** + * Stores signal debug metadata by string ID, holding a `WeakRef` to the `ReactiveNode` + * and a `WeakRef` to any active `DebugWatchNode` so signals and watchers can form isolated + * cycles that are eligible for garbage collection when app references are dropped. + */ +const signalDebugNodeMap = new Map< + string, + { + node: WeakRef | ComputedNode | LinkedSignalNode>; + watch?: WeakRef; + } +>(); + +/** + * Finalization registry that destroys and cleans up a `DebugWatchNode` automatically if the target + * `ReactiveNode` is garbage-collected while being watched. + */ +const watchCleanupRegistry = new FinalizationRegistry<{id: string}>(({id}) => { + unwatchSignal(id); + signalDebugNodeMap.delete(id); +}); let counter = 0; +function isWatched(id: string): boolean { + const watchRef = signalDebugNodeMap.get(id)?.watch; + if (!watchRef) return false; + const watchNode = watchRef.deref(); + return watchNode !== undefined && !watchNode.destroyed; +} + function getNodesAndEdgesFromSignalMap(signalMap: ReadonlyMap): { nodes: DebugSignalGraphNode[]; edges: DebugSignalGraphEdge[]; @@ -91,31 +136,31 @@ function getNodesAndEdgesFromSignalMap(signalMap: ReadonlyMap unknown, + watched: isWatched(id), id, }); } else if (isSignalNode(consumer)) { + if (!signalDebugNodeMap.has(id)) { + signalDebugNodeMap.set(id, {node: new WeakRef(consumer)}); + watchCleanupRegistry.register(consumer, {id}); + } debugSignalGraphNodes.push({ label: consumer.debugName, value: consumer.value, kind: consumer.kind, epoch: consumer.version, + watched: isWatched(id), id, }); } else if (isTemplateEffectNode(consumer)) { @@ -126,6 +171,21 @@ function getNodesAndEdgesFromSignalMap(signalMap: ReadonlyMap unknown) | undefined, + watched: false, + id, + }); + } else if (isLinkedSignalNode(consumer)) { + if (!signalDebugNodeMap.has(id)) { + signalDebugNodeMap.set(id, {node: new WeakRef(consumer)}); + watchCleanupRegistry.register(consumer, {id}); + } + debugSignalGraphNodes.push({ + label: consumer.debugName, + value: consumer.value, + kind: consumer.kind, + epoch: consumer.version, + debuggableFn: consumer.computation as (() => unknown) | undefined, + watched: isWatched(id), id, }); } else if (isEffectNode(consumer)) { @@ -149,6 +209,7 @@ function getNodesAndEdgesFromSignalMap(signalMap: ReadonlyMap { + if (node.destroyed) return; + // Schedule watch re-execution asynchronously in the next microtask after + // the signal update cycle completes, ceding the main thread back to the + // framework and avoiding synchronous re-entrancy during value computation. + queueMicrotask(() => runDebugWatch(node)); + }, + }; + return node; +} + +function runDebugWatch(node: DebugWatchNode): void { + if (node.destroyed) return; + + const targetNode = node.targetNode.deref(); + if (!targetNode) { + node.destroyed = true; + consumerDestroy(node); + return; + } + + node.dirty = false; + if (node.version > 0 && !consumerPollProducersForChange(node)) { + return; + } + node.version++; + + const prevConsumer = consumerBeforeComputation(node); + try { + producerUpdateValueVersion(targetNode); + producerAccessed(targetNode); + const name = targetNode.debugName ? targetNode.debugName : 'DevTools signal watch'; + if ( + (isComputedNode(targetNode) || isLinkedSignalNode(targetNode)) && + targetNode.value === ERRORED + ) { + // tslint:disable-next-line:no-console + console.error(`[${name} (error)]:`, targetNode.error); + return; + } + const value = + isSignalNode(targetNode) || isComputedNode(targetNode) || isLinkedSignalNode(targetNode) + ? targetNode.value + : undefined; + // tslint:disable-next-line:no-console + console.log(`[${name}]:`, value); + } finally { + consumerAfterComputation(node, prevConsumer); + } +} + +function unwatchSignal(id: string) { + const entry = signalDebugNodeMap.get(id); + if (entry?.watch) { + const watchNode = entry.watch.deref(); + if (watchNode && !watchNode.destroyed) { + watchNode.destroyed = true; + consumerDestroy(watchNode); + } + entry.watch = undefined; + } +} diff --git a/packages/core/test/acceptance/signal_debug_spec.ts b/packages/core/test/acceptance/signal_debug_spec.ts index e0b98e3b105..0cd403b590d 100644 --- a/packages/core/test/acceptance/signal_debug_spec.ts +++ b/packages/core/test/acceptance/signal_debug_spec.ts @@ -27,7 +27,7 @@ import { } from '../../src/render3/debug/framework_injector_profiler'; import {setInjectorProfiler} from '../../src/render3/debug/injector_profiler'; import type {DebugSignalGraphEdge, DebugSignalGraphNode} from '../../primitives/devtools'; -import {getSignalGraph} from '../../src/render3/util/signal_debug'; +import {getSignalGraph, toggleWatchSignal} from '../../src/render3/util/signal_debug'; import {TestBed} from '../../testing'; describe('getSignalGraph', () => { @@ -574,3 +574,184 @@ describe('getSignalGraph', () => { }); }); }); + +describe('toggleWatchSignal', () => { + beforeEach(() => { + setInjectorProfiler(null); + setupFrameworkInjectorProfiler(); + }); + + afterEach(() => { + getFrameworkDIDebugData().reset(); + setInjectorProfiler(null); + TestBed.resetTestingModule(); + }); + + it('should toggle watching a signal, printing debugging information when active, and stopping when disposed', async () => { + @Component({selector: 'component-with-watched-signal', template: `{{ mySignal() }}`}) + class WithWatchedSignal { + mySignal = signal(100, {debugName: 'mySignal'}); + } + TestBed.configureTestingModule({imports: [WithWatchedSignal]}); + const fixture = TestBed.createComponent(WithWatchedSignal); + + await fixture.whenStable(); + const injector = fixture.componentRef.injector; + + const initialGraph = getSignalGraph(injector); + const signalNode = initialGraph.nodes.find((node) => node.label === 'mySignal')!; + expect(signalNode).toBeDefined(); + expect(signalNode.watched).toBe(false); + + const spy = spyOn(console, 'log'); + + toggleWatchSignal(signalNode.id); + + expect(spy).toHaveBeenCalledWith('[mySignal]:', 100); + spy.calls.reset(); + + const activeGraph = getSignalGraph(injector); + const activeSignalNode = activeGraph.nodes.find((node) => node.label === 'mySignal')!; + expect(activeSignalNode.watched).toBe(true); + + fixture.componentInstance.mySignal.set(200); + await fixture.whenStable(); + + expect(spy).toHaveBeenCalledWith('[mySignal]:', 200); + spy.calls.reset(); + + toggleWatchSignal(signalNode.id); + + const disposedGraph = getSignalGraph(injector); + const disposedSignalNode = disposedGraph.nodes.find((node) => node.label === 'mySignal')!; + expect(disposedSignalNode.watched).toBe(false); + + fixture.componentInstance.mySignal.set(300); + await fixture.whenStable(); + + expect(spy).not.toHaveBeenCalled(); + }); + + it('should dispose the watch when toggled off', async () => { + @Component({selector: 'component-with-disposed-watch', template: `{{ mySignal() }}`}) + class App { + mySignal = signal('initial'); + } + const fixture = TestBed.createComponent(App); + await fixture.whenStable(); + + const signalGraph = getSignalGraph(fixture.componentRef.injector); + const signalNode = signalGraph.nodes.find((node) => node.kind === 'signal')!; + + const spy = spyOn(console, 'log'); + + // Start watching (triggers initial log execution) + toggleWatchSignal(signalNode.id); + expect(spy).toHaveBeenCalledTimes(1); + spy.calls.reset(); + + // Signal update should trigger log execution while watched + fixture.componentInstance.mySignal.set('watched update'); + await fixture.whenStable(); + expect(spy).toHaveBeenCalledTimes(1); + spy.calls.reset(); + + // Stop watching (disposes watch) + toggleWatchSignal(signalNode.id); + + // Further signal updates should not trigger logging + fixture.componentInstance.mySignal.set('unwatched update'); + await fixture.whenStable(); + expect(spy).not.toHaveBeenCalled(); + }); + + it('should dispose watch and clean up tracking maps if node is dereferenced as undefined', async () => { + @Component({selector: 'component-for-deref-test', template: `{{ mySignal() }}`}) + class App { + mySignal = signal('hello'); + } + TestBed.configureTestingModule({imports: [App]}); + const fixture = TestBed.createComponent(App); + await fixture.whenStable(); + + const signalGraph = getSignalGraph(fixture.componentRef.injector); + const signalNode = signalGraph.nodes.find((node) => node.kind === 'signal')!; + + // Start watching + toggleWatchSignal(signalNode.id); + + // Simulate garbage collection across WeakRef instances + spyOn(WeakRef.prototype, 'deref').and.returnValue(undefined); + + const spy = spyOn(console, 'log'); + + // Triggering signal update causes watch callback to run, detecting node is gone + fixture.componentInstance.mySignal.set('world'); + await fixture.whenStable(); + + // Watch should destroy itself and avoid logging + expect(spy).not.toHaveBeenCalled(); + + // Calling toggleWatchSignal on the dead node ID should run safely without errors + expect(() => toggleWatchSignal(signalNode.id)).not.toThrow(); + }); + + it('should handle non-existent node IDs safely', () => { + const spyLog = spyOn(console, 'log'); + const spyWarn = spyOn(console, 'warn'); + expect(() => toggleWatchSignal('non-existent-id-99999')).not.toThrow(); + expect(spyLog).not.toHaveBeenCalled(); + expect(spyWarn).toHaveBeenCalledTimes(1); + }); + + it('should log the caught error when watching an errored computed signal', async () => { + const computationError = new Error('Computation failed'); + @Component({ + selector: 'component-with-errored-computed', + template: `{{ display() }}`, + }) + class WithErroredComputed { + source = signal(false); + computedVal = computed( + () => { + if (this.source()) { + throw computationError; + } + return 42; + }, + {debugName: 'computedVal'}, + ); + display = computed(() => { + try { + return this.computedVal(); + } catch { + return 'errored'; + } + }); + } + + TestBed.configureTestingModule({imports: [WithErroredComputed]}); + const fixture = TestBed.createComponent(WithErroredComputed); + await fixture.whenStable(); + + const {nodes} = getSignalGraph(fixture.componentRef.injector); + const computedNode = nodes.find((n) => n.label === 'computedVal')!; + expect(computedNode).toBeDefined(); + + const spyLog = spyOn(console, 'log'); + const spyError = spyOn(console, 'error'); + + // Start watching when valid + toggleWatchSignal(computedNode.id); + expect(spyLog).toHaveBeenCalledWith('[computedVal]:', 42); + expect(spyError).not.toHaveBeenCalled(); + spyLog.calls.reset(); + + // Trigger update that causes computation to throw + fixture.componentInstance.source.set(true); + await fixture.whenStable(); + + expect(spyError).toHaveBeenCalledWith('[computedVal (error)]:', computationError); + expect(spyLog).not.toHaveBeenCalled(); + }); +});