Skip to content

Commit

Permalink
fix(router): support outlets within dynamic components
Browse files Browse the repository at this point in the history
Fixes internal b/27294172
  • Loading branch information
btford authored and vikerman committed Mar 3, 2016
1 parent 75343eb commit 7d44b82
Show file tree
Hide file tree
Showing 5 changed files with 142 additions and 25 deletions.
13 changes: 9 additions & 4 deletions modules/angular2/src/router/directives/router_outlet.ts
@@ -1,7 +1,6 @@
import {PromiseWrapper} from 'angular2/src/facade/async';
import {StringMapWrapper} from 'angular2/src/facade/collection';
import {isBlank, isPresent} from 'angular2/src/facade/lang';
import {BaseException, WrappedException} from 'angular2/src/facade/exceptions';

import {
Directive,
Expand All @@ -11,7 +10,8 @@ import {
ElementRef,
Injector,
provide,
Dependency
Dependency,
OnDestroy
} from 'angular2/core';

import * as routerMod from '../router';
Expand All @@ -32,7 +32,7 @@ let _resolveToTrue = PromiseWrapper.resolve(true);
* ```
*/
@Directive({selector: 'router-outlet'})
export class RouterOutlet {
export class RouterOutlet implements OnDestroy {
name: string = null;
private _componentRef: ComponentRef = null;
private _currentInstruction: ComponentInstruction = null;
Expand Down Expand Up @@ -81,8 +81,11 @@ export class RouterOutlet {
var previousInstruction = this._currentInstruction;
this._currentInstruction = nextInstruction;

// it's possible the component is removed before it can be reactivated (if nested withing
// another dynamically loaded component, for instance). In that case, we simply activate
// a new one.
if (isBlank(this._componentRef)) {
throw new BaseException(`Cannot reuse an outlet that does not contain a component.`);
return this.activate(nextInstruction);
}
return PromiseWrapper.resolve(
hasLifecycleHook(hookMod.routerOnReuse, this._currentInstruction.componentType) ?
Expand Down Expand Up @@ -157,4 +160,6 @@ export class RouterOutlet {
}
return PromiseWrapper.resolve(result);
}

ngOnDestroy(): void { this._parentRouter.unregisterPrimaryOutlet(this); }

This comment has been minimized.

Copy link
@klaustietz

klaustietz Mar 11, 2016

Hello,
In my opinion, here must be distinguished whether it is a PrimaryOutlet (has no name) or an AuxOutlet (has a name).
Otherwise if you work with AuxRoute you get an exception like "registerPrimaryOutlet expects to be called with an unnamed outlet" for an AuxRoute, that does not make sense to me.
Kind Regards

}
57 changes: 37 additions & 20 deletions modules/angular2/src/router/router.ts
Expand Up @@ -78,13 +78,30 @@ export class Router {
throw new BaseException(`registerPrimaryOutlet expects to be called with an unnamed outlet.`);
}

if (isPresent(this._outlet)) {
throw new BaseException(`Primary outlet is already registered.`);
}

this._outlet = outlet;
if (isPresent(this._currentInstruction)) {
return this.commit(this._currentInstruction, false);
}
return _resolveToTrue;
}

/**
* Unregister an outlet (because it was destroyed, etc).
*
* You probably don't need to use this unless you're writing a custom outlet implementation.
*/
unregisterPrimaryOutlet(outlet: RouterOutlet): void {
if (isPresent(outlet.name)) {
throw new BaseException(`registerPrimaryOutlet expects to be called with an unnamed outlet.`);
}
this._outlet = null;
}


/**
* Register an outlet to notified of auxiliary route changes.
*
Expand Down Expand Up @@ -198,6 +215,26 @@ export class Router {
});
}

/** @internal */
_settleInstruction(instruction: Instruction): Promise<any> {
return instruction.resolveComponent().then((_) => {
var unsettledInstructions: Array<Promise<any>> = [];

if (isPresent(instruction.component)) {
instruction.component.reuse = false;
}

if (isPresent(instruction.child)) {
unsettledInstructions.push(this._settleInstruction(instruction.child));
}

StringMapWrapper.forEach(instruction.auxInstruction, (instruction, _) => {
unsettledInstructions.push(this._settleInstruction(instruction));
});
return PromiseWrapper.all(unsettledInstructions);
});
}

/** @internal */
_navigate(instruction: Instruction, _skipLocationChange: boolean): Promise<any> {
return this._settleInstruction(instruction)
Expand All @@ -220,26 +257,6 @@ export class Router {
});
}

/** @internal */
_settleInstruction(instruction: Instruction): Promise<any> {
return instruction.resolveComponent().then((_) => {
var unsettledInstructions: Array<Promise<any>> = [];

if (isPresent(instruction.component)) {
instruction.component.reuse = false;
}

if (isPresent(instruction.child)) {
unsettledInstructions.push(this._settleInstruction(instruction.child));
}

StringMapWrapper.forEach(instruction.auxInstruction, (instruction, _) => {
unsettledInstructions.push(this._settleInstruction(instruction));
});
return PromiseWrapper.all(unsettledInstructions);
});
}

private _emitNavigationFinish(url): void { ObservableWrapper.callEmit(this._subject, url); }

private _afterPromiseFinishNavigating(promise: Promise<any>): Promise<any> {
Expand Down
Expand Up @@ -9,6 +9,12 @@ import {
ROUTER_DIRECTIVES
} from 'angular2/router';
import {PromiseWrapper} from 'angular2/src/facade/async';
import {isPresent} from 'angular2/src/facade/lang';
import {
DynamicComponentLoader,
ComponentRef
} from 'angular2/src/core/linker/dynamic_component_loader';
import {ElementRef} from 'angular2/src/core/linker/element_ref';

@Component({selector: 'goodbye-cmp', template: `{{farewell}}`})
export class GoodbyeCmp {
Expand Down Expand Up @@ -135,3 +141,31 @@ export function asyncRouteDataCmp() {
@RouteConfig([new Redirect({path: '/child-redirect', redirectTo: ['../HelloSib']})])
export class RedirectToParentCmp {
}


@Component({selector: 'dynamic-loader-cmp', template: `{ <div #viewport></div> }`})
@RouteConfig([new Route({path: '/', component: HelloCmp})])
export class DynamicLoaderCmp {
private _componentRef: ComponentRef = null;
constructor(private _dynamicComponentLoader: DynamicComponentLoader,
private _elementRef: ElementRef) {}

onSomeAction(): Promise<any> {
if (isPresent(this._componentRef)) {
this._componentRef.dispose();
this._componentRef = null;
}
return this._dynamicComponentLoader.loadIntoLocation(DynamicallyLoadedComponent,
this._elementRef, 'viewport')
.then((cmp) => { this._componentRef = cmp; });
}
}


@Component({
selector: 'loaded-cmp',
template: '<router-outlet></router-outlet>',
directives: [ROUTER_DIRECTIVES]
})
class DynamicallyLoadedComponent {
}
Expand Up @@ -17,7 +17,16 @@ import {specs, compile, TEST_ROUTER_PROVIDERS, clickOnElement, getHref} from '..
import {By} from 'angular2/platform/common_dom';
import {Router, Route, Location} from 'angular2/router';

import {HelloCmp, UserCmp, TeamCmp, ParentCmp, ParentWithDefaultCmp} from './fixture_components';
import {
HelloCmp,
UserCmp,
TeamCmp,
ParentCmp,
ParentWithDefaultCmp,
DynamicLoaderCmp
} from './fixture_components';

import {PromiseWrapper} from 'angular2/src/facade/async';


function getLinkElement(rtc: ComponentFixture) {
Expand Down Expand Up @@ -420,6 +429,55 @@ function syncRoutesWithSyncChildrenWithDefaultRoutesWithoutParams() {
}));
}

function syncRoutesWithDynamicComponents() {
var fixture;
var tcb;
var rtr: Router;

beforeEachProviders(() => TEST_ROUTER_PROVIDERS);

beforeEach(inject([TestComponentBuilder, Router], (tcBuilder, router) => {
tcb = tcBuilder;
rtr = router;
}));


it('should work',
inject([AsyncTestCompleter],
(async) => {tcb.createAsync(DynamicLoaderCmp)
.then((rtc) => {fixture = rtc})
.then((_) => rtr.config([new Route({path: '/', component: HelloCmp})]))
.then((_) => {
fixture.detectChanges();
expect(fixture.debugElement.nativeElement).toHaveText('{ }');
return fixture.componentInstance.onSomeAction();
})
.then((_) => {
fixture.detectChanges();
return rtr.navigateByUrl('/');
})
.then((_) => {
fixture.detectChanges();
expect(fixture.debugElement.nativeElement).toHaveText('{ hello }');

return fixture.componentInstance.onSomeAction();
})
.then((_) => {

// TODO(i): This should be rewritten to use NgZone#onStable or
// something
// similar basically the assertion needs to run when the world is
// stable and we don't know when that is, only zones know.
PromiseWrapper.resolve(null).then((_) => {
fixture.detectChanges();
expect(fixture.debugElement.nativeElement).toHaveText('{ hello }');
async.done();
});
})}));
}



export function registerSpecs() {
specs['syncRoutesWithoutChildrenWithoutParams'] = syncRoutesWithoutChildrenWithoutParams;
specs['syncRoutesWithoutChildrenWithParams'] = syncRoutesWithoutChildrenWithParams;
Expand All @@ -429,4 +487,5 @@ export function registerSpecs() {
syncRoutesWithSyncChildrenWithoutDefaultRoutesWithParams;
specs['syncRoutesWithSyncChildrenWithDefaultRoutesWithoutParams'] =
syncRoutesWithSyncChildrenWithDefaultRoutesWithoutParams;
specs['syncRoutesWithDynamicComponents'] = syncRoutesWithDynamicComponents;
}
2 changes: 2 additions & 0 deletions modules/angular2/test/router/integration/sync_route_spec.ts
Expand Up @@ -20,5 +20,7 @@ export function main() {
describeWith('default routes', () => { describeWithout('params', itShouldRoute); });

});

describeWith('dynamic components', itShouldRoute);
});
}

0 comments on commit 7d44b82

Please sign in to comment.