Skip to content

Ensure to match against the most specific route #87

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

Merged
agubler merged 4 commits into from
Sep 6, 2018
Merged

Ensure to match against the most specific route #87

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
19ee130
Ensure to match against the most specific route
agubler Aug 29, 2018
25e2bd7
Add another unit test
agubler Aug 29, 2018
34952d9
Couple more assertions
agubler Aug 29, 2018
daa95fb
Use constants for the route ranking
agubler Aug 30, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
155 changes: 90 additions & 65 deletions src/routing/Router.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,19 @@
import QueuingEvented from '../core/QueuingEvented';
import {
RouteConfig,
History,
MatchType,
OutletContext,
Params,
RouterInterface,
Route,
RouterOptions
} from './interfaces';
import { RouteConfig, History, OutletContext, Params, RouterInterface, Route, RouterOptions } from './interfaces';
import { HashHistory } from './history/HashHistory';
import { EventObject } from '../core/Evented';

const PARAM = Symbol('routing param');
const PARAM = '__PARAM__';

const paramRegExp = new RegExp(/^{.+}$/);

interface RouteWrapper {
route: Route;
segments: string[];
parent?: RouteWrapper;
type?: string;
params: Params;
}

export interface NavEvent extends EventObject<string> {
outlet: string;
Expand All @@ -24,6 +25,9 @@ export interface OutletEvent extends EventObject<string> {
action: 'enter' | 'exit';
}

const ROUTE_SEGMENT_SCORE = 7;
Copy link
Contributor

@JamesLMilner JamesLMilner Sep 4, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These numbers look fairly arbitrary at first glance. Perhaps a comment to clarify?

Copy link
Member Author

@agubler agubler Sep 4, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They actually are fairly arbitrary, I went for a score that meant we could introduce other types of segments in the future (like maybe wild card, which would have an even smaller score).

const DYNAMIC_SEGMENT_PENALTY = 2;

export class Router extends QueuingEvented<{ nav: NavEvent; outlet: OutletEvent }> implements RouterInterface {
private _routes: Route[] = [];
private _outletMap: { [index: string]: Route } = Object.create(null);
Expand Down Expand Up @@ -139,7 +143,7 @@ export class Router extends QueuingEvented<{ nav: NavEvent; outlet: OutletEvent
let queryParams: string[] = [];
parsedPath = this._stripLeadingSlash(parsedPath);

const segments: (symbol | string)[] = parsedPath.split('/');
const segments: string[] = parsedPath.split('/');
const route: Route = {
params: [],
outlet,
Expand All @@ -151,14 +155,17 @@ export class Router extends QueuingEvented<{ nav: NavEvent; outlet: OutletEvent
fullParams: [],
fullQueryParams: [],
onEnter,
score: parentRoute ? parentRoute.score : 0,
onExit
};
if (defaultRoute) {
this._defaultOutlet = outlet;
}
for (let i = 0; i < segments.length; i++) {
const segment = segments[i];
if (typeof segment === 'string' && segment[0] === '{') {
route.score += ROUTE_SEGMENT_SCORE;
if (paramRegExp.test(segment)) {
route.score -= DYNAMIC_SEGMENT_PENALTY;
route.params.push(segment.replace('{', '').replace('}', ''));
segments[i] = PARAM;
}
Expand Down Expand Up @@ -205,31 +212,26 @@ export class Router extends QueuingEvented<{ nav: NavEvent; outlet: OutletEvent
*/
private _onChange = (requestedPath: string): void => {
this.emit({ type: 'navstart' });
requestedPath = this._stripLeadingSlash(requestedPath);
const previousMatchedOutlets = this._matchedOutlets;
this._matchedOutlets = Object.create(null);
this._currentParams = {};
requestedPath = this._stripLeadingSlash(requestedPath);

const [path, queryParamString] = requestedPath.split('?');
this._currentQueryParams = this._getQueryParams(queryParamString);
let matchedOutletContext: OutletContext | undefined;
let matchedOutlet: string | undefined;
let routes = [...this._routes];
let paramIndex = 0;
let segments = path.split('/');
let routeMatched = false;
let previousOutlet: string | undefined;
while (routes.length > 0) {
if (segments.length === 0) {
break;
}
const route = routes.shift()!;
const { onEnter, onExit } = route;
let type: MatchType = 'index';
const segmentsForRoute = [...segments];
let routeMatch = true;
const segments = path.split('/');
let routeConfigs: RouteWrapper[] = this._routes.map((route) => ({
route,
segments: [...segments],
parent: undefined,
params: {}
}));
let routeConfig: RouteWrapper | undefined;
let matchedRoutes: RouteWrapper[] = [];
while ((routeConfig = routeConfigs.pop())) {
const { route, parent, segments, params } = routeConfig;
let segmentIndex = 0;

let type = 'index';
let paramIndex = 0;
let routeMatch = true;
if (segments.length < route.segments.length) {
routeMatch = false;
} else {
Expand All @@ -240,43 +242,75 @@ export class Router extends QueuingEvented<{ nav: NavEvent; outlet: OutletEvent
}
const segment = segments.shift()!;
if (route.segments[segmentIndex] === PARAM) {
this._currentParams[route.params[paramIndex++]] = segment;
params[route.params[paramIndex++]] = segment;
this._currentParams = { ...this._currentParams, ...params };
} else if (route.segments[segmentIndex] !== segment) {
routeMatch = false;
break;
}
segmentIndex++;
}
}
if (routeMatch === true) {
previousOutlet = route.outlet;
routeMatched = true;

if (routeMatch) {
routeConfig.type = type;
matchedRoutes.push({ route, parent, type, params, segments: [] });
if (segments.length) {
routeConfigs = [
...routeConfigs,
...route.children.map((childRoute) => ({
route: childRoute,
segments: [...segments],
parent: routeConfig,
type,
params: { ...params }
}))
];
}
}
}

let matchedOutletName: string | undefined = undefined;
let matchedRoute: any = matchedRoutes.reduce((match: any, matchedRoute: any) => {
if (!match) {
return matchedRoute;
}
if (match.route.score > matchedRoute.route.score) {
return match;
}
return matchedRoute;
}, undefined);

if (matchedRoute) {
if (matchedRoute.type === 'partial') {
matchedRoute.type = 'error';
}
matchedOutletName = matchedRoute.route.outlet;
while (matchedRoute) {
let { type, params, parent, route } = matchedRoute;

if (!previousMatchedOutlets[route.outlet]) {
this.emit({ type: 'outlet', outlet: route.outlet, action: 'enter' });
}

this._matchedOutlets[route.outlet] = {
this._matchedOutlets[matchedRoute.route.outlet] = {
queryParams: this._currentQueryParams,
params: { ...this._currentParams },
params,
type,
isError: () => type === 'error',
isExact: () => type === 'index',
onEnter,
onExit
onEnter: route.onEnter,
onExit: route.onExit
};
matchedOutletContext = this._matchedOutlets[route.outlet];
matchedOutlet = route.outlet;
if (route.children.length) {
paramIndex = 0;
}
routes = [...route.children];
} else {
if (previousOutlet !== undefined && routes.length === 0) {
this._matchedOutlets[previousOutlet].type = 'error';
this._matchedOutlets[previousOutlet].isError = () => true;
}
segments = [...segmentsForRoute];
matchedRoute = parent;
}
} else {
this._matchedOutlets.errorOutlet = {
queryParams: {},
params: {},
isError: () => true,
isExact: () => false,
type: 'error'
};
}

const previousMatchedOutletKeys = Object.keys(previousMatchedOutlets);
Expand All @@ -285,17 +319,8 @@ export class Router extends QueuingEvented<{ nav: NavEvent; outlet: OutletEvent
this.emit({ type: 'outlet', outlet: previousMatchedOutletKeys[i], action: 'exit' });
}
}
if (routeMatched === false) {
this._matchedOutlets.errorOutlet = {
queryParams: this._currentQueryParams,
params: { ...this._currentParams },
isError: () => true,
isExact: () => false,
type: 'error'
};
}
if (matchedOutlet && matchedOutletContext) {
this.emit({ type: 'nav', outlet: matchedOutlet, context: matchedOutletContext });
if (matchedOutletName) {
this.emit({ type: 'nav', outlet: matchedOutletName, context: this._matchedOutlets[matchedOutletName] });
}
};
}
Expand Down
3 changes: 2 additions & 1 deletion src/routing/interfaces.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,13 @@ export interface Route {
path: string;
outlet: string;
params: string[];
segments: (symbol | string)[];
segments: string[];
children: Route[];
fullPath: string;
fullParams: string[];
fullQueryParams: string[];
defaultParams: Params;
score: number;
onEnter?: OnEnter;
onExit?: OnExit;
}
Expand Down
85 changes: 85 additions & 0 deletions tests/routing/unit/Router.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,56 @@ const routeConfigWithParamsAndQueryParams = [
}
];

const orderIndependentRouteConfig = [
{
path: '{foo}',
outlet: 'partial',
children: [
{
path: 'bar/{bar}',
outlet: 'bar-with-param'
},
{
path: 'bar/bar',
outlet: 'bar'
}
]
},
{
path: 'foo',
outlet: 'foo'
},
{
path: '/',
outlet: 'home'
}
];

const config = [
{
path: 'foo',
outlet: 'foo-one'
},
{
path: 'foo',
outlet: 'foo-two',
children: [
{
path: 'baz',
outlet: 'baz'
}
]
},
{
path: '{bar}',
outlet: 'param'
},
{
path: 'bar',
outlet: 'bar'
}
];

describe('Router', () => {
it('Navigates to current route if matches against a registered outlet', () => {
const router = new Router(routeConfig, { HistoryManager });
Expand All @@ -119,6 +169,17 @@ describe('Router', () => {
assert.isOk(context);
});

it('should match against the most exact outlet specified in the configuration based on the outlets score', () => {
const router = new Router(config, { HistoryManager });
router.setPath('/bar');
assert.isOk(router.getOutlet('bar'));
assert.isUndefined(router.getOutlet('param'));
router.setPath('/foo/baz');
assert.isOk(router.getOutlet('baz'));
assert.isOk(router.getOutlet('foo-two'));
assert.isUndefined(router.getOutlet('foo-one'));
});

it('Navigates to global "errorOutlet" if current route does not match a registered outlet and no default route is configured', () => {
const router = new Router(routeConfigNoRoot, { HistoryManager });
const context = router.getOutlet('errorOutlet');
Expand Down Expand Up @@ -154,6 +215,30 @@ describe('Router', () => {
assert.strictEqual(context!.isExact(), true);
});

it('should find the most specific match from the routing configuration', () => {
const router = new Router(orderIndependentRouteConfig, { HistoryManager });
router.setPath('/foo');
const fooContext = router.getOutlet('foo');
assert.isOk(fooContext);
assert.deepEqual(fooContext!.params, {});
assert.deepEqual(fooContext!.queryParams, {});
assert.deepEqual(fooContext!.type, 'index');
assert.strictEqual(fooContext!.isExact(), true);
router.setPath('/foo/bar/bar');
const barContext = router.getOutlet('bar');
assert.isOk(barContext);
assert.deepEqual(barContext!.params, { foo: 'foo' });
assert.deepEqual(barContext!.queryParams, {});
assert.deepEqual(barContext!.type, 'index');
assert.strictEqual(barContext!.isExact(), true);
const partialContext = router.getOutlet('partial');
assert.isOk(partialContext);
assert.deepEqual(partialContext!.params, { foo: 'foo' });
assert.deepEqual(partialContext!.queryParams, {});
assert.deepEqual(partialContext!.type, 'partial');
assert.strictEqual(partialContext!.isExact(), false);
});

it('Should register as a partial match for an outlet that matches a section of the route', () => {
const router = new Router(routeConfig, { HistoryManager });
router.setPath('/foo/bar');
Expand Down