Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only create locale domain links if on locale domain #22032

Merged
merged 3 commits into from
Feb 11, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,7 @@ export function getUtils({
if (detectedDomain) {
defaultLocale = detectedDomain.defaultLocale
detectedLocale = defaultLocale
;(req as any).__nextIsLocaleDomain = true
}

// if not domain specific locale use accept-language preferred
Expand Down
17 changes: 11 additions & 6 deletions packages/next/client/link.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -301,12 +301,17 @@ function Link(props: React.PropsWithChildren<LinkProps>) {
const curLocale =
typeof locale !== 'undefined' ? locale : router && router.locale

const localeDomain = getDomainLocale(
as,
curLocale,
router && router.locales,
router && router.domainLocales
)
// we only render domain locales if we are currently on a domain locale
// so that locale links are still visitable in development/preview envs
const localeDomain =
router &&
router.isLocaleDomain &&
getDomainLocale(
as,
curLocale,
router && router.locales,
router && router.domainLocales
)

childProps.href =
localeDomain ||
Expand Down
1 change: 1 addition & 0 deletions packages/next/client/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ const urlPropertyFields = [
'locales',
'defaultLocale',
'isReady',
'isLocaleDomain',
]
const routerEvents = [
'routeChangeStart',
Expand Down
8 changes: 8 additions & 0 deletions packages/next/next-server/lib/router/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,7 @@ export type BaseRouter = {
locales?: string[]
defaultLocale?: string
domainLocales?: DomainLocales
isLocaleDomain: boolean
}

export type NextRouter = BaseRouter &
Expand Down Expand Up @@ -488,6 +489,7 @@ export default class Router implements BaseRouter {
defaultLocale?: string
domainLocales?: DomainLocales
isReady: boolean
isLocaleDomain: boolean

private _idx: number = 0

Expand Down Expand Up @@ -579,12 +581,17 @@ export default class Router implements BaseRouter {
self.__NEXT_DATA__.gip ||
(!autoExportDynamic && !self.location.search)
)
this.isLocaleDomain = false

if (process.env.__NEXT_I18N_SUPPORT) {
this.locale = locale
this.locales = locales
this.defaultLocale = defaultLocale
this.domainLocales = domainLocales
this.isLocaleDomain = !!detectDomainLocale(
domainLocales,
self.location.hostname
)
}

if (typeof window !== 'undefined') {
Expand Down Expand Up @@ -817,6 +824,7 @@ export default class Router implements BaseRouter {
if (
!didNavigate &&
detectedDomain &&
this.isLocaleDomain &&
self.location.hostname !== detectedDomain.domain
) {
const asNoBasePath = delBasePath(as)
Expand Down
1 change: 1 addition & 0 deletions packages/next/next-server/server/next-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -443,6 +443,7 @@ export default class Server {
if (detectedDomain) {
defaultLocale = detectedDomain.defaultLocale
detectedLocale = defaultLocale
;(req as any).__nextIsLocaleDomain = true
}

// if not domain specific locale use accept-language preferred
Expand Down
14 changes: 9 additions & 5 deletions packages/next/next-server/server/render.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ class ServerRouter implements NextRouter {
locales?: string[]
defaultLocale?: string
domainLocales?: DomainLocales
isLocaleDomain: boolean
// TODO: Remove in the next major version, as this would mean the user is adding event listeners in server-side `render` method
static events: MittEmitter = mitt()

Expand All @@ -90,10 +91,11 @@ class ServerRouter implements NextRouter {
{ isFallback }: { isFallback: boolean },
isReady: boolean,
basePath: string,
locale?: string,
locales?: string[],
defaultLocale?: string,
domainLocales?: DomainLocales
locale: string | undefined,
locales: string[] | undefined,
defaultLocale: string | undefined,
domainLocales: DomainLocales | undefined,
isLocaleDomain: boolean
) {
this.route = pathname.replace(/\/$/, '') || '/'
this.pathname = pathname
Expand All @@ -106,6 +108,7 @@ class ServerRouter implements NextRouter {
this.defaultLocale = defaultLocale
this.isReady = isReady
this.domainLocales = domainLocales
this.isLocaleDomain = isLocaleDomain
}

push(): any {
Expand Down Expand Up @@ -550,7 +553,8 @@ export async function renderToHTML(
renderOpts.locale,
renderOpts.locales,
renderOpts.defaultLocale,
renderOpts.domainLocales
renderOpts.domainLocales,
(req as any).__nextIsLocaleDomain
)
const ctx = {
err,
Expand Down
48 changes: 46 additions & 2 deletions test/integration/i18n-support/test/shared.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,9 @@ export function runTests(ctx) {
})
})

it('should redirect to locale domain correctly client-side', async () => {
// this test can not currently be tested in browser without modifying the
// host resolution since it needs a domain to test locale domains behavior
it.skip('should redirect to locale domain correctly client-side', async () => {
const browser = await webdriver(ctx.appPort, `${ctx.basePath || '/'}`)

await browser.eval(`(function() {
Expand Down Expand Up @@ -104,7 +106,9 @@ export function runTests(ctx) {
)
})

it('should render the correct href for locale domain', async () => {
// this test can not currently be tested in browser without modifying the
// host resolution since it needs a domain to test locale domains behavior
it.skip('should render the correct href for locale domain', async () => {
let browser = await webdriver(
ctx.appPort,
`${ctx.basePath || ''}/links?nextLocale=go`
Expand Down Expand Up @@ -142,6 +146,46 @@ export function runTests(ctx) {
}
})

it('should render the correct href with locale domains but not on a locale domain', async () => {
let browser = await webdriver(
ctx.appPort,
`${ctx.basePath || ''}/links?nextLocale=go`
)

for (const [element, pathname] of [
['#to-another', '/another'],
['#to-gsp', '/gsp'],
['#to-fallback-first', '/gsp/fallback/first'],
['#to-fallback-hello', '/gsp/fallback/hello'],
['#to-gssp', '/gssp'],
['#to-gssp-slug', '/gssp/first'],
]) {
const href = await browser.elementByCss(element).getAttribute('href')
const { hostname, pathname: hrefPathname } = url.parse(href)
expect(hostname).not.toBe('example.com')
expect(hrefPathname).toBe(`${ctx.basePath || ''}/go${pathname}`)
}

browser = await webdriver(
ctx.appPort,
`${ctx.basePath || ''}/links?nextLocale=go-BE`
)

for (const [element, pathname] of [
['#to-another', '/another'],
['#to-gsp', '/gsp'],
['#to-fallback-first', '/gsp/fallback/first'],
['#to-fallback-hello', '/gsp/fallback/hello'],
['#to-gssp', '/gssp'],
['#to-gssp-slug', '/gssp/first'],
]) {
const href = await browser.elementByCss(element).getAttribute('href')
const { hostname, pathname: hrefPathname } = url.parse(href)
expect(hostname).not.toBe('example.com')
expect(hrefPathname).toBe(`${ctx.basePath || ''}/go-BE${pathname}`)
}
})

it('should navigate through history with query correctly', async () => {
const browser = await webdriver(ctx.appPort, `${ctx.basePath || '/'}`)

Expand Down