Skip to content

Commit

Permalink
fix(router): incorrect injector is used when instantiating components…
Browse files Browse the repository at this point in the history 
… loaded lazily (angular#12817)
  • Loading branch information
@vsavkin @vicb
vsavkin authored and vicb committed Nov 12, 2016
1 parent 69dfcf7 commit 52be848
Show file tree
Hide file tree
Showing 7 changed files with 110 additions and 75 deletions.
4 changes: 2 additions & 2 deletions modules/@angular/router/src/apply_redirects.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ class ApplyRedirects {

private getChildConfig(injector: Injector, route: Route): Observable<LoadedRouterConfig> {
if (route.children) {
return of (new LoadedRouterConfig(route.children, injector, null));
return of (new LoadedRouterConfig(route.children, injector, null, null));
} else if (route.loadChildren) {
return mergeMap.call(runGuards(injector, route), (shouldLoad: any) => {
if (shouldLoad) {
Expand All @@ -281,7 +281,7 @@ class ApplyRedirects {
}
});
} else {
return of (new LoadedRouterConfig([], injector, null));
return of (new LoadedRouterConfig([], injector, null, null));
}
}
}
Expand Down
17 changes: 6 additions & 11 deletions modules/@angular/router/src/directives/router_outlet.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@ export class RouterOutlet implements OnDestroy {

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

get locationInjector(): Injector { return this.location.injector; }
get locationFactoryResolver(): ComponentFactoryResolver { return this.resolver; }

get isActivated(): boolean { return !!this.activated; }
get component(): Object {
if (!this.activated) throw new Error('Outlet is not activated');
Expand All @@ -74,9 +77,8 @@ export class RouterOutlet implements OnDestroy {
}

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

const snapshot = activatedRoute._futureSnapshot;
const component: any = <any>snapshot._routeConfig.component;
const factory = resolver.resolveComponentFactory(component);

let factory: ComponentFactory<any>;
if (loadedResolver) {
factory = loadedResolver.resolveComponentFactory(component);
} else {
factory = this.resolver.resolveComponentFactory(component);
}

const injector = loadedInjector ? loadedInjector : this.location.parentInjector;
const inj = ReflectiveInjector.fromResolvedProviders(providers, injector);
this.activated = this.location.createComponent(factory, this.location.length, inj, []);
this.activated.changeDetectorRef.detectChanges();
Expand Down
18 changes: 10 additions & 8 deletions modules/@angular/router/src/router.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1095,17 +1095,19 @@ class ActivateRoutes {

const config = parentLoadedConfig(future.snapshot);

let loadedFactoryResolver: ComponentFactoryResolver = null;
let loadedInjector: Injector = null;
let resolver: ComponentFactoryResolver = null;
let injector: Injector = null;

if (config) {
loadedFactoryResolver = config.factoryResolver;
loadedInjector = config.injector;
resolved.push({provide: ComponentFactoryResolver, useValue: loadedFactoryResolver});
injector = config.injectorFactory(outlet.locationInjector);
resolver = config.factoryResolver;
resolved.push({provide: ComponentFactoryResolver, useValue: resolver});
} else {
injector = outlet.locationInjector;
resolver = outlet.locationFactoryResolver;
}
outlet.activate(
future, loadedFactoryResolver, loadedInjector, ReflectiveInjector.resolve(resolved),
outletMap);

outlet.activate(future, resolver, injector, ReflectiveInjector.resolve(resolved), outletMap);
}

private deactiveRouteAndItsChildren(
Expand Down
8 changes: 5 additions & 3 deletions modules/@angular/router/src/router_config_loader.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,17 +24,19 @@ export const ROUTES = new OpaqueToken('ROUTES');
export class LoadedRouterConfig {
constructor(
public routes: Route[], public injector: Injector,
public factoryResolver: ComponentFactoryResolver) {}
public factoryResolver: ComponentFactoryResolver, public injectorFactory: Function) {}
}

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

load(parentInjector: Injector, loadChildren: LoadChildren): Observable<LoadedRouterConfig> {
return map.call(this.loadModuleFactory(loadChildren), (r: any) => {
return map.call(this.loadModuleFactory(loadChildren), (r: NgModuleFactory<any>) => {
const ref = r.create(parentInjector);
const injectorFactory = (parent: Injector) => r.create(parent).injector;
return new LoadedRouterConfig(
flatten(ref.injector.get(ROUTES)), ref.injector, ref.componentFactoryResolver);
flatten(ref.injector.get(ROUTES)), ref.injector, ref.componentFactoryResolver,
injectorFactory);
});
}

Expand Down
30 changes: 20 additions & 10 deletions modules/@angular/router/test/apply_redirects.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,8 @@ describe('applyRedirects', () => {
describe('lazy loading', () => {
it('should load config on demand', () => {
const loadedConfig = new LoadedRouterConfig(
[{path: 'b', component: ComponentB}], <any>'stubInjector', <any>'stubFactoryResolver');
[{path: 'b', component: ComponentB}], <any>'stubInjector', <any>'stubFactoryResolver',
<any>'injectorFactory');
const loader = {
load: (injector: any, p: any) => {
if (injector !== 'providedInjector') throw 'Invalid Injector';
Expand Down Expand Up @@ -171,7 +172,8 @@ describe('applyRedirects', () => {

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

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

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

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

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

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

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

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

it('should work with absolute redirects', () => {
const loadedConfig = new LoadedRouterConfig(
[{path: '', component: ComponentB}], <any>'stubInjector', <any>'stubFactoryResolver');
[{path: '', component: ComponentB}], <any>'stubInjector', <any>'stubFactoryResolver',
<any>'injectorFactory');

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

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

it('should load the configuration only once', () => {
const loadedConfig = new LoadedRouterConfig(
[{path: '', component: ComponentB}], <any>'stubInjector', <any>'stubFactoryResolver');
[{path: '', component: ComponentB}], <any>'stubInjector', <any>'stubFactoryResolver',
<any>'injectorFactory');

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

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

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

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

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

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

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

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

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

Expand Down
104 changes: 64 additions & 40 deletions modules/@angular/router/test/integration.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2024,54 +2024,78 @@ describe('Integration', () => {
expect(location.path()).toEqual('/lazy2/loaded');
})));

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

@Component({selector: 'lazy', template: 'lazy-loaded'})
class LazyLoadedChildComponent {
constructor(service: LazyLoadedServiceDefinedInCmp) {}
}
describe('should use the injector of the lazily-loaded configuration', () => {
class LazyLoadedServiceDefinedInModule {}
class LazyLoadedServiceDefinedInCmp {}

@Component({
selector: 'lazy',
template: '<router-outlet></router-outlet>',
providers: [LazyLoadedServiceDefinedInCmp]
})
class LazyLoadedParentComponent {
constructor(service: LazyLoadedServiceDefinedInModule) {}
}
@Component({
selector: 'eager-parent',
template: 'eager-parent <router-outlet></router-outlet>',
})
class EagerParentComponent {
}

@NgModule({
declarations: [LazyLoadedParentComponent, LazyLoadedChildComponent],
imports: [RouterModule.forChild([{
path: '',
children: [{
path: 'loaded',
component: LazyLoadedParentComponent,
children: [{path: 'child', component: LazyLoadedChildComponent}]
}]
}])],
providers: [LazyLoadedServiceDefinedInModule]
})
class LoadedModule {
}
@Component({selector: 'lazy-parent', template: 'lazy-parent <router-outlet></router-outlet>'})
class LazyParentComponent {
}

loader.stubbedModules = {expected: LoadedModule};
@Component({selector: 'lazy-child', template: 'lazy-child'})
class LazyChildComponent {
constructor(
lazy: LazyParentComponent, // should be able to inject lazy/direct parent
lazyService: LazyLoadedServiceDefinedInModule, // should be able to inject lazy service
eager:
EagerParentComponent // should use the injector of the location to create a parent
) {}
}

const fixture = createRoot(router, RootCmp);
@NgModule({
declarations: [LazyParentComponent, LazyChildComponent],
imports: [RouterModule.forChild([{
path: '',
children: [{
path: 'lazy-parent',
component: LazyParentComponent,
children: [{path: 'lazy-child', component: LazyChildComponent}]
}]
}])],
providers: [LazyLoadedServiceDefinedInModule]
})
class LoadedModule {
}

router.resetConfig([{path: 'lazy', loadChildren: 'expected'}]);
@NgModule({
declarations: [EagerParentComponent],
entryComponents: [EagerParentComponent],
imports: [RouterModule]
})
class TestModule {
}

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

expect(location.path()).toEqual('/lazy/loaded/child');
expect(fixture.nativeElement).toHaveText('lazy-loaded');
})));
it('should use the injector of the lazily-loaded configuration',
fakeAsync(inject(
[Router, Location, NgModuleFactoryLoader],
(router: Router, location: Location, loader: SpyNgModuleFactoryLoader) => {
loader.stubbedModules = {expected: LoadedModule};

const fixture = createRoot(router, RootCmp);

router.resetConfig([{
path: 'eager-parent',
component: EagerParentComponent,
children: [{path: 'lazy', loadChildren: 'expected'}]
}]);

router.navigateByUrl('/eager-parent/lazy/lazy-parent/lazy-child');
advance(fixture);

expect(location.path()).toEqual('/eager-parent/lazy/lazy-parent/lazy-child');
expect(fixture.nativeElement).toHaveText('eager-parent lazy-parent lazy-child');
})));
});

it('works when given a callback',
fakeAsync(inject(
Expand Down
4 changes: 3 additions & 1 deletion tools/public_api_guard/router/index.d.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -289,9 +289,11 @@ export declare class RouterOutlet implements OnDestroy {
component: Object;
deactivateEvents: EventEmitter<any>;
isActivated: boolean;
locationFactoryResolver: ComponentFactoryResolver;
locationInjector: Injector;
outletMap: RouterOutletMap;
constructor(parentOutletMap: RouterOutletMap, location: ViewContainerRef, resolver: ComponentFactoryResolver, name: string);
activate(activatedRoute: ActivatedRoute, loadedResolver: ComponentFactoryResolver, loadedInjector: Injector, providers: ResolvedReflectiveProvider[], outletMap: RouterOutletMap): void;
activate(activatedRoute: ActivatedRoute, resolver: ComponentFactoryResolver, injector: Injector, providers: ResolvedReflectiveProvider[], outletMap: RouterOutletMap): void;
deactivate(): void;
ngOnDestroy(): void;
}
Expand Down

0 comments on commit 52be848

Please sign in to comment.