From b8d9f95faa656d2789bbb3006b465a5da289ebda Mon Sep 17 00:00:00 2001 From: Andrew Scott Date: Tue, 8 Apr 2025 12:18:44 -0700 Subject: [PATCH] Revert "refactor(core): allow multiple DI profilers (#60562)" (#60793) This reverts commit 56cc5fdb57f4dacc50b17e5a3e03ed321b16a71e. CI tests are consistently timing out after this commit PR Close #60793 --- .../debug/framework_injector_profiler.ts | 7 +- .../src/render3/debug/injector_profiler.ts | 40 ++------ .../test/acceptance/injector_profiler_spec.ts | 96 ++----------------- .../core/test/acceptance/signal_debug_spec.ts | 9 +- 4 files changed, 23 insertions(+), 129 deletions(-) diff --git a/packages/core/src/render3/debug/framework_injector_profiler.ts b/packages/core/src/render3/debug/framework_injector_profiler.ts index 56e6997d91f..728ec17b24c 100644 --- a/packages/core/src/render3/debug/framework_injector_profiler.ts +++ b/packages/core/src/render3/debug/framework_injector_profiler.ts @@ -20,7 +20,6 @@ import {EffectRef} from '../reactivity/effect'; import { InjectedService, InjectorCreatedInstance, - InjectorProfiler, InjectorProfilerContext, InjectorProfilerEvent, InjectorProfilerEventType, @@ -102,10 +101,12 @@ export function getFrameworkDIDebugData(): DIDebugData { */ export function setupFrameworkInjectorProfiler(): void { frameworkDIDebugData.reset(); - setInjectorProfiler(injectorProfilerEventHandler); + setInjectorProfiler((injectorProfilerEvent) => + handleInjectorProfilerEvent(injectorProfilerEvent), + ); } -function injectorProfilerEventHandler(injectorProfilerEvent: InjectorProfilerEvent): void { +function handleInjectorProfilerEvent(injectorProfilerEvent: InjectorProfilerEvent): void { const {context, type} = injectorProfilerEvent; if (type === InjectorProfilerEventType.Inject) { diff --git a/packages/core/src/render3/debug/injector_profiler.ts b/packages/core/src/render3/debug/injector_profiler.ts index 955c437ed81..509c2763014 100644 --- a/packages/core/src/render3/debug/injector_profiler.ts +++ b/packages/core/src/render3/debug/injector_profiler.ts @@ -194,55 +194,33 @@ export function setInjectorProfilerContext(context: InjectorProfilerContext) { return previous; } -const injectorProfilerCallbacks: InjectorProfiler[] = []; - -const NOOP_PROFILER_REMOVAL = () => {}; - -function removeProfiler(profiler: InjectorProfiler) { - const profilerIdx = injectorProfilerCallbacks.indexOf(profiler); - if (profilerIdx !== -1) { - injectorProfilerCallbacks.splice(profilerIdx, 1); - } -} +let injectorProfilerCallback: InjectorProfiler | null = null; /** - * Adds a callback function which will be invoked during certain DI events within the - * runtime (for example: injecting services, creating injectable instances, configuring providers). - * Multiple profiler callbacks can be set: in this case profiling events are - * reported to every registered callback. + * Sets the callback function which will be invoked during certain DI events within the + * runtime (for example: injecting services, creating injectable instances, configuring providers) * * Warning: this function is *INTERNAL* and should not be relied upon in application's code. * The contract of the function might be changed in any release and/or the function can be removed * completely. * * @param profiler function provided by the caller or null value to disable profiling. - * @returns a cleanup function that, when invoked, removes a given profiler callback. */ -export function setInjectorProfiler(injectorProfiler: InjectorProfiler | null): () => void { +export const setInjectorProfiler = (injectorProfiler: InjectorProfiler | null) => { !ngDevMode && throwError('setInjectorProfiler should never be called in production mode'); - - if (injectorProfiler !== null) { - if (!injectorProfilerCallbacks.includes(injectorProfiler)) { - injectorProfilerCallbacks.push(injectorProfiler); - } - return () => removeProfiler(injectorProfiler); - } else { - injectorProfilerCallbacks.length = 0; - return NOOP_PROFILER_REMOVAL; - } -} + injectorProfilerCallback = injectorProfiler; +}; /** * Injector profiler function which emits on DI events executed by the runtime. * * @param event InjectorProfilerEvent corresponding to the DI event being emitted */ -export function injectorProfiler(event: InjectorProfilerEvent): void { +function injectorProfiler(event: InjectorProfilerEvent): void { !ngDevMode && throwError('Injector profiler should never be called in production mode'); - for (let i = 0; i < injectorProfilerCallbacks.length; i++) { - const injectorProfilerCallback = injectorProfilerCallbacks[i]; - injectorProfilerCallback(event); + if (injectorProfilerCallback != null /* both `null` and `undefined` */) { + injectorProfilerCallback!(event); } } diff --git a/packages/core/test/acceptance/injector_profiler_spec.ts b/packages/core/test/acceptance/injector_profiler_spec.ts index d0a207c7909..f8f4046582a 100644 --- a/packages/core/test/acceptance/injector_profiler_spec.ts +++ b/packages/core/test/acceptance/injector_profiler_spec.ts @@ -43,8 +43,6 @@ import { InjectorProfilerEventType, ProviderConfiguredEvent, setInjectorProfiler, - injectorProfiler, - InjectorProfilerContext, } from '../../src/render3/debug/injector_profiler'; import {getNodeInjectorLView, NodeInjector} from '../../src/render3/di'; import { @@ -77,7 +75,6 @@ describe('setProfiler', () => { createEvents = []; providerConfiguredEvents = []; - setInjectorProfiler(null); setInjectorProfiler((injectorProfilerEvent: InjectorProfilerEvent) => { const {type} = injectorProfilerEvent; if (type === InjectorProfilerEventType.Inject) { @@ -106,7 +103,7 @@ describe('setProfiler', () => { }); }); - afterEach(() => setInjectorProfiler(null)); + afterAll(() => setInjectorProfiler(null)); it('should emit DI events when a component contains a provider and injects it', () => { class MyService {} @@ -385,77 +382,7 @@ describe('setProfiler', () => { }); }); -describe('profiler activation and removal', () => { - class SomeClass {} - - const fakeContext: InjectorProfilerContext = { - injector: Injector.create({providers: []}), - token: SomeClass, - }; - - const fakeEvent: InjectorCreatedInstanceEvent = { - type: InjectorProfilerEventType.InstanceCreatedByInjector, - context: fakeContext, - instance: {value: new SomeClass()}, - }; - - it('should allow adding and removing multiple profilers', () => { - const events: string[] = []; - const r1 = setInjectorProfiler((e) => events.push('P1: ' + e.type)); - const r2 = setInjectorProfiler((e) => events.push('P2: ' + e.type)); - - injectorProfiler(fakeEvent); - expect(events).toEqual(['P1: 1', 'P2: 1']); - - r1(); - injectorProfiler(fakeEvent); - expect(events).toEqual(['P1: 1', 'P2: 1', 'P2: 1']); - - r2(); - injectorProfiler(fakeEvent); - expect(events).toEqual(['P1: 1', 'P2: 1', 'P2: 1']); - }); - - it('should not add / remove the same profiler twice', () => { - const events: string[] = []; - const p1 = (e: InjectorProfilerEvent) => events.push('P1: ' + e.type); - const r1 = setInjectorProfiler(p1); - const r2 = setInjectorProfiler(p1); - - injectorProfiler(fakeEvent); - expect(events).toEqual(['P1: 1']); - - r1(); - injectorProfiler(fakeEvent); - expect(events).toEqual(['P1: 1']); - - // subsequent removals should be noop - r1(); - r2(); - }); - - it('should clear all profilers when passing null', () => { - const events: string[] = []; - setInjectorProfiler((e) => events.push('P1: ' + e.type)); - setInjectorProfiler((e) => events.push('P2: ' + e.type)); - - injectorProfiler(fakeEvent); - expect(events).toEqual(['P1: 1', 'P2: 1']); - - // clear all profilers - setInjectorProfiler(null); - injectorProfiler(fakeEvent); - expect(events).toEqual(['P1: 1', 'P2: 1']); - }); -}); - describe('getInjectorMetadata', () => { - beforeEach(() => { - setInjectorProfiler(null); - setupFrameworkInjectorProfiler(); - }); - afterEach(() => setInjectorProfiler(null)); - it('should be able to determine injector type and name', fakeAsync(() => { class MyServiceA {} @NgModule({providers: [MyServiceA]}) @@ -559,11 +486,8 @@ describe('getInjectorMetadata', () => { }); describe('getInjectorProviders', () => { - beforeEach(() => { - setInjectorProfiler(null); - setupFrameworkInjectorProfiler(); - }); - afterEach(() => setInjectorProfiler(null)); + beforeEach(() => setupFrameworkInjectorProfiler()); + afterAll(() => setInjectorProfiler(null)); it('should be able to get the providers from a components injector', () => { class MyService {} @@ -1028,11 +952,8 @@ describe('getInjectorProviders', () => { }); describe('getDependenciesFromInjectable', () => { - beforeEach(() => { - setInjectorProfiler(null); - setupFrameworkInjectorProfiler(); - }); - afterEach(() => setInjectorProfiler(null)); + beforeEach(() => setupFrameworkInjectorProfiler()); + afterAll(() => setInjectorProfiler(null)); it('should be able to determine which injector dependencies come from', fakeAsync(() => { class MyService {} @@ -1322,11 +1243,8 @@ describe('getDependenciesFromInjectable', () => { }); describe('getInjectorResolutionPath', () => { - beforeEach(() => { - setInjectorProfiler(null); - setupFrameworkInjectorProfiler(); - }); - afterEach(() => setInjectorProfiler(null)); + beforeEach(() => setupFrameworkInjectorProfiler()); + afterAll(() => setInjectorProfiler(null)); it('should be able to inspect injector hierarchy structure', fakeAsync(() => { class MyServiceA {} diff --git a/packages/core/test/acceptance/signal_debug_spec.ts b/packages/core/test/acceptance/signal_debug_spec.ts index 3f6d86397d5..45d62998d53 100644 --- a/packages/core/test/acceptance/signal_debug_spec.ts +++ b/packages/core/test/acceptance/signal_debug_spec.ts @@ -6,29 +6,26 @@ * found in the LICENSE file at https://angular.dev/license */ -import {Component, computed, effect, inject, Injectable, signal} from '../../src/core'; +import {Component, signal, effect, computed, Injectable, inject} from '../../src/core'; import { - getFrameworkDIDebugData, setupFrameworkInjectorProfiler, + getFrameworkDIDebugData, } from '../../src/render3/debug/framework_injector_profiler'; -import {setInjectorProfiler} from '../../src/render3/debug/injector_profiler'; import { DebugSignalGraphEdge, DebugSignalGraphNode, getSignalGraph, } from '../../src/render3/util/signal_debug'; -import {fakeAsync, TestBed, tick} from '../../testing'; +import {TestBed, fakeAsync, tick} from '../../testing'; describe('getSignalGraph', () => { beforeEach(() => { // Effect detection depends on the framework injector profiler being enabled - setInjectorProfiler(null); setupFrameworkInjectorProfiler(); }); afterEach(() => { getFrameworkDIDebugData().reset(); - setInjectorProfiler(null); TestBed.resetTestingModule(); });