diff --git a/packages/core/src/view/entrypoint.ts b/packages/core/src/view/entrypoint.ts index 7bd118e6cf..ff2933baf1 100644 --- a/packages/core/src/view/entrypoint.ts +++ b/packages/core/src/view/entrypoint.ts @@ -12,7 +12,7 @@ import {NgModuleFactory, NgModuleRef} from '../linker/ng_module_factory'; import {Type} from '../type'; import {initServicesIfNeeded} from './services'; -import {NgModuleDefinitionFactory, ProviderOverride, Services, ViewDefinition} from './types'; +import {NgModuleDefinition, NgModuleDefinitionFactory, NgModuleProviderDef, ProviderOverride, Services, ViewDefinition} from './types'; import {resolveDefinition} from './util'; export function overrideProvider(override: ProviderOverride) { @@ -38,6 +38,20 @@ export function createNgModuleFactory( return new NgModuleFactory_(ngModuleType, bootstrapComponents, defFactory); } +function cloneNgModuleDefinition(def: NgModuleDefinition): NgModuleDefinition { + const providers = Array.from(def.providers); + const modules = Array.from(def.modules); + const providersByKey: {[tokenKey: string]: NgModuleProviderDef} = {}; + for (const key in def.providersByKey) { + providersByKey[key] = def.providersByKey[key]; + } + + return { + factory: def.factory, + isRoot: def.isRoot, providers, modules, providersByKey, + }; +} + class NgModuleFactory_ extends NgModuleFactory { constructor( public readonly moduleType: Type, private _bootstrapComponents: Type[], @@ -49,7 +63,10 @@ class NgModuleFactory_ extends NgModuleFactory { create(parentInjector: Injector|null): NgModuleRef { initServicesIfNeeded(); - const def = resolveDefinition(this._ngModuleDefFactory); + // Clone the NgModuleDefinition so that any tree shakeable provider definition + // added to this instance of the NgModuleRef doesn't affect the cached copy. + // See https://github.com/angular/angular/issues/25018. + const def = cloneNgModuleDefinition(resolveDefinition(this._ngModuleDefFactory)); return Services.createNgModuleRef( this.moduleType, parentInjector || Injector.NULL, this._bootstrapComponents, def); } diff --git a/packages/core/test/bundling/hello_world_r2/bundle.golden_symbols.json b/packages/core/test/bundling/hello_world_r2/bundle.golden_symbols.json index cfb0615eeb..cabdda2fb0 100644 --- a/packages/core/test/bundling/hello_world_r2/bundle.golden_symbols.json +++ b/packages/core/test/bundling/hello_world_r2/bundle.golden_symbols.json @@ -2414,6 +2414,9 @@ { "name": "checkStable" }, + { + "name": "cloneNgModuleDefinition" + }, { "name": "collectReferences" }, diff --git a/packages/core/test/linker/ng_module_integration_spec.ts b/packages/core/test/linker/ng_module_integration_spec.ts index 45f530f76d..6a9369abd7 100644 --- a/packages/core/test/linker/ng_module_integration_spec.ts +++ b/packages/core/test/linker/ng_module_integration_spec.ts @@ -8,10 +8,13 @@ import {ANALYZE_FOR_ENTRY_COMPONENTS, CUSTOM_ELEMENTS_SCHEMA, Compiler, Component, ComponentFactoryResolver, Directive, HostBinding, Inject, Injectable, InjectionToken, Injector, Input, NgModule, NgModuleRef, Optional, Pipe, Provider, Self, Type, forwardRef, getModuleFactory} from '@angular/core'; import {Console} from '@angular/core/src/console'; +import {InjectableDef, defineInjectable} from '@angular/core/src/di/defs'; +import {NgModuleData} from '@angular/core/src/view/types'; +import {tokenKey} from '@angular/core/src/view/util'; import {ComponentFixture, TestBed, inject} from '@angular/core/testing'; import {expect} from '@angular/platform-browser/testing/src/matchers'; -import {InternalNgModuleRef} from '../../src/linker/ng_module_factory'; +import {InternalNgModuleRef, NgModuleFactory} from '../../src/linker/ng_module_factory'; import {clearModulesForTest} from '../../src/linker/ng_module_factory_loader'; import {stringify} from '../../src/util'; @@ -117,9 +120,13 @@ function declareTests({useJit}: {useJit: boolean}) { injector = _injector; })); + function createModuleFactory(moduleType: Type): NgModuleFactory { + return compiler.compileModuleSync(moduleType); + } + function createModule( moduleType: Type, parentInjector?: Injector | null): NgModuleRef { - return compiler.compileModuleSync(moduleType).create(parentInjector || null); + return createModuleFactory(moduleType).create(parentInjector || null); } function createComp(compType: Type, moduleType: Type): ComponentFixture { @@ -1290,6 +1297,37 @@ function declareTests({useJit}: {useJit: boolean}) { `Invalid provider for the NgModule 'ImportedModule1' - only instances of Provider and Type are allowed, got: [?broken?]`); }); }); + + describe('tree shakable providers', () => { + it('definition should not persist across NgModuleRef instances', () => { + @NgModule() + class SomeModule { + } + + class Bar { + static ngInjectableDef: InjectableDef = defineInjectable({ + factory: () => new Bar(), + providedIn: SomeModule, + }); + } + + const factory = createModuleFactory(SomeModule); + const ngModuleRef1 = factory.create(null); + + // Inject a tree shakeable provider token. + ngModuleRef1.injector.get(Bar); + + // Tree Shakeable provider definition should get added to the NgModule data. + const providerDef1 = (ngModuleRef1 as NgModuleData)._def.providersByKey[tokenKey(Bar)]; + expect(providerDef1).not.toBeUndefined(); + + // Instantiate the same module. The tree shakeable provider + // definition should not already be present. + const ngModuleRef2 = factory.create(null); + const providerDef2 = (ngModuleRef2 as NgModuleData)._def.providersByKey[tokenKey(Bar)]; + expect(providerDef2).toBeUndefined(); + }); + }); }); }); }