Skip to content

Commit

Permalink
Do not add locale to link for api route and middleware preflight (#35994
Browse files Browse the repository at this point in the history
)

Fixes #33578

This PR fixes `Link` component as well to not add locale for api routes since both preflight redirect and link should work the same.

Additionally, it fixes the webdriver method `browser.waitForCondition()` for playwright to not wrap with function string.

## Bug

- [x] Related issues linked using `fixes #number`
- [x] Integration tests added
- [ ] Errors have helpful link attached, see `contributing.md`
  • Loading branch information
nkzawa committed Apr 11, 2022
1 parent aaa823c commit 93678b5
Show file tree
Hide file tree
Showing 8 changed files with 53 additions and 17 deletions.
28 changes: 17 additions & 11 deletions packages/next/shared/lib/router/router.ts
Expand Up @@ -125,6 +125,11 @@ function addPathPrefix(path: string, prefix?: string) {
)
}

function hasPathPrefix(path: string, prefix: string) {
path = pathNoQueryHash(path)
return path === prefix || path.startsWith(prefix + '/')
}

export function getDomainLocale(
path: string,
locale?: string | false,
Expand Down Expand Up @@ -153,16 +158,18 @@ export function addLocale(
defaultLocale?: string
) {
if (process.env.__NEXT_I18N_SUPPORT) {
const pathname = pathNoQueryHash(path)
const pathLower = pathname.toLowerCase()
const localeLower = locale && locale.toLowerCase()
if (locale && locale !== defaultLocale) {
const pathname = pathNoQueryHash(path)
const pathLower = pathname.toLowerCase()
const localeLower = locale.toLowerCase()

return locale &&
locale !== defaultLocale &&
!pathLower.startsWith('/' + localeLower + '/') &&
pathLower !== '/' + localeLower
? addPathPrefix(path, '/' + locale)
: path
if (
!hasPathPrefix(pathLower, '/' + localeLower) &&
!hasPathPrefix(pathLower, '/api')
) {
return addPathPrefix(path, '/' + locale)
}
}
}
return path
}
Expand Down Expand Up @@ -194,8 +201,7 @@ function pathNoQueryHash(path: string) {
}

export function hasBasePath(path: string): boolean {
path = pathNoQueryHash(path)
return path === basePath || path.startsWith(basePath + '/')
return hasPathPrefix(path, basePath)
}

export function addBasePath(path: string): string {
Expand Down
4 changes: 4 additions & 0 deletions test/integration/i18n-support/pages/index.js
Expand Up @@ -47,6 +47,10 @@ export default function Page(props) {
<a id="to-gssp-slug">to /gssp/first</a>
</Link>
<br />
<Link href="/api/post/asdf">
<a id="to-api-post">to /api/post/[slug]</a>
</Link>
<br />
</>
)
}
Expand Down
10 changes: 10 additions & 0 deletions test/integration/i18n-support/test/index.test.js
Expand Up @@ -495,6 +495,16 @@ describe('i18n Support', () => {
expect(await browser.eval('window.location.pathname')).toBe(
`${localePath}gssp/first/`
)

await browser.back().waitForElementByCss('#index')
await browser.elementByCss('#to-api-post').click()

await browser.waitForCondition(
'window.location.pathname === "/api/post/asdf/"'
)
const body = await browser.elementByCss('body').text()
const json = JSON.parse(body)
expect(json.post).toBe(true)
}
})
}
Expand Down
3 changes: 3 additions & 0 deletions test/integration/middleware/core/pages/api/ok.js
@@ -0,0 +1,3 @@
export default function handler(req, res) {
res.send('ok')
}
Expand Up @@ -58,4 +58,10 @@ export async function middleware(request) {
url.pathname = '/redirects/infinite-loop'
return Response.redirect(url)
}

if (url.pathname === '/redirects/to') {
url.pathname = url.searchParams.get('pathname')
url.searchParams.delete('pathname')
return Response.redirect(url)
}
}
4 changes: 4 additions & 0 deletions test/integration/middleware/core/pages/redirects/index.js
Expand Up @@ -28,6 +28,10 @@ export default function Home() {
<a>Redirect me alot (infinite loop)</a>
</Link>
<div />
<Link href="/redirects/to?pathname=/api/ok" locale="nl">
<a id="link-to-api-with-locale">>Redirect me to api with locale</a>
</Link>
<div />
</div>
)
}
8 changes: 8 additions & 0 deletions test/integration/middleware/core/test/index.test.js
Expand Up @@ -579,6 +579,14 @@ function redirectTests(locale = '') {
fetchViaHTTP(context.appPort, `${locale}/redirects/infinite-loop`)
).rejects.toThrow()
})

it(`${locale} should redirect to api route with locale`, async () => {
const browser = await webdriver(context.appPort, `${locale}/redirects`)
await browser.elementByCss('#link-to-api-with-locale').click()
await browser.waitForCondition('window.location.pathname === "/api/ok"')
const body = await browser.elementByCss('body').text()
expect(body).toBe('ok')
})
}

function responseTests(locale = '') {
Expand Down
7 changes: 1 addition & 6 deletions test/lib/browsers/playwright.ts
Expand Up @@ -315,12 +315,7 @@ class Playwright extends BrowserInterface {

waitForCondition(condition, timeout) {
return this.chain(() => {
return page.waitForFunction(
`function() {
return ${condition}
}`,
{ timeout }
)
return page.waitForFunction(condition, { timeout })
})
}

Expand Down

0 comments on commit 93678b5

Please sign in to comment.