fix(router): guards are now triggered on initial navigation (#23123)

resolves #22936
This commit is contained in:
Victor Berchet
2021-05-20 07:05:49 -07:00
committed by GitHub
parent 6bfe1ceee7
commit 56f6f56c66
12 changed files with 132 additions and 73 deletions

View File

@ -1914,7 +1914,7 @@ export namespace Components {
*/
"root": string;
/**
* The router can work in two "modes": - With hash: `/index.html#/path/to/page` - Without hash: `/path/to/page` Using one or another might depend in the requirements of your app and/or where it's deployed. Usually "hash-less" navigation works better for SEO and it's more user friendly too, but it might requires additional server-side configuration in order to properly work. On the otherside hash-navigation is much easier to deploy, it even works over the file protocol. By default, this property is `true`, change to `false` to allow hash-less URLs.
* The router can work in two "modes": - With hash: `/index.html#/path/to/page` - Without hash: `/path/to/page` Using one or another might depend in the requirements of your app and/or where it's deployed. Usually "hash-less" navigation works better for SEO and it's more user friendly too, but it might requires additional server-side configuration in order to properly work. On the other side hash-navigation is much easier to deploy, it even works over the file protocol. By default, this property is `true`, change to `false` to allow hash-less URLs.
*/
"useHash": boolean;
}
@ -5200,7 +5200,7 @@ declare namespace LocalJSX {
*/
"root"?: string;
/**
* The router can work in two "modes": - With hash: `/index.html#/path/to/page` - Without hash: `/path/to/page` Using one or another might depend in the requirements of your app and/or where it's deployed. Usually "hash-less" navigation works better for SEO and it's more user friendly too, but it might requires additional server-side configuration in order to properly work. On the otherside hash-navigation is much easier to deploy, it even works over the file protocol. By default, this property is `true`, change to `false` to allow hash-less URLs.
* The router can work in two "modes": - With hash: `/index.html#/path/to/page` - Without hash: `/path/to/page` Using one or another might depend in the requirements of your app and/or where it's deployed. Usually "hash-less" navigation works better for SEO and it's more user friendly too, but it might requires additional server-side configuration in order to properly work. On the other side hash-navigation is much easier to deploy, it even works over the file protocol. By default, this property is `true`, change to `false` to allow hash-less URLs.
*/
"useHash"?: boolean;
}

View File

@ -52,10 +52,10 @@ In order to configure this relationship between components (to load/select) and
## Properties
| Property | Attribute | Description | Type | Default |
| --------- | ---------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------- | ------- |
| `root` | `root` | By default `ion-router` will match the routes at the root path ("/"). That can be changed when | `string` | `'/'` |
| `useHash` | `use-hash` | The router can work in two "modes": - With hash: `/index.html#/path/to/page` - Without hash: `/path/to/page` Using one or another might depend in the requirements of your app and/or where it's deployed. Usually "hash-less" navigation works better for SEO and it's more user friendly too, but it might requires additional server-side configuration in order to properly work. On the otherside hash-navigation is much easier to deploy, it even works over the file protocol. By default, this property is `true`, change to `false` to allow hash-less URLs. | `boolean` | `true` |
| Property | Attribute | Description | Type | Default |
| --------- | ---------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------- | ------- |
| `root` | `root` | By default `ion-router` will match the routes at the root path ("/"). That can be changed when | `string` | `'/'` |
| `useHash` | `use-hash` | The router can work in two "modes": - With hash: `/index.html#/path/to/page` - Without hash: `/path/to/page` Using one or another might depend in the requirements of your app and/or where it's deployed. Usually "hash-less" navigation works better for SEO and it's more user friendly too, but it might requires additional server-side configuration in order to properly work. On the other side hash-navigation is much easier to deploy, it even works over the file protocol. By default, this property is `true`, change to `false` to allow hash-less URLs. | `boolean` | `true` |
## Events

View File

@ -40,7 +40,7 @@ export class Router implements ComponentInterface {
* Usually "hash-less" navigation works better for SEO and it's more user friendly too, but it might
* requires additional server-side configuration in order to properly work.
*
* On the otherside hash-navigation is much easier to deploy, it even works over the file protocol.
* On the other side hash-navigation is much easier to deploy, it even works over the file protocol.
*
* By default, this property is `true`, change to `false` to allow hash-less URLs.
*/
@ -57,11 +57,19 @@ export class Router implements ComponentInterface {
@Event() ionRouteDidChange!: EventEmitter<RouterEventDetail>;
async componentWillLoad() {
console.debug('[ion-router] router will load');
await waitUntilNavNode();
console.debug('[ion-router] found nav');
await this.onRoutesChanged();
const canProceed = await this.runGuards(this.getPath());
if (canProceed !== true) {
if (typeof canProceed === 'object') {
const { redirect } = canProceed;
const path = parsePath(redirect);
this.setPath(path.segments, ROUTER_INTENT_NONE, path.queryString);
await this.writeNavStateRoot(path.segments, ROUTER_INTENT_NONE);
}
} else {
await this.onRoutesChanged();
}
}
componentDidLoad() {
@ -72,17 +80,17 @@ export class Router implements ComponentInterface {
@Listen('popstate', { target: 'window' })
protected async onPopState() {
const direction = this.historyDirection();
let path = this.getPath();
let segments = this.getPath();
const canProceed = await this.runGuards(path);
const canProceed = await this.runGuards(segments);
if (canProceed !== true) {
if (typeof canProceed === 'object') {
path = parsePath(canProceed.redirect);
segments = parsePath(canProceed.redirect).segments;
} else {
return false;
}
return false;
}
console.debug('[ion-router] URL changed -> update nav', path, direction);
return this.writeNavStateRoot(path, direction);
return this.writeNavStateRoot(segments, direction);
}
@Listen('ionBackButton', { target: 'document' })
@ -119,23 +127,20 @@ export class Router implements ComponentInterface {
if (url.startsWith('.')) {
url = (new URL(url, window.location.href)).pathname;
}
console.debug('[ion-router] URL pushed -> updating nav', url, direction);
let path = parsePath(url);
let queryString = url.split('?')[1];
let parsedPath = parsePath(url);
const canProceed = await this.runGuards(path);
const canProceed = await this.runGuards(parsedPath.segments);
if (canProceed !== true) {
if (typeof canProceed === 'object') {
path = parsePath(canProceed.redirect);
queryString = canProceed.redirect.split('?')[1];
parsedPath = parsePath(canProceed.redirect)
} else {
return false;
}
}
this.setPath(path, direction, queryString);
return this.writeNavStateRoot(path, direction, animation);
this.setPath(parsedPath.segments, direction, parsedPath.queryString);
return this.writeNavStateRoot(parsedPath.segments, direction, animation);
}
/**
@ -150,8 +155,6 @@ export class Router implements ComponentInterface {
/** @internal */
@Method()
async printDebug() {
console.debug('CURRENT PATH', this.getPath());
console.debug('PREVIOUS PATH', this.previousPath);
printRoutes(readRoutes(this.el));
printRedirects(readRedirects(this.el));
}
@ -177,7 +180,6 @@ export class Router implements ComponentInterface {
return false;
}
console.debug('[ion-router] nav changed -> update URL', ids, path);
this.setPath(path, direction);
await this.safeWriteNavState(outlet, chain, ROUTER_INTENT_NONE, path, null, ids.length);
@ -272,24 +274,30 @@ export class Router implements ComponentInterface {
}
return resolve;
}
private async runGuards(to: string[] | null = this.getPath(), from: string[] | null = parsePath(this.previousPath)) {
// Executes the beforeLeave hook of the source route and the beforeEnter hook of the target route if they exist.
//
// When the beforeLeave hook does not return true (to allow navigating) then that value is returned early and the beforeEnter is executed.
// Otherwise the beforeEnterHook hook of the target route is executed.
private async runGuards(to: string[] | null = this.getPath(), from?: string[] | null) {
if (from === undefined) {
from = parsePath(this.previousPath).segments;
}
if (!to || !from) { return true; }
const routes = readRoutes(this.el);
const toChain = routerPathToChain(to, routes);
const fromChain = routerPathToChain(from, routes);
const beforeEnterHook = toChain && toChain[toChain.length - 1].beforeEnter;
const beforeLeaveHook = fromChain && fromChain[fromChain.length - 1].beforeLeave;
const canLeave = beforeLeaveHook ? await beforeLeaveHook() : true;
if (canLeave === false || typeof canLeave === 'object') { return canLeave; }
const canEnter = beforeEnterHook ? await beforeEnterHook() : true;
if (canEnter === false || typeof canEnter === 'object') { return canEnter; }
const toChain = routerPathToChain(to, routes);
const beforeEnterHook = toChain && toChain[toChain.length - 1].beforeEnter;
return true;
return beforeEnterHook ? beforeEnterHook() : true;
}
private async writeNavState(
@ -312,10 +320,6 @@ export class Router implements ComponentInterface {
const changed = await writeNavState(node, chain, direction, index, false, animation);
this.busy = false;
if (changed) {
console.debug('[ion-router] route changed', path);
}
// emit did change
if (routeEvent) {
this.ionRouteDidChange.emit(routeEvent);

View File

@ -77,7 +77,7 @@ describe('ionic-conference-app', () => {
});
export function getRouteIDs(path: string, routes: RouteChain[]): string[] {
return routerPathToChain(parsePath(path), routes)!.map(r => r.id);
return routerPathToChain(parsePath(path).segments, routes)!.map(r => r.id);
}
export function getRoutePath(ids: RouteID[], routes: RouteChain[]): string {

View File

@ -0,0 +1,16 @@
import { newE2EPage } from '@stencil/core/testing';
test('router: guards - guards should be run on initial load', async () => {
const page = await newE2EPage({
url: '/src/components/router/test/guards#/guard-initial-page?ionic:_testing=true'
});
await page.waitForChanges();
await checkUrl(page, '#/child/1');
});
const checkUrl = async (page, url: string) => {
const getUrl = await page.url();
expect(getUrl).toContain(url);
}

View File

@ -80,9 +80,26 @@
}
}
class GuardInitialPage extends HTMLElement {
connectedCallback() {
this.innerHTML = `
<ion-header>
<ion-toolbar>
<ion-buttons>
<ion-back-button></ion-back-button>
</ion-buttons>
<ion-title>Guard Initial Page</ion-title>
</ion-toolbar>
</ion-header>
<ion-content>
</ion-content>`;
}
}
customElements.define('home-page', HomePage);
customElements.define('child-page', ChildPage);
customElements.define('test-page', TestPage);
customElements.define('guard-initial-page', GuardInitialPage);
</script>
</head>
@ -146,6 +163,7 @@
<ion-route url="/home" component="home-page"></ion-route>
<ion-route url="/test" component="test-page"></ion-route>
<ion-route url="/child/:id" component="child-page"></ion-route>
<ion-route url="/guard-initial-page" component="guard-initial-page"></ion-route>
</ion-router>
<ion-nav></ion-nav>
@ -188,6 +206,12 @@
const page = document.querySelector('ion-route[component="child-page"]');
page.beforeEnter = allow;
page.beforeLeave = allow;
const guardPage = document.querySelector('ion-route[component="guard-initial-page"]');
guardPage.beforeEnter = () => {
return { redirect: '/child/1' };
}
</script>
</ion-app>
</body>

View File

@ -88,7 +88,6 @@ test('router: guards - router-link - allow/block', async () => {
await checkUrl(page, '#/child/1');
});
// TODO this is an actual bug in the code.
test('router: guards - router-link - allow/redirect', async () => {
const page = await newE2EPage({
url: '/src/components/router/test/guards?ionic:_testing=true'

View File

@ -88,7 +88,6 @@ test('router: guards - router.push - allow/block', async () => {
await checkUrl(page, '#/child');
});
// TODO this is an actual bug in the code.
test('router: guards - router.push - allow/redirect', async () => {
const page = await newE2EPage({
url: '/src/components/router/test/guards?ionic:_testing=true'

View File

@ -107,7 +107,7 @@ describe('matchesPath', () => {
{ id: '5', path: ['image'], params: { size: 'lg' } },
{ id: '5', path: ['image', ':size', ':type'], params: { size: 'mg' } },
];
const matched = matchesPath(parsePath('/profile/manu/image/image/large/retina'), chain);
const matched = matchesPath(parsePath('/profile/manu/image/image/large/retina').segments, chain);
expect(matched).toEqual([
{ id: '5', path: ['profile', ':name'], params: { name: 'manu' } },
{ id: '5', path: [''], params: undefined },

View File

@ -4,43 +4,50 @@ import { chainToPath, generatePath, parsePath, readPath, writePath } from '../ut
describe('parseURL', () => {
it('should parse empty path', () => {
expect(parsePath('')).toEqual(['']);
expect(parsePath('').segments).toEqual(['']);
});
it('should parse slash path', () => {
expect(parsePath('/')).toEqual(['']);
expect(parsePath(' / ')).toEqual(['']);
expect(parsePath('/').segments).toEqual(['']);
expect(parsePath(' / ').segments).toEqual(['']);
});
it('should parse empty path (2)', () => {
expect(parsePath(' ')).toEqual(['']);
expect(parsePath(' ').segments).toEqual(['']);
});
it('should parse null path', () => {
expect(parsePath(null)).toEqual(['']);
expect(parsePath(null).segments).toEqual(['']);
});
it('should parse undefined path', () => {
expect(parsePath(undefined)).toEqual(['']);
expect(parsePath(undefined).segments).toEqual(['']);
});
it('should parse single segment', () => {
expect(parsePath('path')).toEqual(['path']);
expect(parsePath('path/')).toEqual(['path']);
expect(parsePath('/path/')).toEqual(['path']);
expect(parsePath('/path')).toEqual(['path']);
expect(parsePath('path').segments).toEqual(['path']);
expect(parsePath('path/').segments).toEqual(['path']);
expect(parsePath('/path/').segments).toEqual(['path']);
expect(parsePath('/path').segments).toEqual(['path']);
});
it('should parse relative path', () => {
expect(parsePath('path/to/file.js')).toEqual(['path', 'to', 'file.js']);
expect(parsePath('path/to/file.js').segments).toEqual(['path', 'to', 'file.js']);
});
it('should parse absolute path', () => {
expect(parsePath('/path/to/file.js')).toEqual(['path', 'to', 'file.js']);
expect(parsePath('/path/to/file.js').segments).toEqual(['path', 'to', 'file.js']);
});
it('should parse relative path', () => {
expect(parsePath('/PATH///to//file.js//')).toEqual(['PATH', 'to', 'file.js']);
expect(parsePath('/PATH///to//file.js//').segments).toEqual(['PATH', 'to', 'file.js']);
});
it('should parse query string', () => {
expect(parsePath(null).queryString).toBe(undefined);
expect(parsePath('path/to/file.js').queryString).toBe(undefined);
expect(parsePath('path/to/file.js?').queryString).toEqual('');
expect(parsePath('path/to/file.js?a=b').queryString).toEqual('a=b');
});
});
describe('generatePath', () => {

View File

@ -7,8 +7,8 @@ export const readRedirects = (root: Element): RouteRedirect[] => {
.map(el => {
const to = readProp(el, 'to');
return {
from: parsePath(readProp(el, 'from')),
to: to == null ? undefined : parsePath(to),
from: parsePath(readProp(el, 'from')).segments,
to: to == null ? undefined : parsePath(to).segments,
};
});
};
@ -26,7 +26,7 @@ export const readRouteNodes = (root: Element, node = root): RouteTree => {
throw new Error('component missing in ion-route');
}
return {
path: parsePath(readProp(el, 'url')),
path: parsePath(readProp(el, 'url')).segments,
id: component.toLowerCase(),
params: el.componentProps,
beforeLeave: el.beforeLeave,

View File

@ -30,7 +30,7 @@ export const chainToPath = (chain: RouteChain): string[] | null => {
export const writePath = (history: History, root: string, useHash: boolean, path: string[], direction: RouterDirection, state: number, queryString?: string) => {
let url = generatePath([
...parsePath(root),
...parsePath(root).segments,
...path
]);
if (useHash) {
@ -73,23 +73,33 @@ export const readPath = (loc: Location, root: string, useHash: boolean): string[
: '';
}
const prefix = parsePath(root);
const path = parsePath(pathname);
const prefix = parsePath(root).segments;
const path = parsePath(pathname).segments;
return removePrefix(prefix, path);
};
export const parsePath = (path: string | undefined | null): string[] => {
if (path == null) {
return [''];
}
const removeQueryString = path.split('?')[0];
const segments = removeQueryString.split('/')
.map(s => s.trim())
.filter(s => s.length > 0);
// Parses the path to:
// - segments an array of '/' separated parts,
// - queryString (undefined when no query string).
export const parsePath = (path: string | undefined | null): {segments: string[], queryString?: string} => {
let segments = [''];
let queryString;
if (segments.length === 0) {
return [''];
} else {
return segments;
if (path != null) {
const qsStart = path.indexOf('?');
if (qsStart > -1) {
queryString = path.substr(qsStart + 1);
path = path.substr(0, qsStart);
}
segments = path.split('/')
.map(s => s.trim())
.filter(s => s.length > 0);
if (segments.length === 0) {
segments = [''];
}
}
return { segments, queryString };
};