From 1c13acc4ca646b50f372a01864eb6d6dc4921bc0 Mon Sep 17 00:00:00 2001 From: Rob Stanford Date: Thu, 13 Jun 2024 14:15:40 +0100 Subject: [PATCH 1/5] fix: middleware i18n normalization --- edge-runtime/lib/next-request.ts | 39 ++++++++++++-------------------- 1 file changed, 15 insertions(+), 24 deletions(-) diff --git a/edge-runtime/lib/next-request.ts b/edge-runtime/lib/next-request.ts index 45744a882c..5673109a08 100644 --- a/edge-runtime/lib/next-request.ts +++ b/edge-runtime/lib/next-request.ts @@ -1,6 +1,12 @@ import type { Context } from '@netlify/edge-functions' -import { addBasePath, normalizeDataUrl, normalizeLocalePath, removeBasePath } from './util.ts' +import { + addBasePath, + addTrailingSlash, + normalizeDataUrl, + normalizeLocalePath, + removeBasePath, +} from './util.ts' interface I18NConfig { defaultLocale: string @@ -40,44 +46,29 @@ const normalizeRequestURL = ( nextConfig?: RequestData['nextConfig'], ): { url: string; detectedLocale?: string } => { const url = new URL(originalURL) - - url.pathname = removeBasePath(url.pathname, nextConfig?.basePath) - const didRemoveBasePath = url.toString() !== originalURL - let detectedLocale: string | undefined + let pathname = removeBasePath(url.pathname, nextConfig?.basePath) + + // If the i18n config is present, remove the locale from the URL and store it if (nextConfig?.i18n) { - const { pathname, detectedLocale: detected } = normalizeLocalePath( - url.pathname, - nextConfig?.i18n?.locales, - ) - if (!nextConfig?.skipMiddlewareUrlNormalize) { - url.pathname = pathname || '/' - } - detectedLocale = detected + ;({ detectedLocale } = normalizeLocalePath(pathname, nextConfig?.i18n?.locales)) } if (!nextConfig?.skipMiddlewareUrlNormalize) { // We want to run middleware for data requests and expose the URL of the // corresponding pages, so we have to normalize the URLs before running // the handler. - url.pathname = normalizeDataUrl(url.pathname) + pathname = normalizeDataUrl(pathname) // Normalizing the trailing slash based on the `trailingSlash` configuration // property from the Next.js config. - if (nextConfig?.trailingSlash && url.pathname !== '/' && !url.pathname.endsWith('/')) { - url.pathname = `${url.pathname}/` + if (nextConfig?.trailingSlash) { + pathname = addTrailingSlash(pathname) } } - if (didRemoveBasePath) { - url.pathname = addBasePath(url.pathname, nextConfig?.basePath) - } - - // keep the locale in the url for request.nextUrl object - if (detectedLocale) { - url.pathname = `/${detectedLocale}${url.pathname}` - } + url.pathname = addBasePath(pathname, nextConfig?.basePath) return { url: url.toString(), From 0cfbc6df836f00a28ddb1aa3869cc3ca4578677e Mon Sep 17 00:00:00 2001 From: Rob Stanford Date: Fri, 14 Jun 2024 13:37:40 +0100 Subject: [PATCH 2/5] test: add new middleware i18n fixture for skipping normalization --- .../middleware.js | 91 +++++++++++++++++++ .../next.config.js | 24 +++++ .../package.json | 18 ++++ .../pages/_app.js | 6 ++ .../pages/api/ok.js | 3 + .../pages/dynamic/[slug].js | 15 +++ .../pages/index.js | 35 +++++++ .../pages/new-home.js | 7 ++ tests/fixtures/middleware-i18n/middleware.js | 6 +- 9 files changed, 204 insertions(+), 1 deletion(-) create mode 100644 tests/fixtures/middleware-i18n-skip-normalize/middleware.js create mode 100644 tests/fixtures/middleware-i18n-skip-normalize/next.config.js create mode 100644 tests/fixtures/middleware-i18n-skip-normalize/package.json create mode 100644 tests/fixtures/middleware-i18n-skip-normalize/pages/_app.js create mode 100644 tests/fixtures/middleware-i18n-skip-normalize/pages/api/ok.js create mode 100644 tests/fixtures/middleware-i18n-skip-normalize/pages/dynamic/[slug].js create mode 100644 tests/fixtures/middleware-i18n-skip-normalize/pages/index.js create mode 100644 tests/fixtures/middleware-i18n-skip-normalize/pages/new-home.js diff --git a/tests/fixtures/middleware-i18n-skip-normalize/middleware.js b/tests/fixtures/middleware-i18n-skip-normalize/middleware.js new file mode 100644 index 0000000000..24517d72de --- /dev/null +++ b/tests/fixtures/middleware-i18n-skip-normalize/middleware.js @@ -0,0 +1,91 @@ +import { NextResponse } from 'next/server' + +export async function middleware(request) { + const url = request.nextUrl + + // this is needed for tests to get the BUILD_ID + if (url.pathname.startsWith('/_next/static/__BUILD_ID')) { + return NextResponse.next() + } + + if (url.pathname === '/old-home') { + if (url.searchParams.get('override') === 'external') { + return Response.redirect('https://example.vercel.sh') + } else { + url.pathname = '/new-home' + return Response.redirect(url) + } + } + + if (url.searchParams.get('foo') === 'bar') { + url.pathname = '/new-home' + url.searchParams.delete('foo') + return Response.redirect(url) + } + + // Chained redirects + if (url.pathname === '/redirect-me-alot') { + url.pathname = '/redirect-me-alot-2' + return Response.redirect(url) + } + + if (url.pathname === '/redirect-me-alot-2') { + url.pathname = '/redirect-me-alot-3' + return Response.redirect(url) + } + + if (url.pathname === '/redirect-me-alot-3') { + url.pathname = '/redirect-me-alot-4' + return Response.redirect(url) + } + + if (url.pathname === '/redirect-me-alot-4') { + url.pathname = '/redirect-me-alot-5' + return Response.redirect(url) + } + + if (url.pathname === '/redirect-me-alot-5') { + url.pathname = '/redirect-me-alot-6' + return Response.redirect(url) + } + + if (url.pathname === '/redirect-me-alot-6') { + url.pathname = '/redirect-me-alot-7' + return Response.redirect(url) + } + + if (url.pathname === '/redirect-me-alot-7') { + url.pathname = '/new-home' + return Response.redirect(url) + } + + // Infinite loop + if (url.pathname === '/infinite-loop') { + url.pathname = '/infinite-loop-1' + return Response.redirect(url) + } + + if (url.pathname === '/infinite-loop-1') { + url.pathname = '/infinite-loop' + return Response.redirect(url) + } + + if (url.pathname === '/to') { + url.pathname = url.searchParams.get('pathname') + url.searchParams.delete('pathname') + return Response.redirect(url) + } + + if (url.pathname === '/with-fragment') { + console.log(String(new URL('/new-home#fragment', url))) + return Response.redirect(new URL('/new-home#fragment', url)) + } + + if (url.pathname.includes('/json')) { + return NextResponse.json({ + requestUrlPathname: new URL(request.url).pathname, + nextUrlPathname: request.nextUrl.pathname, + nextUrlLocale: request.nextUrl.locale, + }) + } +} diff --git a/tests/fixtures/middleware-i18n-skip-normalize/next.config.js b/tests/fixtures/middleware-i18n-skip-normalize/next.config.js new file mode 100644 index 0000000000..eadf9cf8fb --- /dev/null +++ b/tests/fixtures/middleware-i18n-skip-normalize/next.config.js @@ -0,0 +1,24 @@ +module.exports = { + output: 'standalone', + eslint: { + ignoreDuringBuilds: true, + }, + i18n: { + locales: ['en', 'fr', 'nl', 'es'], + defaultLocale: 'en', + }, + skipMiddlewareUrlNormalize: true, + experimental: { + clientRouterFilter: true, + clientRouterFilterRedirects: true, + }, + redirects() { + return [ + { + source: '/to-new', + destination: '/dynamic/new', + permanent: false, + }, + ] + }, +} diff --git a/tests/fixtures/middleware-i18n-skip-normalize/package.json b/tests/fixtures/middleware-i18n-skip-normalize/package.json new file mode 100644 index 0000000000..5708c88b50 --- /dev/null +++ b/tests/fixtures/middleware-i18n-skip-normalize/package.json @@ -0,0 +1,18 @@ +{ + "name": "middleware-pages", + "version": "0.1.0", + "private": true, + "scripts": { + "postinstall": "next build", + "dev": "next dev", + "build": "next build" + }, + "dependencies": { + "next": "latest", + "react": "18.2.0", + "react-dom": "18.2.0" + }, + "devDependencies": { + "@types/react": "18.2.47" + } +} diff --git a/tests/fixtures/middleware-i18n-skip-normalize/pages/_app.js b/tests/fixtures/middleware-i18n-skip-normalize/pages/_app.js new file mode 100644 index 0000000000..4f7709a5bc --- /dev/null +++ b/tests/fixtures/middleware-i18n-skip-normalize/pages/_app.js @@ -0,0 +1,6 @@ +export default function App({ Component, pageProps }) { + if (!pageProps || typeof pageProps !== 'object') { + throw new Error(`Invariant: received invalid pageProps in _app, received ${pageProps}`) + } + return +} diff --git a/tests/fixtures/middleware-i18n-skip-normalize/pages/api/ok.js b/tests/fixtures/middleware-i18n-skip-normalize/pages/api/ok.js new file mode 100644 index 0000000000..fb91e8b611 --- /dev/null +++ b/tests/fixtures/middleware-i18n-skip-normalize/pages/api/ok.js @@ -0,0 +1,3 @@ +export default function handler(req, res) { + res.send('ok') +} diff --git a/tests/fixtures/middleware-i18n-skip-normalize/pages/dynamic/[slug].js b/tests/fixtures/middleware-i18n-skip-normalize/pages/dynamic/[slug].js new file mode 100644 index 0000000000..61131835fa --- /dev/null +++ b/tests/fixtures/middleware-i18n-skip-normalize/pages/dynamic/[slug].js @@ -0,0 +1,15 @@ +export default function Account({ slug }) { + return ( +

+ Welcome to a /dynamic/[slug]: {slug} +

+ ) +} + +export function getServerSideProps({ params }) { + return { + props: { + slug: params.slug, + }, + } +} diff --git a/tests/fixtures/middleware-i18n-skip-normalize/pages/index.js b/tests/fixtures/middleware-i18n-skip-normalize/pages/index.js new file mode 100644 index 0000000000..362aa85570 --- /dev/null +++ b/tests/fixtures/middleware-i18n-skip-normalize/pages/index.js @@ -0,0 +1,35 @@ +import Link from 'next/link' + +export default function Home() { + return ( +
+

Home Page

+ + Redirect me to a new version of a page + +
+ + Redirect me to an external site + +
+ Redirect me with URL params intact +
+ Redirect me to Google (with no body response) +
+ Redirect me to Google (with no stream response) +
+ Redirect me alot (chained requests) +
+ Redirect me alot (infinite loop) +
+ + Redirect me to api with locale + +
+ + Redirect me to a redirecting page of new version of page + +
+
+ ) +} diff --git a/tests/fixtures/middleware-i18n-skip-normalize/pages/new-home.js b/tests/fixtures/middleware-i18n-skip-normalize/pages/new-home.js new file mode 100644 index 0000000000..313011766e --- /dev/null +++ b/tests/fixtures/middleware-i18n-skip-normalize/pages/new-home.js @@ -0,0 +1,7 @@ +export default function Account() { + return ( +

+ Welcome to a new page +

+ ) +} diff --git a/tests/fixtures/middleware-i18n/middleware.js b/tests/fixtures/middleware-i18n/middleware.js index ef6fa57296..24517d72de 100644 --- a/tests/fixtures/middleware-i18n/middleware.js +++ b/tests/fixtures/middleware-i18n/middleware.js @@ -82,6 +82,10 @@ export async function middleware(request) { } if (url.pathname.includes('/json')) { - return NextResponse.json({ url: request.nextUrl.href, locale: request.nextUrl.locale }) + return NextResponse.json({ + requestUrlPathname: new URL(request.url).pathname, + nextUrlPathname: request.nextUrl.pathname, + nextUrlLocale: request.nextUrl.locale, + }) } } From 384d32565409ebae44fd29be0e96df8c92906045 Mon Sep 17 00:00:00 2001 From: Rob Stanford Date: Fri, 14 Jun 2024 13:38:36 +0100 Subject: [PATCH 3/5] test: more test coverage for middleware i18n and for skipping normalization --- tests/integration/edge-handler.test.ts | 84 ++++++++++++++++++++++++-- 1 file changed, 80 insertions(+), 4 deletions(-) diff --git a/tests/integration/edge-handler.test.ts b/tests/integration/edge-handler.test.ts index 91b6d9ff21..ade2ec3ea7 100644 --- a/tests/integration/edge-handler.test.ts +++ b/tests/integration/edge-handler.test.ts @@ -513,16 +513,92 @@ describe('page router', () => { res.end() }) ctx.cleanup?.push(() => origin.stop()) + const response = await invokeEdgeFunction(ctx, { functions: ['___netlify-edge-handler-middleware'], origin, - url: `/fr/json`, + url: `/json`, }) expect(response.status).toBe(200) + const body = await response.json() + + expect(body.requestUrlPathname).toBe('/json') + expect(body.nextUrlPathname).toBe('/json') + expect(body.nextUrlLocale).toBe('en') + + const responseEn = await invokeEdgeFunction(ctx, { + functions: ['___netlify-edge-handler-middleware'], + origin, + url: `/en/json`, + }) + expect(responseEn.status).toBe(200) + const bodyEn = await responseEn.json() + + expect(bodyEn.requestUrlPathname).toBe('/json') + expect(bodyEn.nextUrlPathname).toBe('/json') + expect(bodyEn.nextUrlLocale).toBe('en') + const responseFr = await invokeEdgeFunction(ctx, { + functions: ['___netlify-edge-handler-middleware'], + origin, + url: `/fr/json`, + }) + expect(responseFr.status).toBe(200) + const bodyFr = await responseFr.json() + + expect(bodyFr.requestUrlPathname).toBe('/fr/json') + expect(bodyFr.nextUrlPathname).toBe('/json') + expect(bodyFr.nextUrlLocale).toBe('fr') + }) + + test.only('should preserve locale in request.nextUrl with skipMiddlewareUrlNormalize', async (ctx) => { + await createFixture('middleware-i18n-skip-normalize', ctx) + await runPlugin(ctx) + const origin = await LocalServer.run(async (req, res) => { + res.write( + JSON.stringify({ + url: req.url, + headers: req.headers, + }), + ) + res.end() + }) + ctx.cleanup?.push(() => origin.stop()) + + const response = await invokeEdgeFunction(ctx, { + functions: ['___netlify-edge-handler-middleware'], + origin, + url: `/json`, + }) + expect(response.status).toBe(200) const body = await response.json() - const bodyUrl = new URL(body.url) - expect(bodyUrl.pathname).toBe('/fr/json') - expect(body.locale).toBe('fr') + + expect(body.requestUrlPathname).toBe('/json') + expect(body.nextUrlPathname).toBe('/json') + expect(body.nextUrlLocale).toBe('en') + + const responseEn = await invokeEdgeFunction(ctx, { + functions: ['___netlify-edge-handler-middleware'], + origin, + url: `/en/json`, + }) + expect(responseEn.status).toBe(200) + const bodyEn = await responseEn.json() + + expect(bodyEn.requestUrlPathname).toBe('/en/json') + expect(bodyEn.nextUrlPathname).toBe('/json') + expect(bodyEn.nextUrlLocale).toBe('en') + + const responseFr = await invokeEdgeFunction(ctx, { + functions: ['___netlify-edge-handler-middleware'], + origin, + url: `/fr/json`, + }) + expect(responseFr.status).toBe(200) + const bodyFr = await responseFr.json() + + expect(bodyFr.requestUrlPathname).toBe('/fr/json') + expect(bodyFr.nextUrlPathname).toBe('/json') + expect(bodyFr.nextUrlLocale).toBe('fr') }) }) From ff7f82e839647315b804d47b3754c1edd27f7163 Mon Sep 17 00:00:00 2001 From: Rob Stanford Date: Fri, 14 Jun 2024 13:47:03 +0100 Subject: [PATCH 4/5] chore: remove test.only --- tests/integration/edge-handler.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/edge-handler.test.ts b/tests/integration/edge-handler.test.ts index ade2ec3ea7..9aca477dfb 100644 --- a/tests/integration/edge-handler.test.ts +++ b/tests/integration/edge-handler.test.ts @@ -551,7 +551,7 @@ describe('page router', () => { expect(bodyFr.nextUrlLocale).toBe('fr') }) - test.only('should preserve locale in request.nextUrl with skipMiddlewareUrlNormalize', async (ctx) => { + test('should preserve locale in request.nextUrl with skipMiddlewareUrlNormalize', async (ctx) => { await createFixture('middleware-i18n-skip-normalize', ctx) await runPlugin(ctx) const origin = await LocalServer.run(async (req, res) => { From 13a8cce82389dbaf53147d8c24eea92c157b6613 Mon Sep 17 00:00:00 2001 From: Rob Stanford Date: Mon, 17 Jun 2024 12:38:45 +0100 Subject: [PATCH 5/5] chore: refactor detectedLocale assignment --- edge-runtime/lib/next-request.ts | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/edge-runtime/lib/next-request.ts b/edge-runtime/lib/next-request.ts index 5673109a08..e6a1bb95f8 100644 --- a/edge-runtime/lib/next-request.ts +++ b/edge-runtime/lib/next-request.ts @@ -46,14 +46,11 @@ const normalizeRequestURL = ( nextConfig?: RequestData['nextConfig'], ): { url: string; detectedLocale?: string } => { const url = new URL(originalURL) - let detectedLocale: string | undefined let pathname = removeBasePath(url.pathname, nextConfig?.basePath) - // If the i18n config is present, remove the locale from the URL and store it - if (nextConfig?.i18n) { - ;({ detectedLocale } = normalizeLocalePath(pathname, nextConfig?.i18n?.locales)) - } + // If it exists, remove the locale from the URL and store it + const { detectedLocale } = normalizeLocalePath(pathname, nextConfig?.i18n?.locales) if (!nextConfig?.skipMiddlewareUrlNormalize) { // We want to run middleware for data requests and expose the URL of the