Skip to content

Commit 52be848

Browse files
vsavkinvicb
vsavkin
authored and
vicb
committed
fix(router): incorrect injector is used when instantiating components loaded lazily (#12817)
1 parent 69dfcf7 commit 52be848

File tree

7 files changed

+110
-75
lines changed

7 files changed

+110
-75
lines changed

‎modules/@angular/router/src/apply_redirects.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -264,7 +264,7 @@ class ApplyRedirects {
264264

265265
private getChildConfig(injector: Injector, route: Route): Observable<LoadedRouterConfig> {
266266
if (route.children) {
267-
return of (new LoadedRouterConfig(route.children, injector, null));
267+
return of (new LoadedRouterConfig(route.children, injector, null, null));
268268
} else if (route.loadChildren) {
269269
return mergeMap.call(runGuards(injector, route), (shouldLoad: any) => {
270270
if (shouldLoad) {
@@ -281,7 +281,7 @@ class ApplyRedirects {
281281
}
282282
});
283283
} else {
284-
return of (new LoadedRouterConfig([], injector, null));
284+
return of (new LoadedRouterConfig([], injector, null, null));
285285
}
286286
}
287287
}

‎modules/@angular/router/src/directives/router_outlet.ts

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,9 @@ export class RouterOutlet implements OnDestroy {
5454

5555
ngOnDestroy(): void { this.parentOutletMap.removeOutlet(this.name ? this.name : PRIMARY_OUTLET); }
5656

57+
get locationInjector(): Injector { return this.location.injector; }
58+
get locationFactoryResolver(): ComponentFactoryResolver { return this.resolver; }
59+
5760
get isActivated(): boolean { return !!this.activated; }
5861
get component(): Object {
5962
if (!this.activated) throw new Error('Outlet is not activated');
@@ -74,9 +77,8 @@ export class RouterOutlet implements OnDestroy {
7477
}
7578

7679
activate(
77-
activatedRoute: ActivatedRoute, loadedResolver: ComponentFactoryResolver,
78-
loadedInjector: Injector, providers: ResolvedReflectiveProvider[],
79-
outletMap: RouterOutletMap): void {
80+
activatedRoute: ActivatedRoute, resolver: ComponentFactoryResolver, injector: Injector,
81+
providers: ResolvedReflectiveProvider[], outletMap: RouterOutletMap): void {
8082
if (this.isActivated) {
8183
throw new Error('Cannot activate an already activated outlet');
8284
}
@@ -86,15 +88,8 @@ export class RouterOutlet implements OnDestroy {
8688

8789
const snapshot = activatedRoute._futureSnapshot;
8890
const component: any = <any>snapshot._routeConfig.component;
91+
const factory = resolver.resolveComponentFactory(component);
8992

90-
let factory: ComponentFactory<any>;
91-
if (loadedResolver) {
92-
factory = loadedResolver.resolveComponentFactory(component);
93-
} else {
94-
factory = this.resolver.resolveComponentFactory(component);
95-
}
96-
97-
const injector = loadedInjector ? loadedInjector : this.location.parentInjector;
9893
const inj = ReflectiveInjector.fromResolvedProviders(providers, injector);
9994
this.activated = this.location.createComponent(factory, this.location.length, inj, []);
10095
this.activated.changeDetectorRef.detectChanges();

‎modules/@angular/router/src/router.ts

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1095,17 +1095,19 @@ class ActivateRoutes {
10951095

10961096
const config = parentLoadedConfig(future.snapshot);
10971097

1098-
let loadedFactoryResolver: ComponentFactoryResolver = null;
1099-
let loadedInjector: Injector = null;
1098+
let resolver: ComponentFactoryResolver = null;
1099+
let injector: Injector = null;
11001100

11011101
if (config) {
1102-
loadedFactoryResolver = config.factoryResolver;
1103-
loadedInjector = config.injector;
1104-
resolved.push({provide: ComponentFactoryResolver, useValue: loadedFactoryResolver});
1102+
injector = config.injectorFactory(outlet.locationInjector);
1103+
resolver = config.factoryResolver;
1104+
resolved.push({provide: ComponentFactoryResolver, useValue: resolver});
1105+
} else {
1106+
injector = outlet.locationInjector;
1107+
resolver = outlet.locationFactoryResolver;
11051108
}
1106-
outlet.activate(
1107-
future, loadedFactoryResolver, loadedInjector, ReflectiveInjector.resolve(resolved),
1108-
outletMap);
1109+
1110+
outlet.activate(future, resolver, injector, ReflectiveInjector.resolve(resolved), outletMap);
11091111
}
11101112

11111113
private deactiveRouteAndItsChildren(

‎modules/@angular/router/src/router_config_loader.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,17 +24,19 @@ export const ROUTES = new OpaqueToken('ROUTES');
2424
export class LoadedRouterConfig {
2525
constructor(
2626
public routes: Route[], public injector: Injector,
27-
public factoryResolver: ComponentFactoryResolver) {}
27+
public factoryResolver: ComponentFactoryResolver, public injectorFactory: Function) {}
2828
}
2929

3030
export class RouterConfigLoader {
3131
constructor(private loader: NgModuleFactoryLoader, private compiler: Compiler) {}
3232

3333
load(parentInjector: Injector, loadChildren: LoadChildren): Observable<LoadedRouterConfig> {
34-
return map.call(this.loadModuleFactory(loadChildren), (r: any) => {
34+
return map.call(this.loadModuleFactory(loadChildren), (r: NgModuleFactory<any>) => {
3535
const ref = r.create(parentInjector);
36+
const injectorFactory = (parent: Injector) => r.create(parent).injector;
3637
return new LoadedRouterConfig(
37-
flatten(ref.injector.get(ROUTES)), ref.injector, ref.componentFactoryResolver);
38+
flatten(ref.injector.get(ROUTES)), ref.injector, ref.componentFactoryResolver,
39+
injectorFactory);
3840
});
3941
}
4042

‎modules/@angular/router/test/apply_redirects.spec.ts

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,8 @@ describe('applyRedirects', () => {
143143
describe('lazy loading', () => {
144144
it('should load config on demand', () => {
145145
const loadedConfig = new LoadedRouterConfig(
146-
[{path: 'b', component: ComponentB}], <any>'stubInjector', <any>'stubFactoryResolver');
146+
[{path: 'b', component: ComponentB}], <any>'stubInjector', <any>'stubFactoryResolver',
147+
<any>'injectorFactory');
147148
const loader = {
148149
load: (injector: any, p: any) => {
149150
if (injector !== 'providedInjector') throw 'Invalid Injector';
@@ -171,7 +172,8 @@ describe('applyRedirects', () => {
171172

172173
it('should load when all canLoad guards return true', () => {
173174
const loadedConfig = new LoadedRouterConfig(
174-
[{path: 'b', component: ComponentB}], <any>'stubInjector', <any>'stubFactoryResolver');
175+
[{path: 'b', component: ComponentB}], <any>'stubInjector', <any>'stubFactoryResolver',
176+
<any>'injectorFactory');
175177
const loader = {load: (injector: any, p: any) => of (loadedConfig)};
176178

177179
const guard = () => true;
@@ -191,7 +193,8 @@ describe('applyRedirects', () => {
191193

192194
it('should not load when any canLoad guards return false', () => {
193195
const loadedConfig = new LoadedRouterConfig(
194-
[{path: 'b', component: ComponentB}], <any>'stubInjector', <any>'stubFactoryResolver');
196+
[{path: 'b', component: ComponentB}], <any>'stubInjector', <any>'stubFactoryResolver',
197+
<any>'injectorFactory');
195198
const loader = {load: (injector: any, p: any) => of (loadedConfig)};
196199

197200
const trueGuard = () => true;
@@ -216,7 +219,8 @@ describe('applyRedirects', () => {
216219

217220
it('should not load when any canLoad guards is rejected (promises)', () => {
218221
const loadedConfig = new LoadedRouterConfig(
219-
[{path: 'b', component: ComponentB}], <any>'stubInjector', <any>'stubFactoryResolver');
222+
[{path: 'b', component: ComponentB}], <any>'stubInjector', <any>'stubFactoryResolver',
223+
<any>'injectorFactory');
220224
const loader = {load: (injector: any, p: any) => of (loadedConfig)};
221225

222226
const trueGuard = () => Promise.resolve(true);
@@ -237,7 +241,8 @@ describe('applyRedirects', () => {
237241

238242
it('should work with objects implementing the CanLoad interface', () => {
239243
const loadedConfig = new LoadedRouterConfig(
240-
[{path: 'b', component: ComponentB}], <any>'stubInjector', <any>'stubFactoryResolver');
244+
[{path: 'b', component: ComponentB}], <any>'stubInjector', <any>'stubFactoryResolver',
245+
<any>'injectorFactory');
241246
const loader = {load: (injector: any, p: any) => of (loadedConfig)};
242247

243248
const guard = {canLoad: () => Promise.resolve(true)};
@@ -254,7 +259,8 @@ describe('applyRedirects', () => {
254259

255260
it('should work with absolute redirects', () => {
256261
const loadedConfig = new LoadedRouterConfig(
257-
[{path: '', component: ComponentB}], <any>'stubInjector', <any>'stubFactoryResolver');
262+
[{path: '', component: ComponentB}], <any>'stubInjector', <any>'stubFactoryResolver',
263+
<any>'injectorFactory');
258264

259265
const loader = {load: (injector: any, p: any) => of (loadedConfig)};
260266

@@ -269,7 +275,8 @@ describe('applyRedirects', () => {
269275

270276
it('should load the configuration only once', () => {
271277
const loadedConfig = new LoadedRouterConfig(
272-
[{path: '', component: ComponentB}], <any>'stubInjector', <any>'stubFactoryResolver');
278+
[{path: '', component: ComponentB}], <any>'stubInjector', <any>'stubFactoryResolver',
279+
<any>'injectorFactory');
273280

274281
let called = false;
275282
const loader = {
@@ -295,7 +302,8 @@ describe('applyRedirects', () => {
295302

296303
it('should load the configuration of a wildcard route', () => {
297304
const loadedConfig = new LoadedRouterConfig(
298-
[{path: '', component: ComponentB}], <any>'stubInjector', <any>'stubFactoryResolver');
305+
[{path: '', component: ComponentB}], <any>'stubInjector', <any>'stubFactoryResolver',
306+
<any>'injectorFactory');
299307

300308
const loader = {load: (injector: any, p: any) => of (loadedConfig)};
301309

@@ -308,7 +316,8 @@ describe('applyRedirects', () => {
308316

309317
it('should load the configuration after a local redirect from a wildcard route', () => {
310318
const loadedConfig = new LoadedRouterConfig(
311-
[{path: '', component: ComponentB}], <any>'stubInjector', <any>'stubFactoryResolver');
319+
[{path: '', component: ComponentB}], <any>'stubInjector', <any>'stubFactoryResolver',
320+
<any>'injectorFactory');
312321

313322
const loader = {load: (injector: any, p: any) => of (loadedConfig)};
314323

@@ -322,7 +331,8 @@ describe('applyRedirects', () => {
322331

323332
it('should load the configuration after an absolute redirect from a wildcard route', () => {
324333
const loadedConfig = new LoadedRouterConfig(
325-
[{path: '', component: ComponentB}], <any>'stubInjector', <any>'stubFactoryResolver');
334+
[{path: '', component: ComponentB}], <any>'stubInjector', <any>'stubFactoryResolver',
335+
<any>'injectorFactory');
326336

327337
const loader = {load: (injector: any, p: any) => of (loadedConfig)};
328338

‎modules/@angular/router/test/integration.spec.ts

Lines changed: 64 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -2024,54 +2024,78 @@ describe('Integration', () => {
20242024
expect(location.path()).toEqual('/lazy2/loaded');
20252025
})));
20262026

2027-
it('should use the injector of the lazily-loaded configuration',
2028-
fakeAsync(inject(
2029-
[Router, Location, NgModuleFactoryLoader],
2030-
(router: Router, location: Location, loader: SpyNgModuleFactoryLoader) => {
2031-
class LazyLoadedServiceDefinedInModule {}
2032-
class LazyLoadedServiceDefinedInCmp {}
20332027

2034-
@Component({selector: 'lazy', template: 'lazy-loaded'})
2035-
class LazyLoadedChildComponent {
2036-
constructor(service: LazyLoadedServiceDefinedInCmp) {}
2037-
}
2028+
describe('should use the injector of the lazily-loaded configuration', () => {
2029+
class LazyLoadedServiceDefinedInModule {}
2030+
class LazyLoadedServiceDefinedInCmp {}
20382031

2039-
@Component({
2040-
selector: 'lazy',
2041-
template: '<router-outlet></router-outlet>',
2042-
providers: [LazyLoadedServiceDefinedInCmp]
2043-
})
2044-
class LazyLoadedParentComponent {
2045-
constructor(service: LazyLoadedServiceDefinedInModule) {}
2046-
}
2032+
@Component({
2033+
selector: 'eager-parent',
2034+
template: 'eager-parent <router-outlet></router-outlet>',
2035+
})
2036+
class EagerParentComponent {
2037+
}
20472038

2048-
@NgModule({
2049-
declarations: [LazyLoadedParentComponent, LazyLoadedChildComponent],
2050-
imports: [RouterModule.forChild([{
2051-
path: '',
2052-
children: [{
2053-
path: 'loaded',
2054-
component: LazyLoadedParentComponent,
2055-
children: [{path: 'child', component: LazyLoadedChildComponent}]
2056-
}]
2057-
}])],
2058-
providers: [LazyLoadedServiceDefinedInModule]
2059-
})
2060-
class LoadedModule {
2061-
}
2039+
@Component({selector: 'lazy-parent', template: 'lazy-parent <router-outlet></router-outlet>'})
2040+
class LazyParentComponent {
2041+
}
20622042

2063-
loader.stubbedModules = {expected: LoadedModule};
2043+
@Component({selector: 'lazy-child', template: 'lazy-child'})
2044+
class LazyChildComponent {
2045+
constructor(
2046+
lazy: LazyParentComponent, // should be able to inject lazy/direct parent
2047+
lazyService: LazyLoadedServiceDefinedInModule, // should be able to inject lazy service
2048+
eager:
2049+
EagerParentComponent // should use the injector of the location to create a parent
2050+
) {}
2051+
}
20642052

2065-
const fixture = createRoot(router, RootCmp);
2053+
@NgModule({
2054+
declarations: [LazyParentComponent, LazyChildComponent],
2055+
imports: [RouterModule.forChild([{
2056+
path: '',
2057+
children: [{
2058+
path: 'lazy-parent',
2059+
component: LazyParentComponent,
2060+
children: [{path: 'lazy-child', component: LazyChildComponent}]
2061+
}]
2062+
}])],
2063+
providers: [LazyLoadedServiceDefinedInModule]
2064+
})
2065+
class LoadedModule {
2066+
}
20662067

2067-
router.resetConfig([{path: 'lazy', loadChildren: 'expected'}]);
2068+
@NgModule({
2069+
declarations: [EagerParentComponent],
2070+
entryComponents: [EagerParentComponent],
2071+
imports: [RouterModule]
2072+
})
2073+
class TestModule {
2074+
}
20682075

2069-
router.navigateByUrl('/lazy/loaded/child');
2070-
advance(fixture);
2076+
beforeEach(() => { TestBed.configureTestingModule({imports: [TestModule]}); });
20712077

2072-
expect(location.path()).toEqual('/lazy/loaded/child');
2073-
expect(fixture.nativeElement).toHaveText('lazy-loaded');
2074-
})));
2078+
it('should use the injector of the lazily-loaded configuration',
2079+
fakeAsync(inject(
2080+
[Router, Location, NgModuleFactoryLoader],
2081+
(router: Router, location: Location, loader: SpyNgModuleFactoryLoader) => {
2082+
loader.stubbedModules = {expected: LoadedModule};
2083+
2084+
const fixture = createRoot(router, RootCmp);
2085+
2086+
router.resetConfig([{
2087+
path: 'eager-parent',
2088+
component: EagerParentComponent,
2089+
children: [{path: 'lazy', loadChildren: 'expected'}]
2090+
}]);
2091+
2092+
router.navigateByUrl('/eager-parent/lazy/lazy-parent/lazy-child');
2093+
advance(fixture);
2094+
2095+
expect(location.path()).toEqual('/eager-parent/lazy/lazy-parent/lazy-child');
2096+
expect(fixture.nativeElement).toHaveText('eager-parent lazy-parent lazy-child');
2097+
})));
2098+
});
20752099

20762100
it('works when given a callback',
20772101
fakeAsync(inject(

‎tools/public_api_guard/router/index.d.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -289,9 +289,11 @@ export declare class RouterOutlet implements OnDestroy {
289289
component: Object;
290290
deactivateEvents: EventEmitter<any>;
291291
isActivated: boolean;
292+
locationFactoryResolver: ComponentFactoryResolver;
293+
locationInjector: Injector;
292294
outletMap: RouterOutletMap;
293295
constructor(parentOutletMap: RouterOutletMap, location: ViewContainerRef, resolver: ComponentFactoryResolver, name: string);
294-
activate(activatedRoute: ActivatedRoute, loadedResolver: ComponentFactoryResolver, loadedInjector: Injector, providers: ResolvedReflectiveProvider[], outletMap: RouterOutletMap): void;
296+
activate(activatedRoute: ActivatedRoute, resolver: ComponentFactoryResolver, injector: Injector, providers: ResolvedReflectiveProvider[], outletMap: RouterOutletMap): void;
295297
deactivate(): void;
296298
ngOnDestroy(): void;
297299
}

0 commit comments

Comments
 (0)