Skip to content

Commit 8d07126

Browse files
vicbvicb
committed
fix(router): matchesIDs account for fixed segments and route params
1 parent 7d5c6af commit 8d07126

File tree

4 files changed

+248
-33
lines changed

4 files changed

+248
-33
lines changed

‎core/src/components/router/test/all/pr-24645.e2e.ts‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
import { newE2EPage } from '@stencil/core/testing';
2+
3+
test('nav.pop should update the URL', async () => {
4+
const page = await newE2EPage({
5+
url: '/src/components/router/test/all/pr-24645.html?ionic:_testing=true'
6+
});
7+
8+
const push = await page.$('#router-push');
9+
const pop = await page.$('#nav-pop');
10+
11+
const routes = [
12+
'/a',
13+
'/a/b',
14+
'/fixed/a/b/c',
15+
'/routeparam/a',
16+
'/routeparam/a/b',
17+
];
18+
19+
// Push all the routes.
20+
for (const r of routes) {
21+
await page.$eval('#route', (el: any, route) => el.value = route, r);
22+
await push.click();
23+
await page.waitForChanges();
24+
expect(await page.url()).toMatch(new RegExp(`#${r}$`));
25+
}
26+
27+
// Pop should restore the urls.
28+
for (const r of routes.reverse()) {
29+
expect(await page.url()).toMatch(new RegExp(`#${r}$`));
30+
await pop.click();
31+
await page.waitForChanges();
32+
}
33+
});
34+
35+
test('nav.push should update the URL', async () => {
36+
const page = await newE2EPage({
37+
url: '/src/components/router/test/all/pr-24645.html?ionic:_testing=true'
38+
});
39+
40+
const pushX = await page.$('#nav-push-x');
41+
await pushX.click();
42+
await page.waitForChanges();
43+
expect(await page.url()).toMatch(/\/x$/);
44+
45+
const pushFixed = await page.$('#nav-push-fixed');
46+
await pushFixed.click();
47+
await page.waitForChanges();
48+
expect(await page.url()).toMatch(/\/fixed\/x\/y\/z$/);
49+
50+
const pushRouteParam = await page.$('#nav-push-routeparam');
51+
await pushRouteParam.click();
52+
await page.waitForChanges();
53+
expect(await page.url()).toMatch(/\/routeparam\/x$/);
54+
});

‎core/src/components/router/test/all/pr-24645.html‎

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
<!DOCTYPE html>
2+
<html lang="en" dir="ltr">
3+
4+
<head>
5+
<meta charset="UTF-8">
6+
<title>Navigation Guards</title>
7+
<meta name="viewport" content="width=device-width, initial-scale=1.0, minimum-scale=1.0, maximum-scale=1.0, user-scalable=no">
8+
<link href="../../../../../css/ionic.bundle.css" rel="stylesheet">
9+
<link href="../../../../../scripts/testing/styles.css" rel="stylesheet">
10+
<style>
11+
.toolbar {
12+
position: fixed;
13+
top: 20px;
14+
right: 20px;
15+
z-index: 100;
16+
width: 300px;
17+
background: white;
18+
box-shadow: 0px 1px 10px rgba(0,0,0,0.2);
19+
}
20+
</style>
21+
<script src="../../../../../scripts/testing/scripts.js"></script>
22+
<script nomodule src="../../../../../dist/ionic/ionic.js"></script>
23+
<script type="module" src="../../../../../dist/ionic/ionic.esm.js"></script>
24+
<script>
25+
class MyComponent extends HTMLElement {
26+
connectedCallback() {
27+
const props = {
28+
seg1: this.seg1,
29+
seg2: this.seg2,
30+
seg3: this.seg3,
31+
route: this.route,
32+
}
33+
this.innerHTML = `<p>${JSON.stringify(props, null, 2)}</p>`;
34+
}
35+
}
36+
37+
customElements.define('my-component', MyComponent);
38+
</script>
39+
</head>
40+
41+
<body>
42+
<ion-app>
43+
44+
<ion-router>
45+
<ion-route url="/" component="my-component"></ion-route>
46+
<ion-route url="/:seg1" component="my-component"></ion-route>
47+
<ion-route url="/:seg1/:seg2" component="my-component"></ion-route>
48+
<ion-route url="/fixed/:seg1/:seg2/:seg3" component="my-component"></ion-route>
49+
<ion-route id="rp1" url="/routeparam/:seg1" component="my-component"></ion-route>
50+
<ion-route id="rp2" url="/routeparam/:seg1/:seg2" component="my-component"></ion-route>
51+
</ion-router>
52+
53+
<ion-header>
54+
<ion-toolbar>
55+
<ion-title>ion-nav navigation tests</ion-title>
56+
</ion-toolbar>
57+
</ion-header>
58+
59+
<ion-content>
60+
<ion-list>
61+
<ion-item>
62+
<ion-select id="route" placeholder="Select the navigation target" interface="popover">
63+
<ion-select-option value="/">/</ion-select-option>
64+
<ion-select-option value="/a">/a</ion-select-option>
65+
<ion-select-option value="/a/b">/a/b</ion-select-option>
66+
<ion-select-option value="/fixed/a/b/c">/fixed/a/b/c</ion-select-option>
67+
<ion-select-option value="/routeparam/a">/routeparam/a</ion-select-option>
68+
<ion-select-option value="/routeparam/a/b">/routeparam/a/b</ion-select-option>
69+
</ion-select>
70+
</ion-item>
71+
<ion-item>
72+
<ion-button id="router-push">router.push</ion-button>
73+
</ion-item>
74+
<ion-item>
75+
<ion-button id="nav-push-x">nav.push(/x)</ion-button>
76+
</ion-item>
77+
<ion-item>
78+
<ion-button id="nav-push-fixed">nav.push(/fixed/x/y/z)</ion-button>
79+
</ion-item>
80+
<ion-item>
81+
<ion-button id="nav-push-routeparam">nav.push(/routeparam/x)</ion-button>
82+
</ion-item>
83+
<ion-item>
84+
<ion-button id="nav-pop">nav.pop</ion-button>
85+
</ion-item>
86+
<ion-item>
87+
<ion-nav></ion-nav>
88+
</ion-item>
89+
</ion-list>
90+
</ion-content>
91+
92+
93+
<script>
94+
document.querySelector('ion-route#rp1').componentProps = {route: "route param"};
95+
document.querySelector('ion-route#rp2').componentProps = {route: "route param"};
96+
97+
const route = document.querySelector('#route');
98+
const routerPush = document.querySelector('#router-push');
99+
const router = document.querySelector('ion-router');
100+
const nav = document.querySelector('ion-nav');
101+
const navPop = document.querySelector('#nav-pop');
102+
103+
routerPush.addEventListener('click', () => {
104+
if (route.value != null) {
105+
router.push(route.value);
106+
}
107+
});
108+
109+
navPop.addEventListener('click', async () => {
110+
if (await nav.canGoBack()) {
111+
nav.pop();
112+
}
113+
});
114+
115+
document.querySelector('#nav-push-x').addEventListener('click', () => {
116+
nav.push(new MyComponent(), {seg1: 'x'});
117+
});
118+
119+
document.querySelector('#nav-push-fixed').addEventListener('click', () => {
120+
nav.push(new MyComponent(), {seg1: 'x', seg2: 'y', seg3: 'z'});
121+
});
122+
123+
document.querySelector('#nav-push-routeparam').addEventListener('click', () => {
124+
nav.push(new MyComponent(), {seg1: 'x', route: "y"});
125+
});
126+
127+
</script>
128+
</ion-app>
129+
</body>
130+
</html>

‎core/src/components/router/test/matching.spec.tsx‎

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,11 +43,46 @@ describe('matchesIDs', () => {
4343
it('should match path with params', () => {
4444
const ids = [{ id: 'my-page', params: { s1: 'a', s2: 'b' } }];
4545

46+
// The route has only parameter segments.
4647
expect(matchesIDs(ids, [{ id: 'my-page', segments: [''], params: {} }])).toBe(1);
4748
expect(matchesIDs(ids, [{ id: 'my-page', segments: [':s1'], params: {} }])).toBe(1);
48-
expect(matchesIDs(ids, [{ id: 'my-page', segments: [':s1', ':s2'], params: {} }])).toBe(3);
49+
expect(matchesIDs(ids, [{ id: 'my-page', segments: [':s1', ':s2'], params: {} }])).toBe(2);
4950
expect(matchesIDs(ids, [{ id: 'my-page', segments: [':s1', ':s2', ':s3'], params: {} }])).toBe(1);
51+
52+
// The route has a mix of parameter and fixed segments.
53+
expect(matchesIDs(ids, [{ id: 'my-page', segments: ['prefix'], params: {} }])).toBe(1);
54+
expect(matchesIDs(ids, [{ id: 'my-page', segments: ['prefix', ':s1'], params: {} }])).toBe(1);
55+
expect(matchesIDs(ids, [{ id: 'my-page', segments: ['prefix', ':s1', ':s2'], params: {} }])).toBe(2);
56+
expect(matchesIDs(ids, [{ id: 'my-page', segments: ['prefix', ':s1', ':s2', ':s3'], params: {} }])).toBe(1);
57+
58+
// The route has parameters (componentProps).
59+
expect(matchesIDs(ids, [{ id: 'my-page', segments: ['prefix'], params: {s2: 'route param'} }])).toBe(1);
60+
expect(matchesIDs(ids, [{ id: 'my-page', segments: ['prefix', ':s1'], params: {s2: 'route param'} }])).toBe(2);
61+
expect(matchesIDs(ids, [{ id: 'my-page', segments: ['prefix', ':s1', ':s2'], params: {s2: 'route param'} }])).toBe(2);
62+
expect(matchesIDs(ids, [{ id: 'my-page', segments: ['prefix', ':s1', ':s2', ':s3'], params: {s2: 'route param'} }])).toBe(1);
5063
})
64+
65+
it('should match path without params', () => {
66+
const ids = [{ id: 'my-page', params: {} }];
67+
68+
// The route has only parameter segments.
69+
expect(matchesIDs(ids, [{ id: 'my-page', segments: [''], params: {} }])).toBe(2);
70+
expect(matchesIDs(ids, [{ id: 'my-page', segments: [':s1'], params: {} }])).toBe(1);
71+
expect(matchesIDs(ids, [{ id: 'my-page', segments: [':s1', ':s2'], params: {} }])).toBe(1);
72+
expect(matchesIDs(ids, [{ id: 'my-page', segments: [':s1', ':s2', ':s3'], params: {} }])).toBe(1);
73+
74+
// The route has a mix of parameter and fixed segments.
75+
expect(matchesIDs(ids, [{ id: 'my-page', segments: ['prefix'], params: {} }])).toBe(2);
76+
expect(matchesIDs(ids, [{ id: 'my-page', segments: ['prefix', ':s1'], params: {} }])).toBe(1);
77+
expect(matchesIDs(ids, [{ id: 'my-page', segments: ['prefix', ':s1', ':s2'], params: {} }])).toBe(1);
78+
expect(matchesIDs(ids, [{ id: 'my-page', segments: ['prefix', ':s1', ':s2', ':s3'], params: {} }])).toBe(1);
79+
80+
// The route has parameters (componentProps).
81+
expect(matchesIDs(ids, [{ id: 'my-page', segments: ['prefix'], params: {s2: 'route param'} }])).toBe(1);
82+
expect(matchesIDs(ids, [{ id: 'my-page', segments: ['prefix', ':s1'], params: {s2: 'route param'} }])).toBe(1);
83+
expect(matchesIDs(ids, [{ id: 'my-page', segments: ['prefix', ':s1', ':s2'], params: {s2: 'route param'} }])).toBe(1);
84+
expect(matchesIDs(ids, [{ id: 'my-page', segments: ['prefix', ':s1', ':s2', ':s3'], params: {s2: 'route param'} }])).toBe(1);
85+
})
5186
});
5287

5388
describe('matchesSegments', () => {

‎core/src/components/router/utils/matching.ts‎

Lines changed: 28 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -34,55 +34,51 @@ export const findRouteRedirect = (segments: string[], redirects: RouteRedirect[]
3434
return redirects.find(redirect => matchesRedirect(segments, redirect));
3535
};
3636

37+
/**
38+
* Returns the matching score for a RouteID vs a RouteChain.
39+
*
40+
* The best score is returned when the id and the entry match for all the levels.
41+
* They match when:
42+
* - they both use the same id (component tag or tab name),
43+
* - they both have the same parameters.
44+
*/
3745
export const matchesIDs = (ids: Pick<RouteID, 'id' | 'params'>[], chain: RouteChain): number => {
3846
const len = Math.min(ids.length, chain.length);
3947

4048
let score = 0;
4149

4250
for (let i = 0; i < len; i++) {
4351
const routeId = ids[i];
44-
const routeChain = chain[i];
52+
const routeEntry = chain[i];
53+
4554
// Skip results where the route id does not match the chain at the same index
46-
if (routeId.id.toLowerCase() !== routeChain.id) {
55+
if (routeId.id.toLowerCase() === routeEntry.id.toLowerCase()) {
56+
score++;
57+
} else {
4758
break;
4859
}
60+
61+
// Get the parameters of the route (that is the componentProps property of the <ion-route>).
62+
const routeEntryParams = new Set(routeEntry.params ? Object.keys(routeEntry.params) : []);
63+
// Add the parameter segments from the url.
64+
for (const segment of routeEntry.segments) {
65+
if (segment[0] === ':') {
66+
routeEntryParams.add(segment.substring(1));
67+
}
68+
}
69+
70+
// The parameters should be the same for RouteID and RouteEntry.
4971
if (routeId.params) {
5072
const routeIdParams = Object.keys(routeId.params);
51-
// Only compare routes with the chain that have the same number of parameters.
52-
if (routeIdParams.length === routeChain.segments.length) {
53-
// Maps the route's params into a path based on the path variable names,
54-
// to compare against the route chain format.
55-
//
56-
// Before:
57-
// ```ts
58-
// {
59-
// params: {
60-
// s1: 'a',
61-
// s2: 'b'
62-
// }
63-
// }
64-
// ```
65-
//
66-
// After:
67-
// ```ts
68-
// [':s1',':s2']
69-
// ```
70-
//
71-
const pathWithParams = routeIdParams.map(key => `:${key}`);
72-
for (let j = 0; j < pathWithParams.length; j++) {
73-
// Skip results where the path variable is not a match
74-
if (pathWithParams[j].toLowerCase() !== routeChain.segments[j]) {
75-
break;
76-
}
77-
// Weight path matches for the same index higher.
73+
if (routeIdParams.length === routeEntryParams.size) {
74+
const paramsMatch = routeIdParams.every(p => routeEntryParams.has(p));
75+
if (paramsMatch) {
7876
score++;
7977
}
80-
8178
}
8279
}
83-
// Weight id matches
84-
score++;
8580
}
81+
8682
return score;
8783
}
8884

0 commit comments

Comments
 (0)