Skip to content

Commit

Permalink
fix: redirect loop on initial load (static mode & route with no locale)
Browse files Browse the repository at this point in the history
On initial load, if:
  - the Nuxt target was set to "static"
  - the loaded route had locales disabled

Then we've attempted to redirect even when redirect path equaled the
current path. That has triggered an infinite redirect loop.

Fix by not setting a redirect when it matches the current path.

Resolves #798
  • Loading branch information
rchl committed Jul 31, 2020
1 parent 59e33cc commit 4c9bc13
Show file tree
Hide file tree
Showing 4 changed files with 80 additions and 10 deletions.
7 changes: 4 additions & 3 deletions src/templates/plugin.main.js
Original file line number Diff line number Diff line change
Expand Up @@ -118,9 +118,10 @@ export default async (context) => {
if (!redirectPath) {
// Current route could be 404 in which case attempt to find matching route for given locale.
redirectPath = app.localePath(route.fullPath, locale)
if (redirectPath === route.fullPath) {
return ''
}
}

if (redirectPath === route.fullPath) {
return ''
}

return redirectPath
Expand Down
50 changes: 46 additions & 4 deletions test/browser.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,48 @@ describe(browserString, () => {
})
})

describe(`${browserString}, target: static, trailingSlash: true`, () => {
/** @type {Nuxt} */
let nuxt
/** @type {import('playwright-chromium').ChromiumBrowser} */
let browser
/** @type {import('playwright-chromium').Page} */
let page

beforeAll(async () => {
const overrides = {
target: 'static',
router: {
trailingSlash: true
},
i18n: {
parsePages: false,
pages: {
'about-no-locale': false
}
}
}
nuxt = (await setup(loadConfig(__dirname, 'basic', overrides, { merge: true }))).nuxt
browser = await createBrowser()
})

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

await nuxt.close()
})

// Issue https://github.com/nuxt-community/i18n-module/issues/798
// Not specific to trailingSlash === true
test('does not trigger redirect loop on route with disabled locale', async () => {
page = await browser.newPage()
await page.goto(url('/about-no-locale/'), { waitUntil: 'load', timeout: 2000 })
expect(await (await page.$('body'))?.textContent()).toContain('page: About us')
})
})

describe(`${browserString} (generate)`, () => {
/** @type {import('playwright-chromium').ChromiumBrowser} */
let browser
Expand Down Expand Up @@ -208,9 +250,9 @@ describe(`${browserString} (generate)`, () => {
page = await browser.newPage()
await page.goto(server.getUrl('/'))
/**
* @param {string} route
* @param {string | undefined} [locale]
*/
* @param {string} route
* @param {string | undefined} [locale]
*/
const localePath = async (route, locale) => {
// @ts-ignore
return await page.evaluate(args => window.$nuxt.localePath(...args), [route, locale])
Expand All @@ -221,7 +263,7 @@ describe(`${browserString} (generate)`, () => {
})
})

describe(`${browserString} (generate, no subFolders, no trailingSlash)`, () => {
describe(`${browserString} (generate, no subFolders, trailingSlash === false)`, () => {
/** @type {import('playwright-chromium').ChromiumBrowser} */
let browser
/** @type {import('playwright-chromium').Page} */
Expand Down
17 changes: 17 additions & 0 deletions test/fixture/basic/pages/about-no-locale.vue
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<template>
<div>
<LangSwitcher />
<div id="current-page">page: {{ $t('about') }}</div>
<nuxt-link id="link-home" exact :to="localePath('index')">{{ $t('home') }}</nuxt-link>
</div>
</template>

<script>
import LangSwitcher from '../components/LangSwitcher'
export default {
components: {
LangSwitcher
}
}
</script>
16 changes: 13 additions & 3 deletions test/module.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1265,7 +1265,7 @@ describe('parsePages disabled', () => {
i18n: {
parsePages: false,
pages: {
about: false,
'about-no-locale': false,
simple: {
en: '/simple-en',
fr: '/simple-fr'
Expand All @@ -1290,8 +1290,18 @@ describe('parsePages disabled', () => {
})

test('navigates to route with paths disabled in pages option', async () => {
await expect(get('/about')).resolves.toBeDefined()
await expect(get('/fr/about')).rejects.toBeDefined()
await expect(get('/about-no-locale')).resolves.toBeDefined()
await expect(get('/fr/about-no-locale')).rejects.toBeDefined()
})

test('does not trigger redirect loop on route with disabled locale', async () => {
const requestOptions = {
followRedirect: false,
resolveWithFullResponse: true,
simple: false // Don't reject on non-2xx response
}
const response = await get('/about-no-locale', requestOptions)
expect(response.statusCode).toBe(200)
})
})

Expand Down

0 comments on commit 4c9bc13

Please sign in to comment.