Skip to content

Commit

Permalink
fix(router): allow empty route.path for correct catchAll ranking (#582)
Browse files Browse the repository at this point in the history
  • Loading branch information
evenchange4 authored Jun 6, 2024
1 parent 68b1f96 commit a106fee
Show file tree
Hide file tree
Showing 25 changed files with 371 additions and 23 deletions.
1 change: 1 addition & 0 deletions examples/basic/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

# shuvi.js
/.shuvi/
build/

# production
/dist/
Expand Down
7 changes: 4 additions & 3 deletions examples/basic/package.json
Original file line number Diff line number Diff line change
@@ -1,17 +1,18 @@
{
"private": true,
"name": "example-basic",
"scripts": {
"dev": "shuvi dev",
"build": "shuvi build",
"start": "shuvi start"
},
"dependencies": {
"shuvi": "1.0.0-rc.5"
"shuvi": "workspace:*"
},
"devDependencies": {
"@types/node": "^18.6.1",
"@types/react": "^18.0.17",
"@types/react-dom": "^18.0.6",
"@types/react": "18.0.9",
"@types/react-dom": "18.0.6",
"typescript": "^4.7.4"
}
}
20 changes: 20 additions & 0 deletions examples/basic/src/routes/$/layout.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import React from 'react';
import { RouterView } from '@shuvi/runtime';

export default function Layout() {
return (
<div
style={{
minHeight: '100vh'
}}
>
/$/layout.tsx
<RouterView />
</div>
);
}

export const loader = async () => {
console.log('[demo] /$/layout loader');
return {};
};
9 changes: 9 additions & 0 deletions examples/basic/src/routes/$/page.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import * as React from 'react';

export default function Home() {
return <>/$/pages.tsx</>;
}
export const loader = async () => {
console.log('[demo] /$/page loader');
return {};
};
20 changes: 20 additions & 0 deletions examples/basic/src/routes/$symbol/calc/layout.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import React from 'react';
import { RouterView } from '@shuvi/runtime';

export default function Layout() {
return (
<div
style={{
minHeight: '100vh'
}}
>
/calc/layout.tsx
<RouterView />
</div>
);
}

export const loader = async () => {
console.log('[demo] /calc/layout loader');
return {};
};
9 changes: 9 additions & 0 deletions examples/basic/src/routes/$symbol/calc/page.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import * as React from 'react';

export default function Home() {
return <>/calc/pages.tsx</>;
}
export const loader = async () => {
console.log('[demo] /calc/page loader');
return {};
};
20 changes: 20 additions & 0 deletions examples/basic/src/routes/home/layout.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import React from 'react';
import { RouterView } from '@shuvi/runtime';

export default function Layout() {
return (
<div
style={{
minHeight: '100vh'
}}
>
/home/layout.tsx
<RouterView />
</div>
);
}

export const loader = async () => {
console.log('[demo] /home/layout loader');
return {};
};
9 changes: 9 additions & 0 deletions examples/basic/src/routes/home/page.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import * as React from 'react';

export default function Home() {
return <>/home/pages.tsx</>;
}
export const loader = async () => {
console.log('[demo] /home/page loader');
return {};
};
16 changes: 16 additions & 0 deletions packages/router/src/__tests__/createRoutesFromArray.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,4 +47,20 @@ describe('createRoutesFromArray', () => {
{ caseSensitive: false, path: '/' }
]);
});

it('allow children with empty path', () => {
expect(
createRoutesFromArray([
{ path: '/' },
{ path: '/*', children: [{ path: '' }] }
])
).toStrictEqual([
{ path: '/', caseSensitive: false },
{
path: '/*',
children: [{ path: '', caseSensitive: false }],
caseSensitive: false
}
]);
});
});
54 changes: 54 additions & 0 deletions packages/router/src/__tests__/pathRanking.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,56 @@ import { tokensToParser, comparePathParserScore } from '../pathParserRanker';

type PathParserOptions = Parameters<typeof tokensToParser>[1];

const LeafPageRoute = {
path: ''
};

describe('tokensToParser', () => {
it('scores is correct', () => {
expect(tokensToParser(tokenizePath('/')).score).toStrictEqual([[80]]);
expect(tokensToParser(tokenizePath('/home')).score).toStrictEqual([[80]]);
expect(tokensToParser(tokenizePath('/:symbol')).score).toStrictEqual([
[60]
]);
expect(tokensToParser(tokenizePath('/:symbol/calc')).score).toStrictEqual([
[60],
[80]
]);
expect(tokensToParser(tokenizePath('/*')).score).toStrictEqual([[19]]);
});

it('patch score for pageBranch which ends with empty route', () => {
expect(
tokensToParser(
tokenizePath('/home'),
{},
{ routes: [{ path: '/home' }, LeafPageRoute] }
).score
).toStrictEqual([[80], [0.1]]);
expect(
tokensToParser(
tokenizePath('/:symbol'),
{},
{ routes: [{ path: '/:symbol' }, LeafPageRoute] }
).score
).toStrictEqual([[60], [0.1]]);
expect(
tokensToParser(
tokenizePath('/:symbol/calc'),
{},
{ routes: [{ path: '/:symbol' }, { path: '/calc' }, LeafPageRoute] }
).score
).toStrictEqual([[60], [80], [0.1]]);
expect(
tokensToParser(
tokenizePath('/*'),
{},
{ routes: [{ path: '/*' }, LeafPageRoute] }
).score
).toStrictEqual([[19], [0.1]]);
});
});

describe('Path ranking', () => {
describe('comparePathParser', () => {
function compare(a: number[][], b: number[][]): number {
Expand Down Expand Up @@ -241,6 +291,10 @@ describe('Path ranking', () => {
});
});

it('catchAll /* should be the last one', () => {
checkPathOrder(['/home', '/:symbol/calc', '/*']);
});

it('handles sub segments', () => {
checkPathOrder([
'/a/_2_',
Expand Down
14 changes: 11 additions & 3 deletions packages/router/src/createRoutesFromArray.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,24 @@ import { IPartialRouteRecord, IRouteRecord } from './types';
export function createRoutesFromArray<
T extends IPartialRouteRecord,
U extends IRouteRecord
>(array: T[]): U[] {
>(
array: T[],
/**
* allowEmptyPath: allow empty path for children
* A pageBranch could be ended with a page route with empty path.
*/
allowEmptyPath = false
): U[] {
return array.map(partialRoute => {
const defaultPath = allowEmptyPath ? '' : '/';
let route: U = {
...(partialRoute as any),
caseSensitive: !!partialRoute.caseSensitive,
path: partialRoute.path || '/'
path: partialRoute.path || defaultPath
};

if (partialRoute.children) {
route.children = createRoutesFromArray(partialRoute.children);
route.children = createRoutesFromArray(partialRoute.children, true);
}

return route;
Expand Down
14 changes: 11 additions & 3 deletions packages/router/src/matchRoutes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,9 @@ export function rankRouteBranches<T extends [string, ...any[]]>(
}

const normalizedPaths = branches.map((branch, index) => {
const [path] = branch;
const [path, routes] = branch;
return {
...tokensToParser(tokenizePath(path)),
...tokensToParser(tokenizePath(path), undefined, { routes }),
path,
index
};
Expand All @@ -89,7 +89,15 @@ function flattenRoutes<T extends IRouteBaseObject>(
parentIndexes: number[] = []
): IRouteBranch<T>[] {
routes.forEach((route, index) => {
let path = joinPaths([parentPath, route.path]);
let path;
if (route.path === '') {
/**
* An empty path is allowed, don't append a slash.
*/
path = parentPath;
} else {
path = joinPaths([parentPath, route.path]);
}
let routes = parentRoutes.concat(route);
let indexes = parentIndexes.concat(index);

Expand Down
19 changes: 17 additions & 2 deletions packages/router/src/pathParserRanker.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Token, TokenType, TokenCatchAll } from './pathTokenizer';
import { IRouteRecord } from './types';

type PathParams = Record<string, string | string[]>;

Expand Down Expand Up @@ -113,7 +114,8 @@ const enum PathScore {
BonusOptional = -0.8 * _multiplier, // /:w? or /:w*
// these two have to be under 0.1 so a strict /:page is still lower than /:a-:b
BonusStrict = 0.07 * _multiplier, // when options strict: true is passed, as the regex omits \/?
BonusCaseSensitive = 0.025 * _multiplier // when options strict: true is passed, as the regex omits \/?
BonusCaseSensitive = 0.025 * _multiplier, // when options strict: true is passed, as the regex omits \/?
BonusEmptyStringPath = 0.01 * _multiplier // when the path is an empty string
}

// Special Regex characters that must be escaped in static tokens
Expand All @@ -128,7 +130,8 @@ const REGEX_CHARS_RE = /[.+*?^${}()[\]/\\]/g;
*/
export function tokensToParser(
segments: Array<Token[]>,
extraOptions?: _PathParserOptions
extraOptions?: _PathParserOptions,
branchInfo?: { routes: IRouteRecord[] }
): PathParser {
const options = Object.assign({}, BASE_PATH_PARSER_OPTIONS, extraOptions);

Expand Down Expand Up @@ -354,6 +357,18 @@ export function tokensToParser(
return path;
}

/**
* To make sure the score of pageBranch is always higher priority
* than the layoutRoute.
* We append a bonus score if the last route is an empty path.
*/
if (branchInfo?.routes) {
const lastRoutePath = branchInfo.routes[branchInfo.routes.length - 1]?.path;
if (lastRoutePath === '') {
score.push([PathScore.BonusEmptyStringPath]);
}
}

return {
re,
score,
Expand Down
Loading

0 comments on commit a106fee

Please sign in to comment.