Skip to content

Commit

Permalink
fix: Direct navigation to URL in SPA with vue-router in hash mode
Browse files Browse the repository at this point in the history
This fixes bug that triggers in quite specific situation - SPA mode
with Vue-router set to use hash mode.

In that setup if user navigates directly to an URL that will trigger
certain route and that route's `path` does not *exactly* match the
path user navigated to (for example due to extra trailing slash
or query parameter in user's URL), that triggers redirect that fails
due to target route being the same. That leaves current route in a
broken state. I assume because that route wasn't committed yet.

Fix by comparing routes in more sophisticated way than just comparing
route's path. Instead use VueRouter to resolve route from path and then
compare that with current route.

Resolves #490
  • Loading branch information
rchl committed Nov 7, 2019
1 parent 56e5d96 commit 0a9c4c8
Show file tree
Hide file tree
Showing 3 changed files with 113 additions and 5 deletions.
16 changes: 11 additions & 5 deletions src/plugins/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,13 @@ import {
vueI18n,
vuex
} from './options'
import { getLocaleDomain, getLocaleFromRoute, syncVuex, validateRouteParams } from './utils'
import {
getLocaleDomain,
getLocaleFromRoute,
isSameRoute,
syncVuex,
validateRouteParams
} from './utils'

Vue.use(VueI18n)

Expand Down Expand Up @@ -160,13 +166,13 @@ export default async (context) => {
await syncVuex(store, newLocale, app.i18n.getLocaleMessage(newLocale))

if (!initialSetup && strategy !== STRATEGIES.NO_PREFIX) {
const redirectPath = app.switchLocalePath(newLocale) || app.localePath('index', newLocale)
const redirectRoute = app.router.resolve(redirectPath).route

// Must retrieve from context as it might have changed since plugin initialization.
const { route } = context

const routeName = route && route.name ? app.getRouteBaseName(route) : 'index'
const redirectPath = app.localePath(Object.assign({}, route, { name: routeName }), newLocale)

if (route && route.path !== redirectPath) {
if (route && !isSameRoute(route, redirectRoute)) {
redirect(redirectPath)
}
}
Expand Down
57 changes: 57 additions & 0 deletions src/templates/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,63 @@ export const validateRouteParams = routeParams => {
})
}

const trailingSlashRE = /\/?$/

/**
* Determines if objects are equal.
*
* @param {Object} [a={}]
* @param {Object} [b={}]
* @return {boolean} True if objects equal, False otherwise.
*/
function isObjectEqual (a = {}, b = {}) {
// handle null value #1566
if (!a || !b) return a === b
const aKeys = Object.keys(a)
const bKeys = Object.keys(b)
if (aKeys.length !== bKeys.length) {
return false
}
return aKeys.every(key => {
const aVal = a[key]
const bVal = b[key]
// check nested equality
if (typeof aVal === 'object' && typeof bVal === 'object') {
return isObjectEqual(aVal, bVal)
}
return String(aVal) === String(bVal)
})
}

/**
* Determines if two routes are the same.
*
* @param {Route} a
* @param {Route} [b]
* @return {boolean} True if routes the same, False otherwise.
*/
export function isSameRoute (a, b) {
if (!b) {
return false
}
if (a.path && b.path) {
return (
a.path.replace(trailingSlashRE, '') === b.path.replace(trailingSlashRE, '') &&
a.hash === b.hash &&
isObjectEqual(a.query, b.query)
)
}
if (a.name && b.name) {
return (
a.name === b.name &&
a.hash === b.hash &&
isObjectEqual(a.query, b.query) &&
isObjectEqual(a.params, b.params)
)
}
return false
}

/**
* Get x-forwarded-host
* @param {object} req
Expand Down
45 changes: 45 additions & 0 deletions test/browser.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -115,3 +115,48 @@ describe(`${browserString} (generate)`, () => {
expect(await page.getText('body')).toContain('locale: en')
})
})

describe(`${browserString} (SPA with router in hash mode)`, () => {
let nuxt
let browser
let page

beforeAll(async () => {
const overrides = {
mode: 'spa',
router: {
mode: 'hash'
}
}

nuxt = (await setup(loadConfig(__dirname, 'basic', overrides, { merge: true }))).nuxt

browser = await createBrowser(browserString, {
staticServer: false,
extendPage (page) {
return {
navigate: createNavigator(page)
}
}
})
})

afterAll(async () => {
if (browser) {
await browser.close()
}

await nuxt.close()
})

// Issue https://github.com/nuxt-community/nuxt-i18n/issues/490
test('navigates directly to page with trailing slash', async () => {
page = await browser.page(url('/#/fr/'))
expect(await page.getText('body')).toContain('locale: fr')
})

test('navigates directly to page with query', async () => {
page = await browser.page(url('/#/fr?a=1'))
expect(await page.getText('body')).toContain('locale: fr')
})
})

0 comments on commit 0a9c4c8

Please sign in to comment.