Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(router): incorrect injector is used when instantiating components… #12817

Merged
merged 1 commit into from Nov 12, 2016
Merged

fix(router): incorrect injector is used when instantiating components… #12817

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions modules/@angular/router/src/apply_redirects.ts
View file
Open in desktop
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
Open in desktop
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
Open in desktop
Expand Up @@ -1080,17 +1080,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
Open in desktop
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
Open in desktop
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
Open in desktop
Expand Up @@ -1999,54 +1999,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
Open in desktop
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