From 8662d651deeef91a63fc572233cc552bc80d7d19 Mon Sep 17 00:00:00 2001 From: Mike Ryan Date: Tue, 3 May 2016 10:38:33 -0600 Subject: [PATCH] refactor(RouteTraverser): Expand traverser to also track query params and location changes Traverser now accepts a LocationChange instead of a pathname. Returned Match now includes the LocationChange and the query params. RouterInstruction is no longer responsible for parsing query params. Guards now receive query params and the location change as a result of this refactoring. BREAKING CHANGE: TraversalCandidate interface has renamed `params` to `routeParams` and now includes queryParams and locationChange. Before: ```ts const { params, isTerminal, route } = traversalCandidate; ``` After: ```ts const { routeParams, queryParams, isTerminal, route, locationChange } = traversalCandidate; ``` --- docs/overview/guards.md | 23 ++++++++--- lib/index.ts | 6 +-- lib/redirect.ts | 15 +++---- lib/route-traverser.ts | 46 +++++++++++++++------- lib/route-view.ts | 9 +++-- lib/router-instruction.ts | 27 ++----------- spec/guard.spec.ts | 43 +++++++++++++------- spec/params.spec.ts | 9 +++-- spec/redirect.spec.ts | 20 +++++----- spec/route-traverser.spec.ts | 69 +++++++++++++++++++++------------ spec/route-view.spec.ts | 35 +++++++++-------- spec/router-instruction.spec.ts | 25 +++++------- 12 files changed, 187 insertions(+), 140 deletions(-) diff --git a/docs/overview/guards.md b/docs/overview/guards.md index 4200f54..6c6e877 100644 --- a/docs/overview/guards.md +++ b/docs/overview/guards.md @@ -19,15 +19,28 @@ import 'rxjs/add/operator/map'; import { Injectable } from '@angular/core'; import { Http } from '@angular/http'; import { Observable } from 'rxjs/Observable'; -import { Guard, Route, TraversalCandidate } from '@ngrx/router'; +import { Guard, Route, TraversalCandidate, LocationChange } from '@ngrx/router'; @Injectable() class AuthGuard implements Guard { constructor(private _http: Http) { } - // Guards are provided with a traversal candidate object which contains a - // snapshot of the route params parsed so far, the parsed query params, - // the route being evaluated, and the location change that caused traversal. - protectRoute(candidate: TraversalCandidate) { + + protectRoute(candidate: TraversalCandidate) { + // `route` is the current route being evaluated + const route: Route = candidate.route; + + // `locationChange` includes the full path and type of change that caused traversal + const locationChange: LocationChange = candidate.locationChange; + + // `queryParams` are the parsed query parameters + const queryParams: any = candidate.queryParams; + + // `routeParams` is a snapshot of the route parameters discovered so far + const routeParams: any = candidate.routeParams; + + // `isTerminal` indicates that the candidate route is going to be the last route traversed + const isTerminal: boolean = candidate.isTerminal; + return this._http.get('/auth/check') // If request succeeds, return true .map(() => true) diff --git a/lib/index.ts b/lib/index.ts index 6d35d00..3912963 100644 --- a/lib/index.ts +++ b/lib/index.ts @@ -53,13 +53,13 @@ export function provideRouter(routes: Routes, locationStrategy: Type = PathLocat } -export { Guard, provideGuard } from './guard'; +export { Guard } from './guard'; export { LocationChange, Router } from './router'; export { RouteParams, QueryParams } from './params'; -export { ROUTER_HOOKS, INSTRUCTION_HOOKS, RouterInstruction, NextInstruction } from './router-instruction'; +export { ROUTER_HOOKS, INSTRUCTION_HOOKS, RouterInstruction } from './router-instruction'; export { PRE_RENDER_HOOKS, POST_RENDER_HOOKS, RenderInstruction } from './component-renderer'; export { Routes, Route, IndexRoute } from './route'; -export { TRAVERSAL_HOOKS, TraversalCandidate } from './route-traverser'; +export { TRAVERSAL_HOOKS, TraversalCandidate, Match } from './route-traverser'; export { LinkTo } from './link-to'; export { LinkActive, LinkActiveOptions } from './link-active'; export { RouteView } from './route-view'; diff --git a/lib/redirect.ts b/lib/redirect.ts index 33ba8ec..37caaa7 100644 --- a/lib/redirect.ts +++ b/lib/redirect.ts @@ -8,19 +8,20 @@ import { Provider, Injectable } from '@angular/core'; import { Router } from './router'; import { Routes, Route } from './route'; -import { INSTRUCTION_HOOKS, NextInstruction } from './router-instruction'; +import { INSTRUCTION_HOOKS } from './router-instruction'; +import { Match } from './route-traverser'; import { formatPattern } from './match-pattern'; import { Hook } from './hooks'; @Injectable() -export class RedirectHook implements Hook { +export class RedirectHook implements Hook { constructor(private router: Router) { } - apply(next$: Observable): Observable { + apply(next$: Observable): Observable { return next$ .filter(next => { - const last = next.routeConfigs[next.routeConfigs.length - 1]; + const last = next.routes[next.routes.length - 1]; if (last.redirectTo) { this._handleRedirect(last, next); @@ -31,7 +32,7 @@ export class RedirectHook implements Hook { }); } - private _handleRedirect(route: Route, next: NextInstruction) { + private _handleRedirect(route: Route, next: Match) { const { routeParams, queryParams } = next; let pathname; @@ -40,8 +41,8 @@ export class RedirectHook implements Hook { pathname = formatPattern(route.redirectTo, routeParams); } else { - const routeIndex = next.routeConfigs.indexOf(route); - const parentPattern = this._getRoutePattern(next.routeConfigs, routeIndex - 1); + const routeIndex = next.routes.indexOf(route); + const parentPattern = this._getRoutePattern(next.routes, routeIndex - 1); const pattern = parentPattern.replace(/\/*$/, '/') + route.redirectTo; pathname = formatPattern(pattern, routeParams); } diff --git a/lib/route-traverser.ts b/lib/route-traverser.ts index 84ed99f..c0f0a15 100644 --- a/lib/route-traverser.ts +++ b/lib/route-traverser.ts @@ -13,11 +13,13 @@ import 'rxjs/add/operator/concatMap'; import 'rxjs/add/operator/take'; import { Observable } from 'rxjs/Observable'; import { OpaqueToken, Provider, Inject, Injectable, Optional } from '@angular/core'; +import * as queryString from 'query-string'; import { ResourceLoader, Async } from './resource-loader'; import { matchPattern, makeParams } from './match-pattern'; import { Route, IndexRoute, Routes, ROUTES } from './route'; import { Hook, composeHooks } from './hooks'; +import { LocationChange } from './router'; export const TRAVERSAL_HOOKS = new OpaqueToken( '@ngrx/router Traversal Hooks' @@ -25,12 +27,16 @@ export const TRAVERSAL_HOOKS = new OpaqueToken( export interface Match { routes: Routes; - params: any; + routeParams: any; + queryParams: any; + locationChange: LocationChange; }; export interface TraversalCandidate { route: Route; - params: any; + routeParams: any; + queryParams: any; + locationChange: LocationChange; isTerminal: boolean; } @@ -51,24 +57,30 @@ export class RouteTraverser { * - routes An array of routes that matched, in hierarchical order * - params An object of URL parameters */ - find(pathname: string) { - return this._matchRoutes(this._routes, pathname); + find(change: LocationChange) { + const [ pathname, query ] = change.path.split('?'); + const queryParams = queryString.parse(query); + return this._matchRoutes(queryParams, change, pathname); } private _matchRoutes( - routes: Routes, + queryParams: any, + locationChange: LocationChange, pathname: string, remainingPathname = pathname, - paramNames = [], - paramValues = [] + routes: Routes = this._routes, + routeParamNames = [], + routeParamValues = [] ): Observable { return Observable.from(routes) .concatMap(route => this._matchRouteDeep( route, + queryParams, + locationChange, pathname, remainingPathname, - paramNames, - paramValues + routeParamNames, + routeParamValues )) .catch(error => { console.error('Error During Traversal', error); @@ -80,6 +92,8 @@ export class RouteTraverser { private _matchRouteDeep( route: Route, + queryParams: any, + locationChange: LocationChange, pathname: string, remainingPathname: string, paramNames: string[], @@ -105,17 +119,21 @@ export class RouteTraverser { .map(() => { return { route, - params: makeParams(paramNames, paramValues), + queryParams, + locationChange, + routeParams: makeParams(paramNames, paramValues), isTerminal: remainingPathname === '' && !!route.path }; }) .let(composeHooks(this._hooks)) .filter(({ route }) => !!route) - .mergeMap(({ route, params, isTerminal }) => { + .mergeMap(({ route, routeParams, queryParams, isTerminal }) => { if ( isTerminal ) { const match: Match = { routes: [ route ], - params + routeParams, + queryParams, + locationChange }; return Observable.of(route) @@ -132,9 +150,11 @@ export class RouteTraverser { return Observable.of(route) .mergeMap(route => this._loadChildRoutes(route)) .mergeMap(childRoutes => this._matchRoutes( - childRoutes, + queryParams, + locationChange, pathname, remainingPathname, + childRoutes, paramNames, paramValues )) diff --git a/lib/route-view.ts b/lib/route-view.ts index 57942d0..effa556 100644 --- a/lib/route-view.ts +++ b/lib/route-view.ts @@ -24,8 +24,9 @@ import { } from '@angular/core'; import { Route, getNamedComponents } from './route'; -import { RouterInstruction, NextInstruction } from './router-instruction'; +import { RouterInstruction } from './router-instruction'; import { ComponentRenderer } from './component-renderer'; +import { Match } from './route-traverser'; @Component({ selector: 'route-view', @@ -36,10 +37,10 @@ export class RouteView implements OnDestroy, OnInit { private _prev: ComponentRef; private _sub: any; private _routerInstructionProvider = new Provider(RouterInstruction, { - useValue: this._routerInstruction$.map(set => { + useValue: this._routerInstruction$.map(set => { return { locationChange: set.locationChange, - routeConfigs: [ ...set.routeConfigs ].slice(1), + routes: [ ...set.routes ].slice(1), routeParams: set.routeParams, queryParams: set.queryParams }; @@ -58,7 +59,7 @@ export class RouteView implements OnDestroy, OnInit { ngOnInit() { this._sub = this._routerInstruction$ .map(set => { - const route = set.routeConfigs[0]; + const route = set.routes[0]; const components = getNamedComponents(route, this._name); return { route, components }; diff --git a/lib/router-instruction.ts b/lib/router-instruction.ts index ae4b698..d13318c 100644 --- a/lib/router-instruction.ts +++ b/lib/router-instruction.ts @@ -22,26 +22,19 @@ import { Hook, composeHooks } from './hooks'; export const ROUTER_HOOKS = new OpaqueToken('@ngrx/router Router Hooks'); export const INSTRUCTION_HOOKS = new OpaqueToken('@ngrx/router Instruction Hooks'); -export interface NextInstruction { - routeConfigs: Routes; - routeParams: any; - queryParams: any; - locationChange: LocationChange; -} - -export abstract class RouterInstruction extends Observable { } +export abstract class RouterInstruction extends Observable { } @Injectable() export class RouterInstructionFactory { constructor( private _router$: Router, private _traverser: RouteTraverser, - private _zoneOperator: ZoneOperator, + private _zoneOperator: ZoneOperator, @Optional() @Inject(ROUTER_HOOKS) private _routerHooks: Hook[] = [], @Optional() @Inject(INSTRUCTION_HOOKS) - private _instructionHooks: Hook[] = [] + private _instructionHooks: Hook[] = [] ) { } create(): RouterInstruction { @@ -49,19 +42,7 @@ export class RouterInstructionFactory { .observeOn(asap) .distinctUntilChanged((prev, next) => prev.path === next.path) .let(composeHooks(this._routerHooks)) - .switchMap(change => { - const [ pathname, queryString ] = change.path.split('?'); - - return this._traverser.find(pathname) - .map(set => { - return { - locationChange: change, - routeConfigs: set.routes, - queryParams: parseQueryString(queryString), - routeParams: set.params - }; - }); - }) + .switchMap(change => this._traverser.find(change)) .filter(match => !!match) .let(composeHooks(this._instructionHooks)) .lift(this._zoneOperator) diff --git a/spec/guard.spec.ts b/spec/guard.spec.ts index ffcaffd..d4597a3 100644 --- a/spec/guard.spec.ts +++ b/spec/guard.spec.ts @@ -19,8 +19,14 @@ describe('Guard Middleware', function() { protectRoute = () => Observable.of(false); } - function route(route: Route, params = {}, isTerminal = false) { - return Observable.of({ route, params, isTerminal }); + function route( + route: Route, + routeParams = {}, + queryParams = {}, + isTerminal = false, + locationChange = { type: 'push', path: '/' } + ): Observable { + return Observable.of({ route, routeParams, queryParams, isTerminal, locationChange }); } beforeEach(function() { @@ -56,19 +62,26 @@ describe('Guard Middleware', function() { expect(injector.resolveAndInstantiate).toHaveBeenCalledWith(PassGuard); }); - // Intentionally commenting this out because a future PR will refactor - // traversal candidate and will pass that to the guards instead - xit('should provide guards with the TraversalCandidate', function() { - // const testGuard = { run: () => Observable.of(true) }; - // spyOn(testGuard, 'run').and.callThrough(); - // const guard = provideGuard(() => testGuard.run); - // const nextRoute = { guards: [ guard ] }; - // const params = { abc: 123 }; - // const isTerminal = true; - // - // route(nextRoute, params, isTerminal).let(t => guardHook.apply(t)).subscribe(); - // - // expect(testGuard.run).toHaveBeenCalledWith(params, nextRoute, isTerminal); + + it('should provide guards with the TraversalCandidate', function() { + const spy = jasmine.createSpy('protectRoute').and.returnValue(Observable.of(true)); + class MockGuard { + protectRoute = spy; + } + + const traversalCandidate: TraversalCandidate = { + route: { path: '/', guards: [ MockGuard ] }, + queryParams: {}, + routeParams: {}, + locationChange: { type: 'push', path: '/' }, + isTerminal: true + }; + + const stream$ = Observable.of(traversalCandidate); + + stream$.let(t => guardHook.apply(t)).subscribe(); + + expect(spy).toHaveBeenCalledWith(traversalCandidate); }); it('should return true if all of the guards return true', function(done) { diff --git a/spec/params.spec.ts b/spec/params.spec.ts index 2eaf0ad..d8083b6 100644 --- a/spec/params.spec.ts +++ b/spec/params.spec.ts @@ -1,18 +1,19 @@ import { Subject } from 'rxjs/Subject'; import { ReflectiveInjector, provide } from '@angular/core'; -import { NextInstruction, RouterInstruction } from '../lib/router-instruction'; +import { RouterInstruction } from '../lib/router-instruction'; +import { Match } from '../lib/route-traverser'; import { RouteParams, QueryParams, PARAMS_PROVIDERS } from '../lib/params'; describe('Params Services', function() { - let routerInstruction$: Subject; + let routerInstruction$: Subject; let routeParams$: RouteParams; let queryParams$: QueryParams; function nextInstruction(routeParams, queryParams) { routerInstruction$.next({ - routeConfigs: [], + routes: [], routeParams, queryParams, locationChange: { @@ -23,7 +24,7 @@ describe('Params Services', function() { } beforeEach(function() { - routerInstruction$ = new Subject(); + routerInstruction$ = new Subject(); const injector = ReflectiveInjector.resolveAndCreate([ PARAMS_PROVIDERS, provide(RouterInstruction, { useValue: routerInstruction$ }) diff --git a/spec/redirect.spec.ts b/spec/redirect.spec.ts index 49e08b2..e5ed87b 100644 --- a/spec/redirect.spec.ts +++ b/spec/redirect.spec.ts @@ -1,19 +1,19 @@ import { Subject } from 'rxjs/Subject'; import { ReflectiveInjector, provide } from '@angular/core'; -import { NextInstruction } from '../lib/router-instruction'; +import { Match } from '../lib/route-traverser'; import { RedirectHook } from '../lib/redirect'; import { Router } from '../lib/router'; describe('Redirect Middleware', function() { let redirect: RedirectHook; - let routeSet$: Subject; + let routeSet$: Subject; let router = { replace(next: string) { } }; let observer = { next() { } }; function nextInstruction(to: string, routeParams: any = {}, queryParams: any = {}) { routeSet$.next({ - routeConfigs: [ { redirectTo: to } ], + routes: [ { redirectTo: to } ], routeParams, queryParams, locationChange: { @@ -24,7 +24,7 @@ describe('Redirect Middleware', function() { } beforeEach(function() { - routeSet$ = new Subject(); + routeSet$ = new Subject(); spyOn(router, 'replace'); spyOn(observer, 'next'); const injector = ReflectiveInjector.resolveAndCreate([ @@ -39,8 +39,8 @@ describe('Redirect Middleware', function() { }); it('should skip route sets that are not redirects', function(done) { - const NextInstruction: NextInstruction = { - routeConfigs: [ { path: '/first' } ], + const nextInstruction: Match = { + routes: [ { path: '/first' } ], routeParams: {}, queryParams: {}, locationChange: { @@ -50,11 +50,11 @@ describe('Redirect Middleware', function() { }; redirect.apply(routeSet$).subscribe(value => { - expect(value).toBe(NextInstruction); + expect(value).toBe(nextInstruction); done(); }); - routeSet$.next(NextInstruction); + routeSet$.next(nextInstruction); }); it('should correctly redirect basic paths', function() { @@ -79,7 +79,7 @@ describe('Redirect Middleware', function() { redirect.apply(routeSet$).subscribe(observer); routeSet$.next({ - routeConfigs: [ { path: '/first' }, { path: 'second', redirectTo: '/home' } ], + routes: [ { path: '/first' }, { path: 'second', redirectTo: '/home' } ], routeParams: {}, queryParams: {}, locationChange: { @@ -96,7 +96,7 @@ describe('Redirect Middleware', function() { redirect.apply(routeSet$).subscribe(observer); routeSet$.next({ - routeConfigs: [ { path: '/blog' }, { path: ':id', redirectTo: '/posts/:id' } ], + routes: [ { path: '/blog' }, { path: ':id', redirectTo: '/posts/:id' } ], routeParams: { id: '543' }, queryParams: {}, locationChange: { diff --git a/spec/route-traverser.spec.ts b/spec/route-traverser.spec.ts index 8d12b8b..75b05a1 100644 --- a/spec/route-traverser.spec.ts +++ b/spec/route-traverser.spec.ts @@ -1,5 +1,6 @@ import { ReflectiveInjector, provide } from '@angular/core'; +import { LocationChange } from '../lib/router'; import { RESOURCE_LOADER_PROVIDERS } from '../lib/resource-loader'; import { Routes, Route, ROUTES } from '../lib/route'; import { RouteTraverser, MATCH_ROUTE_PROVIDERS } from '../lib/route-traverser'; @@ -92,6 +93,13 @@ describe('RouteTraverser', function() { } ]; + function change(path: string): LocationChange { + return { + type: 'push', + path + }; + } + beforeEach(function() { const injector = ReflectiveInjector.resolveAndCreate([ MATCH_ROUTE_PROVIDERS, @@ -105,7 +113,7 @@ describe('RouteTraverser', function() { describe('when the location matches an index route', function() { it('matches the correct routes', function(done) { traverser - .find('/users') + .find(change('/users')) .subscribe(match => { expect(match).toBeDefined(); expect(match.routes).toEqual([ RootRoute, UsersRoute, UsersIndexRoute ]); @@ -118,11 +126,11 @@ describe('RouteTraverser', function() { describe('when the location matches a nested route with params', function() { it('matches the correct routes and params', function(done) { traverser - .find('/users/5') + .find(change('/users/5')) .subscribe(match => { expect(match).toBeDefined(); expect(match.routes).toEqual([ RootRoute, UsersRoute, UserRoute ]); - expect(match.params).toEqual({ userID: '5' }); + expect(match.routeParams).toEqual({ userID: '5' }); done(); }); @@ -132,11 +140,11 @@ describe('RouteTraverser', function() { describe('when the location matches a deeply nested route with params', function() { it('matches the correct routes and params', function(done) { traverser - .find('/users/5/abc') + .find(change('/users/5/abc')) .subscribe(match => { expect(match).toBeDefined(); expect(match.routes).toEqual([ RootRoute, UsersRoute, UserRoute, PostRoute ]); - expect(match.params).toEqual({ userID: '5', postID: 'abc' }); + expect(match.routeParams).toEqual({ userID: '5', postID: 'abc' }); done(); }); @@ -146,11 +154,11 @@ describe('RouteTraverser', function() { describe('when the location matches a nested route with multiple splat params', function() { it('matches the correct routes and params', function(done) { traverser - .find('/files/a/b/c.jpg') + .find(change('/files/a/b/c.jpg')) .subscribe(match => { expect(match).toBeDefined(); expect(match.routes).toEqual([ FilesRoute ]); - expect(match.params).toEqual({ 0: 'a/b', 1: 'c' }); + expect(match.routeParams).toEqual({ 0: 'a/b', 1: 'c' }); done(); }); @@ -160,11 +168,11 @@ describe('RouteTraverser', function() { describe('when the location matches a nested route with a greedy splat param', function() { it('matches the correct routes and params', function(done) { traverser - .find('/foo/bar/f') + .find(change('/foo/bar/f')) .subscribe(match => { expect(match).toBeDefined(); expect(match.routes).toEqual([ GreedyRoute ]); - expect(match.params).toEqual({ 0: 'foo/bar' }); + expect(match.routeParams).toEqual({ 0: 'foo/bar' }); done(); }); @@ -174,7 +182,7 @@ describe('RouteTraverser', function() { describe('when the location matches an absolute route', function() { it('matches the correct routes', function(done) { traverser - .find('/about') + .find(change('/about')) .subscribe(match => { expect(match).toBeDefined(); expect(match.routes).toEqual([ AboutRoute ]); @@ -187,7 +195,7 @@ describe('RouteTraverser', function() { describe('when the location matches an optional route', function() { it('matches the the optional pattern is missing', function(done) { traverser - .find('/') + .find(change('/')) .subscribe(match => { expect(match).toBeDefined(); expect(match.routes).toEqual([ OptionalRoute ]); @@ -198,7 +206,7 @@ describe('RouteTraverser', function() { it('matches the the optional pattern is present', function(done) { traverser - .find('/optional') + .find(change('/optional')) .subscribe(match => { expect(match).toBeDefined(); expect(match.routes).toEqual([ OptionalRoute ]); @@ -211,7 +219,7 @@ describe('RouteTraverser', function() { describe('when the location matches the child of an optional route', function() { it('matches when the optional pattern is missing', function(done) { traverser - .find('/child') + .find(change('/child')) .subscribe(match => { expect(match).toBeDefined(); expect(match.routes).toEqual([ OptionalRoute, OptionalRouteChild ]); @@ -222,7 +230,7 @@ describe('RouteTraverser', function() { it('matches when the optional pattern is present', function(done) { traverser - .find('/optional/child') + .find(change('/optional/child')) .subscribe(match => { expect(match).toBeDefined(); expect(match.routes).toEqual([ OptionalRoute, OptionalRouteChild ]); @@ -235,7 +243,7 @@ describe('RouteTraverser', function() { describe('when the location does not match any routes', function() { it('matches the "catch-all" route', function(done) { traverser - .find('/not-found') + .find(change('/not-found')) .subscribe(match => { expect(match).toBeDefined(); expect(match.routes).toEqual([ CatchAllRoute ]); @@ -246,7 +254,7 @@ describe('RouteTraverser', function() { it('matches the "catch-all" route on a deep miss', function(done) { traverser - .find('/not-found/foo') + .find(change('/not-found/foo')) .subscribe(match => { expect(match).toBeDefined(); expect(match.routes).toEqual([ CatchAllRoute ]); @@ -257,7 +265,7 @@ describe('RouteTraverser', function() { it('matches the "catch-all" route on missing path separators', function(done) { traverser - .find('/optionalchild') + .find(change('/optionalchild')) .subscribe(match => { expect(match).toBeDefined(); expect(match.routes).toEqual([ CatchAllRoute ]); @@ -268,7 +276,7 @@ describe('RouteTraverser', function() { it('matches the "catch-all" route on a regex miss', function(done) { traverser - .find('/int/foo') + .find(change('/int/foo')) .subscribe(match => { expect(match).toBeDefined(); expect(match.routes).toEqual([ CatchAllRoute ]); @@ -281,11 +289,11 @@ describe('RouteTraverser', function() { describe('when the location matches a route with param regex', function() { it('matches the correct routes and param', function(done) { traverser - .find('/int/42') + .find(change('/int/42')) .subscribe(match => { expect(match).toBeDefined(); expect(match.routes).toEqual([ RegexRoute ]); - expect(match.params).toEqual({ int: '42' }); + expect(match.routeParams).toEqual({ int: '42' }); done(); }); @@ -295,11 +303,11 @@ describe('RouteTraverser', function() { describe('when the location matches a nested route with an unnamed param', function() { it('matches the correct routes and params', function(done) { traverser - .find('/unnamed-params/foo/bar') + .find(change('/unnamed-params/foo/bar')) .subscribe(match => { expect(match).toBeDefined(); expect(match.routes).toEqual([ UnnamedParamsRoute, UnnamedParamsRouteChild ]); - expect(match.params).toEqual({ 0: 'foo', 1: 'bar' }); + expect(match.routeParams).toEqual({ 0: 'foo', 1: 'bar' }); done(); }); @@ -309,7 +317,7 @@ describe('RouteTraverser', function() { describe('when the location matches pathless routes', function() { it('matches the correct routes', function(done) { traverser - .find('/pathless-child') + .find(change('/pathless-child')) .subscribe(match => { expect(match).toBeDefined(); expect(match.routes).toEqual([ @@ -321,6 +329,19 @@ describe('RouteTraverser', function() { }); }); }); + + describe('when the location change includes query params', function() { + it('should correctly parse the query params', function(done) { + traverser + .find(change('/users?a=2')) + .subscribe(({ queryParams }) => { + expect(queryParams).toBeDefined(); + expect(queryParams.a).toBe('2'); + + done(); + }); + }); + }); } describe('synchronous route config', function() { @@ -329,7 +350,7 @@ describe('RouteTraverser', function() { xdescribe('when the location matches a nested absolute route', function() { it('matches the correct routes', function(done) { traverser - .find('/team') + .find(change('/team')) .subscribe(match => { expect(match).toBeDefined(); expect(match.routes).toEqual([ RootRoute, UsersRoute, TeamRoute ]); diff --git a/spec/route-view.spec.ts b/spec/route-view.spec.ts index 0fb4069..2f7d5e7 100644 --- a/spec/route-view.spec.ts +++ b/spec/route-view.spec.ts @@ -14,7 +14,8 @@ import { Observable } from 'rxjs/Observable'; import { RouteView } from '../lib/route-view'; import { ComponentRenderer } from '../lib/component-renderer'; -import { RouterInstruction, NextInstruction } from '../lib/router-instruction'; +import { RouterInstruction } from '../lib/router-instruction'; +import { Match } from '../lib/route-traverser'; class TestRoute {} @@ -26,7 +27,7 @@ class MockRenderer { ); } -class MockRouterInstruction extends BehaviorSubject {} +class MockRouterInstruction extends BehaviorSubject {} const compile = (tcb: TestComponentBuilder) => { return tcb @@ -56,7 +57,7 @@ describe('Route View', () => { it('should not render if missing a route component and component loader', async(inject([TestComponentBuilder, RouterInstruction], (tcb, rs) => { rs.next({ - routeConfigs: [{ + routes: [{ component: undefined, loadComponent: undefined }] @@ -73,7 +74,7 @@ describe('Route View', () => { it('should render with a component', async(inject([TestComponentBuilder, RouterInstruction], (tcb, rs) => { rs.next({ - routeConfigs: [{ + routes: [{ component: TestRoute }] }); @@ -89,7 +90,7 @@ describe('Route View', () => { it('should render with a named component', async(inject([TestComponentBuilder, RouterInstruction], (tcb, rs) => { rs.next({ - routeConfigs: [{ + routes: [{ components: { main: TestRoute } @@ -108,7 +109,7 @@ describe('Route View', () => { it('should render with a component loader', async(inject([TestComponentBuilder, RouterInstruction], (tcb, rs) => { rs.next({ - routeConfigs: [{ + routes: [{ loadComponent: () => Promise.resolve(TestRoute) }] }); @@ -124,7 +125,7 @@ describe('Route View', () => { it('should destroy the previous component on when the route set changes', async(inject([TestComponentBuilder, RouterInstruction], (tcb, rs) => { rs.next({ - routeConfigs: [{ + routes: [{ component: TestRoute }] }); @@ -135,7 +136,7 @@ describe('Route View', () => { instance.ngOnInit(); rs.next({ - routeConfigs: [{ + routes: [{ loadComponent: () => TestRoute2 }] }); @@ -148,7 +149,7 @@ describe('Route View', () => { it('should not destroy if the component doesn\'t change when the route set changes', async(inject([TestComponentBuilder, RouterInstruction], (tcb, rs) => { rs.next({ - routeConfigs: [{ + routes: [{ component: TestRoute }] }); @@ -159,7 +160,7 @@ describe('Route View', () => { instance.ngOnInit(); rs.next({ - routeConfigs: [{ + routes: [{ component: TestRoute }] }); @@ -171,7 +172,7 @@ describe('Route View', () => { it('should not destroy if the component doesn\'t change when the route set changes with named routes', async(inject([TestComponentBuilder, RouterInstruction], (tcb, rs) => { rs.next({ - routeConfigs: [{ + routes: [{ components: { test: TestRoute } @@ -185,7 +186,7 @@ describe('Route View', () => { instance.ngOnInit(); rs.next({ - routeConfigs: [{ + routes: [{ components: { test: TestRoute } @@ -199,7 +200,7 @@ describe('Route View', () => { it('should destroy if the component does change when the route set changes', async(inject([TestComponentBuilder, RouterInstruction], (tcb, rs) => { rs.next({ - routeConfigs: [{ + routes: [{ component: TestRoute }] }); @@ -210,7 +211,7 @@ describe('Route View', () => { instance.ngOnInit(); rs.next({ - routeConfigs: [{ + routes: [{ component: TestRoute2 }] }); @@ -222,7 +223,7 @@ describe('Route View', () => { it('should destroy if the component does change when the route set changes with named routes', async(inject([TestComponentBuilder, RouterInstruction], (tcb, rs) => { rs.next({ - routeConfigs: [{ + routes: [{ components: { main: TestRoute } @@ -236,7 +237,7 @@ describe('Route View', () => { instance.ngOnInit(); rs.next({ - routeConfigs: [{ + routes: [{ components: { main: TestRoute2 } @@ -250,7 +251,7 @@ describe('Route View', () => { it('should destroy the component when destroyed', async(inject([TestComponentBuilder, RouterInstruction], (tcb, rs) => { rs.next({ - routeConfigs: [{ + routes: [{ component: TestRoute }] }); diff --git a/spec/router-instruction.spec.ts b/spec/router-instruction.spec.ts index e8f33d7..433188c 100644 --- a/spec/router-instruction.spec.ts +++ b/spec/router-instruction.spec.ts @@ -7,8 +7,9 @@ import { MockLocationStrategy } from '@angular/common/testing'; import { RouteTraverser } from '../lib/route-traverser'; import { Router, ROUTER_PROVIDERS } from '../lib/router'; -import { NextInstruction, RouterInstruction, ROUTER_INSTRUCTION_PROVIDERS } from '../lib/router-instruction'; +import { RouterInstruction, ROUTER_INSTRUCTION_PROVIDERS } from '../lib/router-instruction'; import { ZONE_OPERATOR_PROVIDERS } from '../lib/zone'; +import { Match } from '../lib/route-traverser'; describe('Router Instruction', function() { @@ -22,8 +23,13 @@ describe('Router Instruction', function() { beforeEach(function() { mockTraverser = { - find() { - return Observable.of({ routes, params }); + find(change) { + return Observable.of({ + routes, + routeParams: params, + queryParams: {}, + locationChange: change + }); } }; @@ -55,7 +61,7 @@ describe('Router Instruction', function() { routerInstruction .withLatestFrom(router) .subscribe(([set, change]) => { - expect(set.routeConfigs).toBe(routes); + expect(set.routes).toBe(routes); expect(set.routeParams).toBe(params); expect(set.queryParams).toBeDefined(); expect(set.locationChange).toBe(change); @@ -65,17 +71,6 @@ describe('Router Instruction', function() { }); - it('should parse a location change with query params', function(done) { - router.go('/test?a=2'); - - routerInstruction.subscribe(set => { - expect(set.queryParams.a).toBeDefined(); - expect(set.queryParams.a).toBe('2'); - - done(); - }); - }); - it('should share a subscription amonst all subscribers', function(done) { router.go('/');