From: Eduardo San Martin Morote Date: Fri, 13 Nov 2020 17:45:29 +0000 (+0100) Subject: fix: trigger redirect on popstate (#592) X-Git-Tag: v4.0.0-rc.3~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=18dbdc2745cf7bd2516d4576a8d6a21de78516ec;p=thirdparty%2Fvuejs%2Frouter.git fix: trigger redirect on popstate (#592) --- diff --git a/__tests__/warnings.spec.ts b/__tests__/warnings.spec.ts index 4dfa8ac5..5273b0fb 100644 --- a/__tests__/warnings.spec.ts +++ b/__tests__/warnings.spec.ts @@ -15,7 +15,9 @@ describe('warnings', () => { { path: '/redirect', redirect: { params: { foo: 'f' } } }, ], }) - await router.push('/redirect').catch(() => {}) + try { + await router.push('/redirect') + } catch (err) {} expect('Invalid redirect found').toHaveBeenWarned() }) diff --git a/e2e/hash/index.ts b/e2e/hash/index.ts index 190efe38..ebf448c0 100644 --- a/e2e/hash/index.ts +++ b/e2e/hash/index.ts @@ -23,6 +23,7 @@ const router = createRouter({ history: createWebHashHistory('/' + __dirname + '/#/'), routes: [ { path: '/', component: Home }, + { path: '/redirect', name: 'redirect', redirect: '/foo' }, { path: '/foo', component: Foo }, { path: '/bar', component: Bar }, { path: '/unicode/:id', name: 'unicode', component: Unicode }, diff --git a/e2e/specs/hash.js b/e2e/specs/hash.js index 7d0beea5..33b38f6c 100644 --- a/e2e/specs/hash.js +++ b/e2e/specs/hash.js @@ -92,4 +92,20 @@ module.exports = { .end() }, + + /** @type {import('nightwatch').NightwatchTest} */ + 'manual hash change to trigger redirect': function (browser) { + browser + .url(baseURL + '/') + .waitForElementPresent('#app > *', 1000) + .assert.containsText('.view', 'home') + + .execute(function () { + window.location.hash = '#/redirect' + }) + .assert.containsText('.view', 'Foo') + .assert.urlEquals(baseURL + '/foo') + + .end() + }, } diff --git a/src/router.ts b/src/router.ts index 7d5cc326..5af46bc6 100644 --- a/src/router.ts +++ b/src/router.ts @@ -545,24 +545,13 @@ export function createRouter(options: RouterOptions): Router { return push(assign(locationAsObject(to), { replace: true })) } - function pushWithRedirect( - to: RouteLocationRaw | RouteLocation, - redirectedFrom?: RouteLocation - ): Promise { - const targetLocation: RouteLocation = (pendingLocation = resolve(to)) - const from = currentRoute.value - const data: HistoryState | undefined = (to as RouteLocationOptions).state - const force: boolean | undefined = (to as RouteLocationOptions).force - // to could be a string where `replace` is a function - const replace = (to as RouteLocationOptions).replace === true - - const lastMatched = - targetLocation.matched[targetLocation.matched.length - 1] + function handleRedirectRecord(to: RouteLocation): RouteLocationRaw | void { + const lastMatched = to.matched[to.matched.length - 1] if (lastMatched && lastMatched.redirect) { const { redirect } = lastMatched // transform it into an object to pass the original RouteLocaleOptions let newTargetLocation = locationAsObject( - typeof redirect === 'function' ? redirect(targetLocation) : redirect + typeof redirect === 'function' ? redirect(to) : redirect ) if ( @@ -576,29 +565,42 @@ export function createRouter(options: RouterOptions): Router { null, 2 )}\n when navigating to "${ - targetLocation.fullPath + to.fullPath }". A redirect must contain a name or path. This will break in production.` ) - return Promise.reject(new Error('Invalid redirect')) + throw new Error('Invalid redirect') } + + return assign( + { + query: to.query, + hash: to.hash, + params: to.params, + }, + newTargetLocation + ) + } + } + + function pushWithRedirect( + to: RouteLocationRaw | RouteLocation, + redirectedFrom?: RouteLocation + ): Promise { + const targetLocation: RouteLocation = (pendingLocation = resolve(to)) + const from = currentRoute.value + const data: HistoryState | undefined = (to as RouteLocationOptions).state + const force: boolean | undefined = (to as RouteLocationOptions).force + // to could be a string where `replace` is a function + const replace = (to as RouteLocationOptions).replace === true + + const shouldRedirect = handleRedirectRecord(targetLocation) + + if (shouldRedirect) return pushWithRedirect( - assign( - { - query: targetLocation.query, - hash: targetLocation.hash, - params: targetLocation.params, - }, - newTargetLocation, - { - state: data, - force, - replace, - } - ), + assign(shouldRedirect, { state: data, force, replace }), // keep original redirectedFrom if it exists redirectedFrom || targetLocation ) - } // if it was a redirect we already called `pushWithRedirect` above const toLocation = targetLocation as RouteLocationNormalized @@ -616,7 +618,7 @@ export function createRouter(options: RouterOptions): Router { from, from, // this is a push, the only way for it to be triggered from a - // history.listen is with a redirect, which makes it become a pus + // history.listen is with a redirect, which makes it become a push true, // This cannot be the first navigation because the initial location // cannot be manually navigated to @@ -625,20 +627,12 @@ export function createRouter(options: RouterOptions): Router { } return (failure ? Promise.resolve(failure) : navigate(toLocation, from)) - .catch((error: NavigationFailure | NavigationRedirectError) => { - if ( - isNavigationFailure( - error, - ErrorTypes.NAVIGATION_ABORTED | - ErrorTypes.NAVIGATION_CANCELLED | - ErrorTypes.NAVIGATION_GUARD_REDIRECT - ) - ) { - return error - } - // unknown error, rejects - return triggerError(error) - }) + .catch((error: NavigationFailure | NavigationRedirectError) => + isNavigationFailure(error) + ? error + : // reject any unknown error + triggerError(error) + ) .then((failure: NavigationFailure | NavigationRedirectError | void) => { if (failure) { if ( @@ -896,7 +890,19 @@ export function createRouter(options: RouterOptions): Router { function setupListeners() { removeHistoryListener = routerHistory.listen((to, _from, info) => { // cannot be a redirect route because it was in history - const toLocation = resolve(to) as RouteLocationNormalized + let toLocation = resolve(to) as RouteLocationNormalized + + // due to dynamic routing, and to hash history with manual navigation + // (manually changing the url or calling history.hash = '#/somewhere'), + // there could be a redirect record in history + const shouldRedirect = handleRedirectRecord(toLocation) + if (shouldRedirect) { + pushWithRedirect( + assign(shouldRedirect, { replace: true }), + toLocation + ).catch(noop) + return + } pendingLocation = toLocation const from = currentRoute.value @@ -927,9 +933,10 @@ export function createRouter(options: RouterOptions): Router { // the error is already handled by router.push we just want to avoid // logging the error pushWithRedirect( + // TODO: should we force replace: true (error as NavigationRedirectError).to, toLocation - // avoid an uncaught rejection + // avoid an uncaught rejection, let push call triggerError ).catch(noop) // avoid the then branch return Promise.reject()