Skip to content

Commit

Permalink
fix(router): matchesIDs account for fixed segments and route params
Browse files Browse the repository at this point in the history
  • Loading branch information
@vicb
vicb committed Jan 25, 2022
1 parent 7d5c6af commit 8d07126
Show file tree
Hide file tree
Showing 4 changed files with 248 additions and 33 deletions.
54 changes: 54 additions & 0 deletions core/src/components/router/test/all/pr-24645.e2e.ts
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import { newE2EPage } from '@stencil/core/testing';

test('nav.pop should update the URL', async () => {
const page = await newE2EPage({
url: '/src/components/router/test/all/pr-24645.html?ionic:_testing=true'
});

const push = await page.$('#router-push');
const pop = await page.$('#nav-pop');

const routes = [
'/a',
'/a/b',
'/fixed/a/b/c',
'/routeparam/a',
'/routeparam/a/b',
];

// Push all the routes.
for (const r of routes) {
await page.$eval('#route', (el: any, route) => el.value = route, r);
await push.click();
await page.waitForChanges();
expect(await page.url()).toMatch(new RegExp(`#${r}$`));
}

// Pop should restore the urls.
for (const r of routes.reverse()) {
expect(await page.url()).toMatch(new RegExp(`#${r}$`));
await pop.click();
await page.waitForChanges();
}
});

test('nav.push should update the URL', async () => {
const page = await newE2EPage({
url: '/src/components/router/test/all/pr-24645.html?ionic:_testing=true'
});

const pushX = await page.$('#nav-push-x');
await pushX.click();
await page.waitForChanges();
expect(await page.url()).toMatch(/\/x$/);

const pushFixed = await page.$('#nav-push-fixed');
await pushFixed.click();
await page.waitForChanges();
expect(await page.url()).toMatch(/\/fixed\/x\/y\/z$/);

const pushRouteParam = await page.$('#nav-push-routeparam');
await pushRouteParam.click();
await page.waitForChanges();
expect(await page.url()).toMatch(/\/routeparam\/x$/);
});
130 changes: 130 additions & 0 deletions core/src/components/router/test/all/pr-24645.html
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
<!DOCTYPE html>
<html lang="en" dir="ltr">

<head>
<meta charset="UTF-8">
<title>Navigation Guards</title>
<meta name="viewport" content="width=device-width, initial-scale=1.0, minimum-scale=1.0, maximum-scale=1.0, user-scalable=no">
<link href="../../../../../css/ionic.bundle.css" rel="stylesheet">
<link href="../../../../../scripts/testing/styles.css" rel="stylesheet">
<style>
.toolbar {
position: fixed;
top: 20px;
right: 20px;
z-index: 100;
width: 300px;
background: white;
box-shadow: 0px 1px 10px rgba(0,0,0,0.2);
}
</style>
<script src="../../../../../scripts/testing/scripts.js"></script>
<script nomodule src="../../../../../dist/ionic/ionic.js"></script>
<script type="module" src="../../../../../dist/ionic/ionic.esm.js"></script>
<script>
class MyComponent extends HTMLElement {
connectedCallback() {
const props = {
seg1: this.seg1,
seg2: this.seg2,
seg3: this.seg3,
route: this.route,
}
this.innerHTML = `<p>${JSON.stringify(props, null, 2)}</p>`;
}
}

customElements.define('my-component', MyComponent);
</script>
</head>

<body>
<ion-app>

<ion-router>
<ion-route url="/" component="my-component"></ion-route>
<ion-route url="/:seg1" component="my-component"></ion-route>
<ion-route url="/:seg1/:seg2" component="my-component"></ion-route>
<ion-route url="/fixed/:seg1/:seg2/:seg3" component="my-component"></ion-route>
<ion-route id="rp1" url="/routeparam/:seg1" component="my-component"></ion-route>
<ion-route id="rp2" url="/routeparam/:seg1/:seg2" component="my-component"></ion-route>
</ion-router>

<ion-header>
<ion-toolbar>
<ion-title>ion-nav navigation tests</ion-title>
</ion-toolbar>
</ion-header>

<ion-content>
<ion-list>
<ion-item>
<ion-select id="route" placeholder="Select the navigation target" interface="popover">
<ion-select-option value="/">/</ion-select-option>
<ion-select-option value="/a">/a</ion-select-option>
<ion-select-option value="/a/b">/a/b</ion-select-option>
<ion-select-option value="/fixed/a/b/c">/fixed/a/b/c</ion-select-option>
<ion-select-option value="/routeparam/a">/routeparam/a</ion-select-option>
<ion-select-option value="/routeparam/a/b">/routeparam/a/b</ion-select-option>
</ion-select>
</ion-item>
<ion-item>
<ion-button id="router-push">router.push</ion-button>
</ion-item>
<ion-item>
<ion-button id="nav-push-x">nav.push(/x)</ion-button>
</ion-item>
<ion-item>
<ion-button id="nav-push-fixed">nav.push(/fixed/x/y/z)</ion-button>
</ion-item>
<ion-item>
<ion-button id="nav-push-routeparam">nav.push(/routeparam/x)</ion-button>
</ion-item>
<ion-item>
<ion-button id="nav-pop">nav.pop</ion-button>
</ion-item>
<ion-item>
<ion-nav></ion-nav>
</ion-item>
</ion-list>
</ion-content>


<script>
document.querySelector('ion-route#rp1').componentProps = {route: "route param"};
document.querySelector('ion-route#rp2').componentProps = {route: "route param"};

const route = document.querySelector('#route');
const routerPush = document.querySelector('#router-push');
const router = document.querySelector('ion-router');
const nav = document.querySelector('ion-nav');
const navPop = document.querySelector('#nav-pop');

routerPush.addEventListener('click', () => {
if (route.value != null) {
router.push(route.value);
}
});

navPop.addEventListener('click', async () => {
if (await nav.canGoBack()) {
nav.pop();
}
});

document.querySelector('#nav-push-x').addEventListener('click', () => {
nav.push(new MyComponent(), {seg1: 'x'});
});

document.querySelector('#nav-push-fixed').addEventListener('click', () => {
nav.push(new MyComponent(), {seg1: 'x', seg2: 'y', seg3: 'z'});
});

document.querySelector('#nav-push-routeparam').addEventListener('click', () => {
nav.push(new MyComponent(), {seg1: 'x', route: "y"});
});

</script>
</ion-app>
</body>
</html>
37 changes: 36 additions & 1 deletion core/src/components/router/test/matching.spec.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,46 @@ describe('matchesIDs', () => {
it('should match path with params', () => {
const ids = [{ id: 'my-page', params: { s1: 'a', s2: 'b' } }];

// The route has only parameter segments.
expect(matchesIDs(ids, [{ id: 'my-page', segments: [''], params: {} }])).toBe(1);
expect(matchesIDs(ids, [{ id: 'my-page', segments: [':s1'], params: {} }])).toBe(1);
expect(matchesIDs(ids, [{ id: 'my-page', segments: [':s1', ':s2'], params: {} }])).toBe(3);
expect(matchesIDs(ids, [{ id: 'my-page', segments: [':s1', ':s2'], params: {} }])).toBe(2);
expect(matchesIDs(ids, [{ id: 'my-page', segments: [':s1', ':s2', ':s3'], params: {} }])).toBe(1);

// The route has a mix of parameter and fixed segments.
expect(matchesIDs(ids, [{ id: 'my-page', segments: ['prefix'], params: {} }])).toBe(1);
expect(matchesIDs(ids, [{ id: 'my-page', segments: ['prefix', ':s1'], params: {} }])).toBe(1);
expect(matchesIDs(ids, [{ id: 'my-page', segments: ['prefix', ':s1', ':s2'], params: {} }])).toBe(2);
expect(matchesIDs(ids, [{ id: 'my-page', segments: ['prefix', ':s1', ':s2', ':s3'], params: {} }])).toBe(1);

// The route has parameters (componentProps).
expect(matchesIDs(ids, [{ id: 'my-page', segments: ['prefix'], params: {s2: 'route param'} }])).toBe(1);
expect(matchesIDs(ids, [{ id: 'my-page', segments: ['prefix', ':s1'], params: {s2: 'route param'} }])).toBe(2);
expect(matchesIDs(ids, [{ id: 'my-page', segments: ['prefix', ':s1', ':s2'], params: {s2: 'route param'} }])).toBe(2);
expect(matchesIDs(ids, [{ id: 'my-page', segments: ['prefix', ':s1', ':s2', ':s3'], params: {s2: 'route param'} }])).toBe(1);
})

it('should match path without params', () => {
const ids = [{ id: 'my-page', params: {} }];

// The route has only parameter segments.
expect(matchesIDs(ids, [{ id: 'my-page', segments: [''], params: {} }])).toBe(2);
expect(matchesIDs(ids, [{ id: 'my-page', segments: [':s1'], params: {} }])).toBe(1);
expect(matchesIDs(ids, [{ id: 'my-page', segments: [':s1', ':s2'], params: {} }])).toBe(1);
expect(matchesIDs(ids, [{ id: 'my-page', segments: [':s1', ':s2', ':s3'], params: {} }])).toBe(1);

// The route has a mix of parameter and fixed segments.
expect(matchesIDs(ids, [{ id: 'my-page', segments: ['prefix'], params: {} }])).toBe(2);
expect(matchesIDs(ids, [{ id: 'my-page', segments: ['prefix', ':s1'], params: {} }])).toBe(1);
expect(matchesIDs(ids, [{ id: 'my-page', segments: ['prefix', ':s1', ':s2'], params: {} }])).toBe(1);
expect(matchesIDs(ids, [{ id: 'my-page', segments: ['prefix', ':s1', ':s2', ':s3'], params: {} }])).toBe(1);

// The route has parameters (componentProps).
expect(matchesIDs(ids, [{ id: 'my-page', segments: ['prefix'], params: {s2: 'route param'} }])).toBe(1);
expect(matchesIDs(ids, [{ id: 'my-page', segments: ['prefix', ':s1'], params: {s2: 'route param'} }])).toBe(1);
expect(matchesIDs(ids, [{ id: 'my-page', segments: ['prefix', ':s1', ':s2'], params: {s2: 'route param'} }])).toBe(1);
expect(matchesIDs(ids, [{ id: 'my-page', segments: ['prefix', ':s1', ':s2', ':s3'], params: {s2: 'route param'} }])).toBe(1);
})
});

describe('matchesSegments', () => {
Expand Down
60 changes: 28 additions & 32 deletions core/src/components/router/utils/matching.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,55 +34,51 @@ export const findRouteRedirect = (segments: string[], redirects: RouteRedirect[]
return redirects.find(redirect => matchesRedirect(segments, redirect));
};

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

let score = 0;

for (let i = 0; i < len; i++) {
const routeId = ids[i];
const routeChain = chain[i];
const routeEntry = chain[i];

// Skip results where the route id does not match the chain at the same index
if (routeId.id.toLowerCase() !== routeChain.id) {
if (routeId.id.toLowerCase() === routeEntry.id.toLowerCase()) {
score++;
} else {
break;
}

// Get the parameters of the route (that is the componentProps property of the <ion-route>).
const routeEntryParams = new Set(routeEntry.params ? Object.keys(routeEntry.params) : []);
// Add the parameter segments from the url.
for (const segment of routeEntry.segments) {
if (segment[0] === ':') {
routeEntryParams.add(segment.substring(1));
}
}

// The parameters should be the same for RouteID and RouteEntry.
if (routeId.params) {
const routeIdParams = Object.keys(routeId.params);
// Only compare routes with the chain that have the same number of parameters.
if (routeIdParams.length === routeChain.segments.length) {
// Maps the route's params into a path based on the path variable names,
// to compare against the route chain format.
//
// Before:
// ```ts
// {
// params: {
// s1: 'a',
// s2: 'b'
// }
// }
// ```
//
// After:
// ```ts
// [':s1',':s2']
// ```
//
const pathWithParams = routeIdParams.map(key => `:${key}`);
for (let j = 0; j < pathWithParams.length; j++) {
// Skip results where the path variable is not a match
if (pathWithParams[j].toLowerCase() !== routeChain.segments[j]) {
break;
}
// Weight path matches for the same index higher.
if (routeIdParams.length === routeEntryParams.size) {
const paramsMatch = routeIdParams.every(p => routeEntryParams.has(p));
if (paramsMatch) {
score++;
}

}
}
// Weight id matches
score++;
}

return score;
}

Expand Down

0 comments on commit 8d07126

Please sign in to comment.