Skip to content

Commit

Permalink
Ensure i18n with basePath works properly (#19083)
Browse files Browse the repository at this point in the history
  • Loading branch information
ijjk committed Nov 12, 2020
1 parent 41866a1 commit 54b7042
Show file tree
Hide file tree
Showing 30 changed files with 3,233 additions and 1,950 deletions.
2 changes: 1 addition & 1 deletion packages/next/build/entries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ export function createEntrypoints(
assetPrefix: config.assetPrefix,
generateEtags: config.generateEtags,
poweredByHeader: config.poweredByHeader,
canonicalBase: config.canonicalBase,
canonicalBase: config.amp.canonicalBase,
basePath: config.basePath,
runtimeConfig: hasRuntimeConfig
? JSON.stringify({
Expand Down
7 changes: 4 additions & 3 deletions packages/next/build/webpack/loaders/next-serverless-loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
BUILD_MANIFEST,
REACT_LOADABLE_MANIFEST,
ROUTES_MANIFEST,
TEMPORARY_REDIRECT_STATUS,
} from '../../../next-server/lib/constants'
import { isDynamicRoute } from '../../../next-server/lib/router/utils'
import { __ApiPreviewProps } from '../../../next-server/server/api-utils'
Expand Down Expand Up @@ -361,11 +362,11 @@ const nextServerlessLoader: loader.Loader = function () {
localeDomainRedirect
? localeDomainRedirect
: shouldStripDefaultLocale
? '/'
: \`/\${detectedLocale}\`,
? "${basePath}" || '/'
: \`${basePath}/\${detectedLocale}\`,
})
)
res.statusCode = 307
res.statusCode = ${TEMPORARY_REDIRECT_STATUS}
res.end()
return
}
Expand Down
25 changes: 13 additions & 12 deletions packages/next/next-server/lib/router/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,8 @@ export default class Router implements BaseRouter {
this.changeState(
'replaceState',
formatWithValidation({ pathname: addBasePath(pathname), query }),
getURL()
getURL(),
{ locale }
)
}

Expand Down Expand Up @@ -621,7 +622,7 @@ export default class Router implements BaseRouter {
normalizeLocalePath,
} = require('../i18n/normalize-locale-path') as typeof import('../i18n/normalize-locale-path')

const parsedAs = parseRelativeUrl(as)
const parsedAs = parseRelativeUrl(hasBasePath(as) ? delBasePath(as) : as)

const localePathResult = normalizeLocalePath(
parsedAs.pathname,
Expand All @@ -630,7 +631,7 @@ export default class Router implements BaseRouter {

if (localePathResult.detectedLocale) {
this.locale = localePathResult.detectedLocale
url = localePathResult.pathname
url = addBasePath(localePathResult.pathname)
}
}

Expand All @@ -646,8 +647,13 @@ export default class Router implements BaseRouter {
this.abortComponentLoad(this._inFlightRoute)
}

as = addLocale(as, options.locale, this.defaultLocale)

as = addBasePath(
addLocale(
hasBasePath(as) ? delBasePath(as) : as,
options.locale,
this.defaultLocale
)
)
const cleanedAs = delLocale(
hasBasePath(as) ? delBasePath(as) : as,
this.locale
Expand Down Expand Up @@ -848,12 +854,7 @@ export default class Router implements BaseRouter {
}

Router.events.emit('beforeHistoryChange', as)
this.changeState(
method,
url,
addLocale(as, options.locale, this.defaultLocale),
options
)
this.changeState(method, url, as, options)

if (process.env.NODE_ENV !== 'production') {
const appComp: any = this.components['/_app'].Component
Expand Down Expand Up @@ -1230,7 +1231,7 @@ export default class Router implements BaseRouter {

const pages = await this.pageLoader.getPageList()

parsed = this._resolveHref(parsed, pages) as typeof parsed
parsed = this._resolveHref(parsed, pages, false) as typeof parsed

if (parsed.pathname !== pathname) {
pathname = parsed.pathname
Expand Down
18 changes: 10 additions & 8 deletions packages/next/next-server/server/next-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -315,9 +315,11 @@ export default class Server {
req.url = req.url!.replace(basePath, '') || '/'
}

if (i18n && !parsedUrl.pathname?.startsWith('/_next')) {
if (i18n && !req.url?.startsWith('/_next')) {
// get pathname from URL with basePath stripped for locale detection
const { pathname, ...parsed } = parseUrl(req.url || '/')
let { pathname, ...parsed } = parseUrl(req.url || '/')
pathname = pathname || '/'

let defaultLocale = i18n.defaultLocale
let detectedLocale = detectLocaleCookie(req, i18n.locales)
let acceptPreferredLocale =
Expand Down Expand Up @@ -348,13 +350,13 @@ export default class Server {
pathname: localePathResult.pathname,
})
;(req as any).__nextStrippedLocale = true
parsedUrl.pathname = localePathResult.pathname
parsedUrl.pathname = `${basePath || ''}${localePathResult.pathname}`
}

// If a detected locale is a domain specific locale and we aren't already
// on that domain and path prefix redirect to it to prevent duplicate
// content from multiple domains
if (detectedDomain && parsedUrl.pathname === '/') {
if (detectedDomain && pathname === '/') {
const localeToCheck = acceptPreferredLocale
// const localeToCheck = localePathResult.detectedLocale
// ? detectedLocale
Expand Down Expand Up @@ -429,8 +431,8 @@ export default class Server {
pathname: localeDomainRedirect
? localeDomainRedirect
: shouldStripDefaultLocale
? '/'
: `/${detectedLocale}`,
? basePath || `/`
: `${basePath || ''}/${detectedLocale}`,
})
)
res.statusCode = 307
Expand Down Expand Up @@ -710,7 +712,7 @@ export default class Server {
type: redirectRoute.type,
match: redirectRoute.match,
statusCode: redirectRoute.statusCode,
name: `Redirect route`,
name: `Redirect route ${redirectRoute.source}`,
fn: async (req, res, params, parsedUrl) => {
const { parsedDestination } = prepareDestination(
redirectRoute.destination,
Expand Down Expand Up @@ -750,7 +752,7 @@ export default class Server {
...rewriteRoute,
check: true,
type: rewriteRoute.type,
name: `Rewrite route`,
name: `Rewrite route ${rewriteRoute.source}`,
match: rewriteRoute.match,
fn: async (req, res, params, parsedUrl) => {
const { newUrl, parsedDestination } = prepareDestination(
Expand Down
8 changes: 8 additions & 0 deletions packages/next/next-server/server/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -183,9 +183,17 @@ export default class Router {
(req as any).__nextStrippedLocale &&
parsedUrl.query.__nextLocale
) {
if (keepBasePath) {
currentPathname = replaceBasePath(this.basePath, currentPathname!)
}

currentPathname = `/${parsedUrl.query.__nextLocale}${
currentPathname === '/' ? '' : currentPathname
}`

if (keepBasePath) {
currentPathname = `${this.basePath}${currentPathname}`
}
}

const newParams = testRoute.match(currentPathname)
Expand Down
2 changes: 1 addition & 1 deletion test/integration/build-output/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ describe('Build Output', () => {
expect(parseFloat(err404Size) - 3.6).toBeLessThanOrEqual(0)
expect(err404Size.endsWith('kB')).toBe(true)

expect(parseFloat(err404FirstLoad) - 64.9).toBeLessThanOrEqual(0)
expect(parseFloat(err404FirstLoad) - 65).toBeLessThanOrEqual(0)
expect(err404FirstLoad.endsWith('kB')).toBe(true)

expect(parseFloat(sharedByAll) - 61.5).toBeLessThanOrEqual(0)
Expand Down
91 changes: 91 additions & 0 deletions test/integration/i18n-support-base-path/next.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
module.exports = {
// target: 'experimental-serverless-trace',
// basePath: '/docs',
i18n: {
// localeDetection: false,
locales: ['nl-NL', 'nl-BE', 'nl', 'fr-BE', 'fr', 'en-US', 'en'],
defaultLocale: 'en-US',
domains: [
{
// used for testing, this should not be needed in most cases
// as production domains should always use https
http: true,
domain: 'example.be',
defaultLocale: 'nl-BE',
locales: ['nl', 'nl-NL', 'nl-BE'],
},
{
http: true,
domain: 'example.fr',
defaultLocale: 'fr',
locales: ['fr-BE'],
},
],
},
async redirects() {
return [
{
source: '/en-US/redirect',
destination: '/somewhere-else',
permanent: false,
},
{
source: '/nl/redirect',
destination: '/somewhere-else',
permanent: false,
},
{
source: '/redirect',
destination: '/somewhere-else',
permanent: false,
},
]
},
async rewrites() {
return [
{
source: '/en-US/rewrite',
destination: '/another',
},
{
source: '/nl/rewrite',
destination: '/another',
},
{
source: '/rewrite',
destination: '/another',
},
]
},
async headers() {
return [
{
source: '/en-US/add-header',
headers: [
{
key: 'x-hello',
value: 'world',
},
],
},
{
source: '/nl/add-header',
headers: [
{
key: 'x-hello',
value: 'world',
},
],
},
{
source: '/add-header',
headers: [
{
key: 'x-hello',
value: 'world',
},
],
},
]
},
}
19 changes: 19 additions & 0 deletions test/integration/i18n-support-base-path/pages/404.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
export default function NotFound(props) {
return (
<>
<h1 id="not-found">This page could not be found | 404</h1>
<p id="props">{JSON.stringify(props)}</p>
</>
)
}

export const getStaticProps = ({ locale, locales, defaultLocale }) => {
return {
props: {
locale,
locales,
defaultLocale,
is404: true,
},
}
}
19 changes: 19 additions & 0 deletions test/integration/i18n-support-base-path/pages/_app.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
if (typeof window !== 'undefined') {
window.caughtWarns = []

const origWarn = window.console.warn
const origError = window.console.error

window.console.warn = function (...args) {
window.caughtWarns.push(args.join(' '))
origWarn(...args)
}
window.console.error = function (...args) {
window.caughtWarns.push(args.join(' '))
origError(...args)
}
}

export default function MyApp({ Component, pageProps }) {
return <Component {...pageProps} />
}
32 changes: 32 additions & 0 deletions test/integration/i18n-support-base-path/pages/amp/amp-first.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { useAmp } from 'next/amp'
import { useRouter } from 'next/router'

export const config = {
amp: true,
}

export default function Page(props) {
const router = useRouter()

return (
<>
<p id="another">another page</p>
<p id="is-amp">{useAmp() ? 'yes' : 'no'}</p>
<p id="props">{JSON.stringify(props)}</p>
<p id="router-locale">{router.locale}</p>
<p id="router-locales">{JSON.stringify(router.locales)}</p>
<p id="router-query">{JSON.stringify(router.query)}</p>
<p id="router-pathname">{router.pathname}</p>
<p id="router-as-path">{router.asPath}</p>
</>
)
}

export const getServerSideProps = ({ locale, locales }) => {
return {
props: {
locale,
locales,
},
}
}
23 changes: 23 additions & 0 deletions test/integration/i18n-support-base-path/pages/amp/amp-hybrid.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { useAmp } from 'next/amp'
import { useRouter } from 'next/router'

export const config = {
amp: 'hybrid',
}

export default function Page(props) {
const router = useRouter()

return (
<>
<p id="another">another page</p>
<p id="is-amp">{useAmp() ? 'yes' : 'no'}</p>
<p id="props">{JSON.stringify(props)}</p>
<p id="router-locale">{router.locale}</p>
<p id="router-locales">{JSON.stringify(router.locales)}</p>
<p id="router-query">{JSON.stringify(router.query)}</p>
<p id="router-pathname">{router.pathname}</p>
<p id="router-as-path">{router.asPath}</p>
</>
)
}

0 comments on commit 54b7042

Please sign in to comment.