diff --git a/edge-runtime/lib/next-request.ts b/edge-runtime/lib/next-request.ts index 45744a882c..e6a1bb95f8 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 @@ -41,43 +47,25 @@ const normalizeRequestURL = ( ): { url: string; detectedLocale?: string } => { const url = new URL(originalURL) - url.pathname = removeBasePath(url.pathname, nextConfig?.basePath) - const didRemoveBasePath = url.toString() !== originalURL + let pathname = removeBasePath(url.pathname, nextConfig?.basePath) - let detectedLocale: string | undefined - - if (nextConfig?.i18n) { - const { pathname, detectedLocale: detected } = normalizeLocalePath( - url.pathname, - nextConfig?.i18n?.locales, - ) - if (!nextConfig?.skipMiddlewareUrlNormalize) { - url.pathname = pathname || '/' - } - detectedLocale = detected - } + // 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 // 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(), 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, + }) } } diff --git a/tests/integration/edge-handler.test.ts b/tests/integration/edge-handler.test.ts index 91b6d9ff21..9aca477dfb 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('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') }) })