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
This commit is contained in:
twerske 2020-10-20 16:15:22 -07:00 committed by Alex Rickabaugh
parent 3f00c6150b
commit dcafac1b8f
14 changed files with 50 additions and 27 deletions

View File

@ -196,7 +196,7 @@ which *is* what parent means.
2. Angular throws a cyclic dependency error if you omit the `@SkipSelf` decorator. 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. Here's *Alice*, *Barry*, and family in action.

View File

@ -63,7 +63,7 @@ describe('Ivy NgModule', () => {
expect(() => createInjector(AModule)) expect(() => createInjector(AModule))
.toThrowError( .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', () => { it('merges imports and exports', () => {

View File

@ -291,8 +291,8 @@ export class R3Injector {
// Check for circular dependencies. // Check for circular dependencies.
if (ngDevMode && parents.indexOf(defType) !== -1) { if (ngDevMode && parents.indexOf(defType) !== -1) {
const defName = stringify(defType); const defName = stringify(defType);
throw new Error(`Circular dependency in DI detected for type ${defName}. Dependency path: ${ const path = parents.map(stringify);
parents.map(defType => stringify(defType)).join(' > ')} > ${defName}.`); throwCyclicDependencyError(defName, path);
} }
// Check for multiple imports of the same module // Check for multiple imports of the same module

View File

@ -15,6 +15,7 @@ import {assertDefined, assertIndexInRange} from '../util/assert';
import {assertComponentType} from './assert'; import {assertComponentType} from './assert';
import {getComponentDef} from './definition'; import {getComponentDef} from './definition';
import {diPublicInInjector, getOrCreateNodeInjectorForNode} from './di'; import {diPublicInInjector, getOrCreateNodeInjectorForNode} from './di';
import {throwProviderNotFoundError} from './errors';
import {registerPostOrderHooks} from './hooks'; import {registerPostOrderHooks} from './hooks';
import {addToViewTree, CLEAN_PROMISE, createLView, createTView, getOrCreateTComponentView, getOrCreateTNode, initTNodeFlags, instantiateRootComponent, invokeHostBindingsInCreationMode, locateHostElement, markAsComponentHost, refreshView, registerHostBindingOpCodes, renderView} from './instructions/shared'; 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'; import {ComponentDef, ComponentType, RenderFlags} from './interfaces/definition';
@ -88,7 +89,7 @@ type HostFeature = (<T>(component: T, componentDef: ComponentDef<T>) => void);
// TODO: A hack to not pull in the NullInjector from @angular/core. // TODO: A hack to not pull in the NullInjector from @angular/core.
export const NULL_INJECTOR: Injector = { export const NULL_INJECTOR: Injector = {
get: (token: any, notFoundValue?: any) => { get: (token: any, notFoundValue?: any) => {
throw new Error('NullInjector: Not found: ' + stringifyForError(token)); throwProviderNotFoundError(token, 'NullInjector');
} }
}; };

View File

@ -18,6 +18,7 @@ import {noSideEffects} from '../util/closure';
import {assertDirectiveDef, assertNodeInjector, assertTNodeForLView} from './assert'; import {assertDirectiveDef, assertNodeInjector, assertTNodeForLView} from './assert';
import {getFactoryDef} from './definition'; import {getFactoryDef} from './definition';
import {throwCyclicDependencyError, throwProviderNotFoundError} from './errors';
import {NG_ELEMENT_ID, NG_FACTORY_DEF} from './fields'; import {NG_ELEMENT_ID, NG_FACTORY_DEF} from './fields';
import {registerPreOrderHooks} from './hooks'; import {registerPreOrderHooks} from './hooks';
import {DirectiveDef, FactoryFn} from './interfaces/definition'; import {DirectiveDef, FactoryFn} from './interfaces/definition';
@ -373,7 +374,7 @@ export function getOrCreateInjectable<T>(
try { try {
const value = bloomHash(); const value = bloomHash();
if (value == null && !(flags & InjectFlags.Optional)) { if (value == null && !(flags & InjectFlags.Optional)) {
throw new Error(`No provider for ${stringifyForError(token)}!`); throwProviderNotFoundError(token);
} else { } else {
return value; return value;
} }
@ -476,7 +477,7 @@ export function getOrCreateInjectable<T>(
if (flags & InjectFlags.Optional) { if (flags & InjectFlags.Optional) {
return notFoundValue; return notFoundValue;
} else { } else {
throw new Error(`NodeInjector: NOT_FOUND [${stringifyForError(token)}]`); throwProviderNotFoundError(token, 'NodeInjector');
} }
} }
@ -575,7 +576,7 @@ export function getNodeInjectable(
if (isFactory(value)) { if (isFactory(value)) {
const factory: NodeInjectorFactory = value; const factory: NodeInjectorFactory = value;
if (factory.resolving) { if (factory.resolving) {
throw new Error(`Circular dep for ${stringifyForError(tData[index])}`); throwCyclicDependencyError(stringifyForError(tData[index]));
} }
const previousIncludeViewProviders = setIncludeViewProviders(factory.canSeeViewProviders); const previousIncludeViewProviders = setIncludeViewProviders(factory.canSeeViewProviders);
factory.resolving = true; factory.resolving = true;

View File

@ -11,13 +11,14 @@ import {stringify} from '../util/stringify';
import {TNode} from './interfaces/node'; import {TNode} from './interfaces/node';
import {LView, TVIEW} from './interfaces/view'; 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) */ /** Called when directives inject each other (creating a circular dependency) */
export function throwCyclicDependencyError(token: any): never { export function throwCyclicDependencyError(token: string, path?: string[]): never {
throw new Error(`Cannot instantiate cyclic dependency! ${token}`); 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 */ /** Called when there are multiple component selectors that match a given node */
@ -116,3 +117,9 @@ export function getExpressionChangedErrorDetails(
} }
return {propName: undefined, oldValue, newValue}; 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}`);
}

View File

@ -11,6 +11,7 @@ import {ChangeDetectorRef} from '../change_detection/change_detector_ref';
import {InjectFlags} from '../di/interface/injector'; import {InjectFlags} from '../di/interface/injector';
import {ElementRef as ViewEngine_ElementRef} from '../linker/element_ref'; import {ElementRef as ViewEngine_ElementRef} from '../linker/element_ref';
import {TemplateRef as ViewEngine_TemplateRef} from '../linker/template_ref'; import {TemplateRef as ViewEngine_TemplateRef} from '../linker/template_ref';
import {throwProviderNotFoundError} from './errors';
import {TNode} from './interfaces/node'; import {TNode} from './interfaces/node';
import {LView} from './interfaces/view'; import {LView} from './interfaces/view';
@ -37,7 +38,7 @@ export function ɵɵtemplateRefExtractor(tNode: TNode, currentView: LView) {
export function ɵɵinjectPipeChangeDetectorRef(flags = InjectFlags.Default): ChangeDetectorRef|null { export function ɵɵinjectPipeChangeDetectorRef(flags = InjectFlags.Default): ChangeDetectorRef|null {
const value = injectChangeDetectorRef(true); const value = injectChangeDetectorRef(true);
if (value == null && !(flags & InjectFlags.Optional)) { if (value == null && !(flags & InjectFlags.Optional)) {
throw new Error(`No provider for ChangeDetectorRef!`); throwProviderNotFoundError('ChangeDetectorRef');
} else { } else {
return value; return value;
} }

View File

@ -615,7 +615,8 @@ describe('di', () => {
} }
TestBed.configureTestingModule({declarations: [DirectiveA, DirectiveB, MyComp]}); 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') onlyInIvy('Ivy has different error message for circular dependency')
@ -630,7 +631,8 @@ describe('di', () => {
} }
TestBed.configureTestingModule({declarations: [DirectiveA, DirectiveB, MyComp]}); 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', () => { describe('flags', () => {
@ -734,7 +736,7 @@ describe('di', () => {
} }
TestBed.configureTestingModule({declarations: [DirectiveA, DirectiveB, MyComp]}); TestBed.configureTestingModule({declarations: [DirectiveA, DirectiveB, MyComp]});
expect(() => TestBed.createComponent(MyComp)) expect(() => TestBed.createComponent(MyComp))
.toThrowError(/NodeInjector: NOT_FOUND \[DirectiveB]/); .toThrowError(/No provider for DirectiveB found in NodeInjector/);
}); });
describe('@Host', () => { describe('@Host', () => {
@ -812,7 +814,7 @@ describe('di', () => {
TestBed.configureTestingModule({declarations: [DirectiveString, MyComp, MyApp]}); TestBed.configureTestingModule({declarations: [DirectiveString, MyComp, MyApp]});
expect(() => TestBed.createComponent(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') onlyInIvy('Ivy has different error message when dependency is not found')
@ -828,7 +830,7 @@ describe('di', () => {
TestBed.configureTestingModule( TestBed.configureTestingModule(
{declarations: [DirectiveA, DirectiveB, MyComp, MyApp]}); {declarations: [DirectiveA, DirectiveB, MyComp, MyApp]});
expect(() => TestBed.createComponent(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') onlyInIvy('Ivy has different error message when dependency is not found')
@ -853,7 +855,7 @@ describe('di', () => {
expect(() => { expect(() => {
fixture.componentInstance.myComp.showing = true; fixture.componentInstance.myComp.showing = true;
fixture.detectChanges(); 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', () => { it('should find providers across embedded views if not passing component boundary', () => {
@ -892,7 +894,7 @@ describe('di', () => {
TestBed.configureTestingModule({declarations: [DirectiveComp, MyComp, MyApp]}); TestBed.configureTestingModule({declarations: [DirectiveComp, MyComp, MyApp]});
expect(() => TestBed.createComponent(MyApp)) expect(() => TestBed.createComponent(MyApp))
.toThrowError(/NodeInjector: NOT_FOUND \[MyApp]/); .toThrowError('No provider for MyApp found in NodeInjector');
}); });
describe('regression', () => { describe('regression', () => {

View File

@ -1568,6 +1568,9 @@
{ {
"name": "syncPendingControls" "name": "syncPendingControls"
}, },
{
"name": "throwProviderNotFoundError"
},
{ {
"name": "toObservable" "name": "toObservable"
}, },

View File

@ -1931,6 +1931,9 @@
{ {
"name": "throwIfEmpty" "name": "throwIfEmpty"
}, },
{
"name": "throwProviderNotFoundError"
},
{ {
"name": "toRefArray" "name": "toRefArray"
}, },

View File

@ -698,6 +698,9 @@
{ {
"name": "stringifyForError" "name": "stringifyForError"
}, },
{
"name": "throwProviderNotFoundError"
},
{ {
"name": "toTStylingRange" "name": "toTStylingRange"
}, },

View File

@ -1024,8 +1024,10 @@ function declareTests(config?: {useJit: boolean}) {
}); });
it('should throw when trying to instantiate a cyclic dependency', () => { 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)) expect(() => createInjector([Car, {provide: Engine, useClass: CyclicEngine}]).get(Car))
.toThrowError(/Cannot instantiate cyclic dependency! Car/g); .toThrowError(errorMessage);
}); });
it('should support null values', () => { it('should support null values', () => {

View File

@ -628,7 +628,7 @@ describe('View injector', () => {
.it('should not instantiate a directive with cyclic dependencies', () => { .it('should not instantiate a directive with cyclic dependencies', () => {
TestBed.configureTestingModule({declarations: [CycleDirective]}); TestBed.configureTestingModule({declarations: [CycleDirective]});
expect(() => createComponent('<div cycleDirective></div>')) expect(() => createComponent('<div cycleDirective></div>'))
.toThrowError('Circular dep for CycleDirective'); .toThrowError('Circular dependency in DI detected for CycleDirective');
}); });
obsoleteInIvy('This error is no longer generated by the compiler') obsoleteInIvy('This error is no longer generated by the compiler')
@ -661,7 +661,7 @@ describe('View injector', () => {
SimpleComponent, {set: {template: '<div needsServiceFromHost><div>'}}); SimpleComponent, {set: {template: '<div needsServiceFromHost><div>'}});
expect(() => createComponent('<div simpleComponent></div>')) expect(() => createComponent('<div simpleComponent></div>'))
.toThrowError('NodeInjector: NOT_FOUND [service]'); .toThrowError('No provider for service found in NodeInjector');
}); });
obsoleteInIvy('This error is no longer generated by the compiler') obsoleteInIvy('This error is no longer generated by the compiler')
@ -694,7 +694,7 @@ describe('View injector', () => {
SimpleComponent, {set: {template: '<div needsServiceFromHost><div>'}}); SimpleComponent, {set: {template: '<div needsServiceFromHost><div>'}});
expect(() => createComponent('<div simpleComponent someOtherDirective></div>')) expect(() => createComponent('<div simpleComponent someOtherDirective></div>'))
.toThrowError('NodeInjector: NOT_FOUND [service]'); .toThrowError('No provider for service found in NodeInjector');
}); });
obsoleteInIvy('This error is no longer generated by the compiler') obsoleteInIvy('This error is no longer generated by the compiler')
@ -717,7 +717,7 @@ describe('View injector', () => {
expect( expect(
() => createComponent( () => createComponent(
'<div simpleDirective><div needsDirectiveFromSelf></div></div>')) '<div simpleDirective><div needsDirectiveFromSelf></div></div>'))
.toThrowError('NodeInjector: NOT_FOUND [SimpleDirective]'); .toThrowError('No provider for SimpleDirective found in NodeInjector');
}); });
it('should instantiate directives that depend on other directives', fakeAsync(() => { it('should instantiate directives that depend on other directives', fakeAsync(() => {
@ -779,7 +779,7 @@ describe('View injector', () => {
TestBed.overrideComponent( TestBed.overrideComponent(
SimpleComponent, {set: {template: '<div needsDirectiveFromHost></div>'}}); SimpleComponent, {set: {template: '<div needsDirectiveFromHost></div>'}});
expect(() => createComponent('<div simpleComponent simpleDirective></div>')) expect(() => createComponent('<div simpleComponent simpleDirective></div>'))
.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', 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', () => { it('should throw if there is no TemplateRef', () => {
TestBed.configureTestingModule({declarations: [NeedsTemplateRef]}); TestBed.configureTestingModule({declarations: [NeedsTemplateRef]});
expect(() => createComponent('<div needsTemplateRef></div>')) expect(() => createComponent('<div needsTemplateRef></div>'))
.toThrowError(/No provider for TemplateRef!/); .toThrowError(/No provider for TemplateRef/);
}); });
it('should inject null if there is no TemplateRef when the dependency is optional', () => { it('should inject null if there is no TemplateRef when the dependency is optional', () => {

View File

@ -107,7 +107,7 @@ describe('di', () => {
(DirA as any)['__NG_ELEMENT_ID__'] = 1; (DirA as any)['__NG_ELEMENT_ID__'] = 1;
(DirC as any)['__NG_ELEMENT_ID__'] = 257; (DirC as any)['__NG_ELEMENT_ID__'] = 257;
new ComponentFixture(App); new ComponentFixture(App);
}).toThrowError(/NodeInjector: NOT_FOUND \[DirB]/); }).toThrowError('No provider for DirB found in NodeInjector');
}); });
}); });
}); });