From dcafac1b8f1ae8ab2f6e78e7e241d17070f85b89 Mon Sep 17 00:00:00 2001 From: twerske Date: Tue, 20 Oct 2020 16:15:22 -0700 Subject: [PATCH] refactor(core): group provider and circular errors (#39251) group together similar error messages as part of error code efforts ProviderNotFound & NodeInjector grouped into throwProviderNotFoundError Cyclic dependency errors grouped into throwCyclicDependencyError PR Close #39251 --- .../guide/dependency-injection-navtree.md | 2 +- .../ivy_build/app/test/module_spec.ts | 2 +- packages/core/src/di/r3_injector.ts | 4 ++-- packages/core/src/render3/component.ts | 3 ++- packages/core/src/render3/di.ts | 7 ++++--- packages/core/src/render3/errors.ts | 13 ++++++++++--- .../view_engine_compatibility_prebound.ts | 3 ++- packages/core/test/acceptance/di_spec.ts | 16 +++++++++------- .../bundling/forms/bundle.golden_symbols.json | 3 +++ .../bundling/router/bundle.golden_symbols.json | 3 +++ .../bundling/todo/bundle.golden_symbols.json | 3 +++ .../test/linker/ng_module_integration_spec.ts | 4 +++- .../linker/view_injector_integration_spec.ts | 12 ++++++------ packages/core/test/render3/di_spec.ts | 2 +- 14 files changed, 50 insertions(+), 27 deletions(-) diff --git a/aio/content/guide/dependency-injection-navtree.md b/aio/content/guide/dependency-injection-navtree.md index 076c1e9d72..3eff1da2c2 100644 --- a/aio/content/guide/dependency-injection-navtree.md +++ b/aio/content/guide/dependency-injection-navtree.md @@ -196,7 +196,7 @@ which *is* what parent means. 2. Angular throws a cyclic dependency error if you omit the `@SkipSelf` decorator. - `Cannot instantiate cyclic dependency! (BethComponent -> Parent -> BethComponent)` + `Circular dependency in DI detected for BethComponent. Dependency path: BethComponent -> Parent -> BethComponent` Here's *Alice*, *Barry*, and family in action. diff --git a/packages/compiler-cli/integrationtest/bazel/injector_def/ivy_build/app/test/module_spec.ts b/packages/compiler-cli/integrationtest/bazel/injector_def/ivy_build/app/test/module_spec.ts index 1e0f4b2dc2..c9dfbe4a92 100644 --- a/packages/compiler-cli/integrationtest/bazel/injector_def/ivy_build/app/test/module_spec.ts +++ b/packages/compiler-cli/integrationtest/bazel/injector_def/ivy_build/app/test/module_spec.ts @@ -63,7 +63,7 @@ describe('Ivy NgModule', () => { expect(() => createInjector(AModule)) .toThrowError( - 'Circular dependency in DI detected for type AModule. Dependency path: AModule > BModule > AModule.'); + 'Circular dependency in DI detected for AModule. Dependency path: AModule > BModule > AModule'); }); it('merges imports and exports', () => { diff --git a/packages/core/src/di/r3_injector.ts b/packages/core/src/di/r3_injector.ts index 449f88f15d..1c147e622e 100644 --- a/packages/core/src/di/r3_injector.ts +++ b/packages/core/src/di/r3_injector.ts @@ -291,8 +291,8 @@ export class R3Injector { // Check for circular dependencies. if (ngDevMode && parents.indexOf(defType) !== -1) { const defName = stringify(defType); - throw new Error(`Circular dependency in DI detected for type ${defName}. Dependency path: ${ - parents.map(defType => stringify(defType)).join(' > ')} > ${defName}.`); + const path = parents.map(stringify); + throwCyclicDependencyError(defName, path); } // Check for multiple imports of the same module diff --git a/packages/core/src/render3/component.ts b/packages/core/src/render3/component.ts index e27a6f9cdd..03876b1ca4 100644 --- a/packages/core/src/render3/component.ts +++ b/packages/core/src/render3/component.ts @@ -15,6 +15,7 @@ import {assertDefined, assertIndexInRange} from '../util/assert'; import {assertComponentType} from './assert'; import {getComponentDef} from './definition'; import {diPublicInInjector, getOrCreateNodeInjectorForNode} from './di'; +import {throwProviderNotFoundError} from './errors'; import {registerPostOrderHooks} from './hooks'; import {addToViewTree, CLEAN_PROMISE, createLView, createTView, getOrCreateTComponentView, getOrCreateTNode, initTNodeFlags, instantiateRootComponent, invokeHostBindingsInCreationMode, locateHostElement, markAsComponentHost, refreshView, registerHostBindingOpCodes, renderView} from './instructions/shared'; import {ComponentDef, ComponentType, RenderFlags} from './interfaces/definition'; @@ -88,7 +89,7 @@ type HostFeature = ((component: T, componentDef: ComponentDef) => void); // TODO: A hack to not pull in the NullInjector from @angular/core. export const NULL_INJECTOR: Injector = { get: (token: any, notFoundValue?: any) => { - throw new Error('NullInjector: Not found: ' + stringifyForError(token)); + throwProviderNotFoundError(token, 'NullInjector'); } }; diff --git a/packages/core/src/render3/di.ts b/packages/core/src/render3/di.ts index e278b1112b..54085071a8 100644 --- a/packages/core/src/render3/di.ts +++ b/packages/core/src/render3/di.ts @@ -18,6 +18,7 @@ import {noSideEffects} from '../util/closure'; import {assertDirectiveDef, assertNodeInjector, assertTNodeForLView} from './assert'; import {getFactoryDef} from './definition'; +import {throwCyclicDependencyError, throwProviderNotFoundError} from './errors'; import {NG_ELEMENT_ID, NG_FACTORY_DEF} from './fields'; import {registerPreOrderHooks} from './hooks'; import {DirectiveDef, FactoryFn} from './interfaces/definition'; @@ -373,7 +374,7 @@ export function getOrCreateInjectable( try { const value = bloomHash(); if (value == null && !(flags & InjectFlags.Optional)) { - throw new Error(`No provider for ${stringifyForError(token)}!`); + throwProviderNotFoundError(token); } else { return value; } @@ -476,7 +477,7 @@ export function getOrCreateInjectable( if (flags & InjectFlags.Optional) { return notFoundValue; } else { - throw new Error(`NodeInjector: NOT_FOUND [${stringifyForError(token)}]`); + throwProviderNotFoundError(token, 'NodeInjector'); } } @@ -575,7 +576,7 @@ export function getNodeInjectable( if (isFactory(value)) { const factory: NodeInjectorFactory = value; if (factory.resolving) { - throw new Error(`Circular dep for ${stringifyForError(tData[index])}`); + throwCyclicDependencyError(stringifyForError(tData[index])); } const previousIncludeViewProviders = setIncludeViewProviders(factory.canSeeViewProviders); factory.resolving = true; diff --git a/packages/core/src/render3/errors.ts b/packages/core/src/render3/errors.ts index ad163b5447..2ffd70fd35 100644 --- a/packages/core/src/render3/errors.ts +++ b/packages/core/src/render3/errors.ts @@ -11,13 +11,14 @@ import {stringify} from '../util/stringify'; import {TNode} from './interfaces/node'; import {LView, TVIEW} from './interfaces/view'; -import {INTERPOLATION_DELIMITER} from './util/misc_utils'; +import {INTERPOLATION_DELIMITER, stringifyForError} from './util/misc_utils'; /** Called when directives inject each other (creating a circular dependency) */ -export function throwCyclicDependencyError(token: any): never { - throw new Error(`Cannot instantiate cyclic dependency! ${token}`); +export function throwCyclicDependencyError(token: string, path?: string[]): never { + const depPath = path ? `. Dependency path: ${path.join(' > ')} > ${token}` : ''; + throw new Error(`Circular dependency in DI detected for ${token}${depPath}`); } /** Called when there are multiple component selectors that match a given node */ @@ -116,3 +117,9 @@ export function getExpressionChangedErrorDetails( } return {propName: undefined, oldValue, newValue}; } + +/** Throws an error when a token is not found in DI. */ +export function throwProviderNotFoundError(token: any, injectorName?: string): never { + const injectorDetails = injectorName ? ` in ${injectorName}` : ''; + throw new Error(`No provider for ${stringifyForError(token)} found${injectorDetails}`); +} diff --git a/packages/core/src/render3/view_engine_compatibility_prebound.ts b/packages/core/src/render3/view_engine_compatibility_prebound.ts index 5842edac49..ebdec19b6b 100644 --- a/packages/core/src/render3/view_engine_compatibility_prebound.ts +++ b/packages/core/src/render3/view_engine_compatibility_prebound.ts @@ -11,6 +11,7 @@ import {ChangeDetectorRef} from '../change_detection/change_detector_ref'; import {InjectFlags} from '../di/interface/injector'; import {ElementRef as ViewEngine_ElementRef} from '../linker/element_ref'; import {TemplateRef as ViewEngine_TemplateRef} from '../linker/template_ref'; +import {throwProviderNotFoundError} from './errors'; import {TNode} from './interfaces/node'; import {LView} from './interfaces/view'; @@ -37,7 +38,7 @@ export function ɵɵtemplateRefExtractor(tNode: TNode, currentView: LView) { export function ɵɵinjectPipeChangeDetectorRef(flags = InjectFlags.Default): ChangeDetectorRef|null { const value = injectChangeDetectorRef(true); if (value == null && !(flags & InjectFlags.Optional)) { - throw new Error(`No provider for ChangeDetectorRef!`); + throwProviderNotFoundError('ChangeDetectorRef'); } else { return value; } diff --git a/packages/core/test/acceptance/di_spec.ts b/packages/core/test/acceptance/di_spec.ts index 4854d3f2e0..fc652dfcbd 100644 --- a/packages/core/test/acceptance/di_spec.ts +++ b/packages/core/test/acceptance/di_spec.ts @@ -615,7 +615,8 @@ describe('di', () => { } TestBed.configureTestingModule({declarations: [DirectiveA, DirectiveB, MyComp]}); - expect(() => TestBed.createComponent(MyComp)).toThrowError(/Circular dep for/); + expect(() => TestBed.createComponent(MyComp)) + .toThrowError('Circular dependency in DI detected for DirectiveA'); }); onlyInIvy('Ivy has different error message for circular dependency') @@ -630,7 +631,8 @@ describe('di', () => { } TestBed.configureTestingModule({declarations: [DirectiveA, DirectiveB, MyComp]}); - expect(() => TestBed.createComponent(MyComp)).toThrowError(/Circular dep for/); + expect(() => TestBed.createComponent(MyComp)) + .toThrowError('Circular dependency in DI detected for DirectiveA'); }); describe('flags', () => { @@ -734,7 +736,7 @@ describe('di', () => { } TestBed.configureTestingModule({declarations: [DirectiveA, DirectiveB, MyComp]}); expect(() => TestBed.createComponent(MyComp)) - .toThrowError(/NodeInjector: NOT_FOUND \[DirectiveB]/); + .toThrowError(/No provider for DirectiveB found in NodeInjector/); }); describe('@Host', () => { @@ -812,7 +814,7 @@ describe('di', () => { TestBed.configureTestingModule({declarations: [DirectiveString, MyComp, MyApp]}); expect(() => TestBed.createComponent(MyApp)) - .toThrowError(/NodeInjector: NOT_FOUND \[String]/); + .toThrowError('No provider for String found in NodeInjector'); }); onlyInIvy('Ivy has different error message when dependency is not found') @@ -828,7 +830,7 @@ describe('di', () => { TestBed.configureTestingModule( {declarations: [DirectiveA, DirectiveB, MyComp, MyApp]}); expect(() => TestBed.createComponent(MyApp)) - .toThrowError(/NodeInjector: NOT_FOUND \[DirectiveB]/); + .toThrowError(/No provider for DirectiveB found in NodeInjector/); }); onlyInIvy('Ivy has different error message when dependency is not found') @@ -853,7 +855,7 @@ describe('di', () => { expect(() => { fixture.componentInstance.myComp.showing = true; fixture.detectChanges(); - }).toThrowError(/NodeInjector: NOT_FOUND \[DirectiveB]/); + }).toThrowError(/No provider for DirectiveB found in NodeInjector/); }); it('should find providers across embedded views if not passing component boundary', () => { @@ -892,7 +894,7 @@ describe('di', () => { TestBed.configureTestingModule({declarations: [DirectiveComp, MyComp, MyApp]}); expect(() => TestBed.createComponent(MyApp)) - .toThrowError(/NodeInjector: NOT_FOUND \[MyApp]/); + .toThrowError('No provider for MyApp found in NodeInjector'); }); describe('regression', () => { diff --git a/packages/core/test/bundling/forms/bundle.golden_symbols.json b/packages/core/test/bundling/forms/bundle.golden_symbols.json index b19e08aa6c..4333990670 100644 --- a/packages/core/test/bundling/forms/bundle.golden_symbols.json +++ b/packages/core/test/bundling/forms/bundle.golden_symbols.json @@ -1568,6 +1568,9 @@ { "name": "syncPendingControls" }, + { + "name": "throwProviderNotFoundError" + }, { "name": "toObservable" }, diff --git a/packages/core/test/bundling/router/bundle.golden_symbols.json b/packages/core/test/bundling/router/bundle.golden_symbols.json index ac4e8100fe..ab1de47fc9 100644 --- a/packages/core/test/bundling/router/bundle.golden_symbols.json +++ b/packages/core/test/bundling/router/bundle.golden_symbols.json @@ -1931,6 +1931,9 @@ { "name": "throwIfEmpty" }, + { + "name": "throwProviderNotFoundError" + }, { "name": "toRefArray" }, diff --git a/packages/core/test/bundling/todo/bundle.golden_symbols.json b/packages/core/test/bundling/todo/bundle.golden_symbols.json index ff13d11633..55f744fd90 100644 --- a/packages/core/test/bundling/todo/bundle.golden_symbols.json +++ b/packages/core/test/bundling/todo/bundle.golden_symbols.json @@ -698,6 +698,9 @@ { "name": "stringifyForError" }, + { + "name": "throwProviderNotFoundError" + }, { "name": "toTStylingRange" }, diff --git a/packages/core/test/linker/ng_module_integration_spec.ts b/packages/core/test/linker/ng_module_integration_spec.ts index 93ca3d2879..d57678cd26 100644 --- a/packages/core/test/linker/ng_module_integration_spec.ts +++ b/packages/core/test/linker/ng_module_integration_spec.ts @@ -1024,8 +1024,10 @@ function declareTests(config?: {useJit: boolean}) { }); it('should throw when trying to instantiate a cyclic dependency', () => { + let errorMessage = ivyEnabled ? /Circular dependency in DI detected for Car/g : + /Cannot instantiate cyclic dependency! Car/g; expect(() => createInjector([Car, {provide: Engine, useClass: CyclicEngine}]).get(Car)) - .toThrowError(/Cannot instantiate cyclic dependency! Car/g); + .toThrowError(errorMessage); }); it('should support null values', () => { diff --git a/packages/core/test/linker/view_injector_integration_spec.ts b/packages/core/test/linker/view_injector_integration_spec.ts index ec3573318c..cee9d4ec50 100644 --- a/packages/core/test/linker/view_injector_integration_spec.ts +++ b/packages/core/test/linker/view_injector_integration_spec.ts @@ -628,7 +628,7 @@ describe('View injector', () => { .it('should not instantiate a directive with cyclic dependencies', () => { TestBed.configureTestingModule({declarations: [CycleDirective]}); expect(() => createComponent('
')) - .toThrowError('Circular dep for CycleDirective'); + .toThrowError('Circular dependency in DI detected for CycleDirective'); }); obsoleteInIvy('This error is no longer generated by the compiler') @@ -661,7 +661,7 @@ describe('View injector', () => { SimpleComponent, {set: {template: '
'}}); expect(() => createComponent('
')) - .toThrowError('NodeInjector: NOT_FOUND [service]'); + .toThrowError('No provider for service found in NodeInjector'); }); obsoleteInIvy('This error is no longer generated by the compiler') @@ -694,7 +694,7 @@ describe('View injector', () => { SimpleComponent, {set: {template: '
'}}); expect(() => createComponent('
')) - .toThrowError('NodeInjector: NOT_FOUND [service]'); + .toThrowError('No provider for service found in NodeInjector'); }); obsoleteInIvy('This error is no longer generated by the compiler') @@ -717,7 +717,7 @@ describe('View injector', () => { expect( () => createComponent( '
')) - .toThrowError('NodeInjector: NOT_FOUND [SimpleDirective]'); + .toThrowError('No provider for SimpleDirective found in NodeInjector'); }); it('should instantiate directives that depend on other directives', fakeAsync(() => { @@ -779,7 +779,7 @@ describe('View injector', () => { TestBed.overrideComponent( SimpleComponent, {set: {template: '
'}}); expect(() => createComponent('
')) - .toThrowError('NodeInjector: NOT_FOUND [SimpleDirective]'); + .toThrowError('No provider for SimpleDirective found in NodeInjector'); }); it('should allow to use the NgModule injector from a root ViewContainerRef.parentInjector', @@ -970,7 +970,7 @@ describe('View injector', () => { it('should throw if there is no TemplateRef', () => { TestBed.configureTestingModule({declarations: [NeedsTemplateRef]}); expect(() => createComponent('
')) - .toThrowError(/No provider for TemplateRef!/); + .toThrowError(/No provider for TemplateRef/); }); it('should inject null if there is no TemplateRef when the dependency is optional', () => { diff --git a/packages/core/test/render3/di_spec.ts b/packages/core/test/render3/di_spec.ts index bd0585da29..768917188c 100644 --- a/packages/core/test/render3/di_spec.ts +++ b/packages/core/test/render3/di_spec.ts @@ -107,7 +107,7 @@ describe('di', () => { (DirA as any)['__NG_ELEMENT_ID__'] = 1; (DirC as any)['__NG_ELEMENT_ID__'] = 257; new ComponentFixture(App); - }).toThrowError(/NodeInjector: NOT_FOUND \[DirB]/); + }).toThrowError('No provider for DirB found in NodeInjector'); }); }); });