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 89c0c09c2b..9b99d84fa5 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 @@ -54,7 +54,8 @@ describe('Ivy NgModule', () => { } expect(() => createInjector(AModule)) - .toThrowError('Circular dependency: type AModule ends up importing itself.'); + .toThrowError( + 'Circular dependency in DI detected for type 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 86271b54df..6bd99964a6 100644 --- a/packages/core/src/di/r3_injector.ts +++ b/packages/core/src/di/r3_injector.ts @@ -108,8 +108,8 @@ export class R3Injector { readonly parent: Injector) { // Start off by creating Records for every provider declared in every InjectorType // included transitively in `def`. - deepForEach( - [def], injectorDef => this.processInjectorType(injectorDef, new Set>())); + const dedupStack: InjectorType[] = []; + deepForEach([def], injectorDef => this.processInjectorType(injectorDef, [], dedupStack)); additionalProviders && deepForEach(additionalProviders, provider => this.processProvider(provider)); @@ -198,7 +198,7 @@ export class R3Injector { */ private processInjectorType( defOrWrappedDef: InjectorType|InjectorTypeWithProviders, - parents: Set>) { + parents: InjectorType[], dedupStack: InjectorType[]) { defOrWrappedDef = resolveForwardRef(defOrWrappedDef); // Either the defOrWrappedDef is an InjectorType (with ngInjectorDef) or an @@ -218,6 +218,18 @@ export class R3Injector { const defType: InjectorType = (ngModule === undefined) ? (defOrWrappedDef as InjectorType) : ngModule; + // 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}.`); + } + + // Check for multiple imports of the same module + if (dedupStack.indexOf(defType) !== -1) { + return; + } + // If defOrWrappedType was an InjectorDefTypeWithProviders, then .providers may hold some // extra providers. const providers = @@ -235,11 +247,6 @@ export class R3Injector { return; } - // Check for circular dependencies. - if (parents.has(defType)) { - throw new Error(`Circular dependency: type ${stringify(defType)} ends up importing itself.`); - } - // Track the InjectorType and add a provider for it. this.injectorDefTypes.add(defType); this.records.set(defType, makeRecord(def.factory)); @@ -250,12 +257,16 @@ export class R3Injector { if (def.imports != null) { // Before processing defType's imports, add it to the set of parents. This way, if it ends // up deeply importing itself, this can be detected. - parents.add(defType); + ngDevMode && parents.push(defType); + // Add it to the set of dedups. This way we can detect multiple imports of the same module + dedupStack.push(defType); + try { - deepForEach(def.imports, imported => this.processInjectorType(imported, parents)); + deepForEach( + def.imports, imported => this.processInjectorType(imported, parents, dedupStack)); } finally { // Remove it from the parents set when finished. - parents.delete(defType); + ngDevMode && parents.pop(); } } diff --git a/packages/platform-server/test/integration_spec.ts b/packages/platform-server/test/integration_spec.ts index 72e921fabb..15be437172 100644 --- a/packages/platform-server/test/integration_spec.ts +++ b/packages/platform-server/test/integration_spec.ts @@ -433,21 +433,20 @@ class HiddenModule { }); })); - fixmeIvy('no deduplication of imported modules') && - it('adds styles with ng-transition attribute', async(() => { - const platform = platformDynamicServer([{ - provide: INITIAL_CONFIG, - useValue: {document: ''} - }]); - platform.bootstrapModule(ExampleStylesModule).then(ref => { - const doc = ref.injector.get(DOCUMENT); - const head = getDOM().getElementsByTagName(doc, 'head')[0]; - const styles: any[] = head.children as any; - expect(styles.length).toBe(1); - expect(getDOM().getText(styles[0])).toContain('color: red'); - expect(getDOM().getAttribute(styles[0], 'ng-transition')).toBe('example-styles'); - }); - })); + it('adds styles with ng-transition attribute', async(() => { + const platform = platformDynamicServer([{ + provide: INITIAL_CONFIG, + useValue: {document: ''} + }]); + platform.bootstrapModule(ExampleStylesModule).then(ref => { + const doc = ref.injector.get(DOCUMENT); + const head = getDOM().getElementsByTagName(doc, 'head')[0]; + const styles: any[] = head.children as any; + expect(styles.length).toBe(1); + expect(getDOM().getText(styles[0])).toContain('color: red'); + expect(getDOM().getAttribute(styles[0], 'ng-transition')).toBe('example-styles'); + }); + })); it('copies known properties to attributes', async(() => { const platform = platformDynamicServer( @@ -718,27 +717,25 @@ class HiddenModule { }); })); - fixmeIvy('no deduplication of imported modules') && - it('requests are macrotasks', async(() => { - const platform = platformDynamicServer( - [{provide: INITIAL_CONFIG, useValue: {document: ''}}]); - platform.bootstrapModule(ExampleModule).then(ref => { - const mock = ref.injector.get(MockBackend); - const http = ref.injector.get(Http); - expect(ref.injector.get(NgZone).hasPendingMacrotasks).toBeFalsy(); - ref.injector.get(NgZone).run(() => { - NgZone.assertInAngularZone(); - mock.connections.subscribe((mc: MockConnection) => { - expect(ref.injector.get(NgZone).hasPendingMacrotasks).toBeTruthy(); - mc.mockRespond( - new Response(new ResponseOptions({body: 'success!', status: 200}))); - }); - http.get('http://localhost/testing').subscribe(resp => { - expect(resp.text()).toBe('success!'); - }); - }); + it('requests are macrotasks', async(() => { + const platform = platformDynamicServer( + [{provide: INITIAL_CONFIG, useValue: {document: ''}}]); + platform.bootstrapModule(ExampleModule).then(ref => { + const mock = ref.injector.get(MockBackend); + const http = ref.injector.get(Http); + expect(ref.injector.get(NgZone).hasPendingMacrotasks).toBeFalsy(); + ref.injector.get(NgZone).run(() => { + NgZone.assertInAngularZone(); + mock.connections.subscribe((mc: MockConnection) => { + expect(ref.injector.get(NgZone).hasPendingMacrotasks).toBeTruthy(); + mc.mockRespond(new Response(new ResponseOptions({body: 'success!', status: 200}))); }); - })); + http.get('http://localhost/testing').subscribe(resp => { + expect(resp.text()).toBe('success!'); + }); + }); + }); + })); it('works when HttpModule is included before ServerModule', async(() => { const platform = platformDynamicServer( @@ -760,27 +757,25 @@ class HiddenModule { }); })); - fixmeIvy('no deduplication of imported modules') && - it('works when HttpModule is included after ServerModule', async(() => { - const platform = platformDynamicServer( - [{provide: INITIAL_CONFIG, useValue: {document: ''}}]); - platform.bootstrapModule(HttpAfterExampleModule).then(ref => { - const mock = ref.injector.get(MockBackend); - const http = ref.injector.get(Http); - expect(ref.injector.get(NgZone).hasPendingMacrotasks).toBeFalsy(); - ref.injector.get(NgZone).run(() => { - NgZone.assertInAngularZone(); - mock.connections.subscribe((mc: MockConnection) => { - expect(ref.injector.get(NgZone).hasPendingMacrotasks).toBeTruthy(); - mc.mockRespond( - new Response(new ResponseOptions({body: 'success!', status: 200}))); - }); - http.get('http://localhost/testing').subscribe(resp => { - expect(resp.text()).toBe('success!'); - }); - }); + it('works when HttpModule is included after ServerModule', async(() => { + const platform = platformDynamicServer( + [{provide: INITIAL_CONFIG, useValue: {document: ''}}]); + platform.bootstrapModule(HttpAfterExampleModule).then(ref => { + const mock = ref.injector.get(MockBackend); + const http = ref.injector.get(Http); + expect(ref.injector.get(NgZone).hasPendingMacrotasks).toBeFalsy(); + ref.injector.get(NgZone).run(() => { + NgZone.assertInAngularZone(); + mock.connections.subscribe((mc: MockConnection) => { + expect(ref.injector.get(NgZone).hasPendingMacrotasks).toBeTruthy(); + mc.mockRespond(new Response(new ResponseOptions({body: 'success!', status: 200}))); }); - })); + http.get('http://localhost/testing').subscribe(resp => { + expect(resp.text()).toBe('success!'); + }); + }); + }); + })); it('throws when given a relative URL', async(() => { const platform = platformDynamicServer(