From 1ff516c1e36e1f8266c983d1e7b3f2afd826a0e4 Mon Sep 17 00:00:00 2001 From: Jan Stevens Date: Fri, 21 Jun 2024 13:48:41 +0200 Subject: [PATCH 1/8] fix: normalize the rawPath without locale before attempting to assert the 404 page --- packages/open-next/src/adapters/config/util.ts | 1 + packages/open-next/src/core/routingHandler.ts | 14 ++++++++++---- packages/open-next/src/types/next-types.ts | 3 +++ 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/packages/open-next/src/adapters/config/util.ts b/packages/open-next/src/adapters/config/util.ts index aa11ee3b5..67192b7fa 100644 --- a/packages/open-next/src/adapters/config/util.ts +++ b/packages/open-next/src/adapters/config/util.ts @@ -59,6 +59,7 @@ export function loadRoutesManifest(nextDir: string) { dynamic: routesManifest.dynamicRoutes ?? [], data: dataRoutes, }, + locales: routesManifest.i18n?.locales ?? [], }; } diff --git a/packages/open-next/src/core/routingHandler.ts b/packages/open-next/src/core/routingHandler.ts index 384bb2cdb..5a35496df 100644 --- a/packages/open-next/src/core/routingHandler.ts +++ b/packages/open-next/src/core/routingHandler.ts @@ -30,6 +30,8 @@ const dynamicRegexp = RoutesManifest.routes.dynamic.map( (route) => new RegExp(route.regex), ); +const localeRegexp = RoutesManifest.locales.length ? new RegExp(RoutesManifest.locales.map((locale) => `(^/${locale}?/)`).join("|")) : null; + export default async function routingHandler( event: InternalEvent, ): Promise { @@ -67,10 +69,11 @@ export default async function routingHandler( internalEvent = beforeRewrites.internalEvent; isExternalRewrite = beforeRewrites.isExternalRewrite; } + let normalizedRawPath = internalEvent.rawPath.replace(localeRegexp, "/"); const isStaticRoute = !isExternalRewrite && staticRegexp.some((route) => - route.test((internalEvent as InternalEvent).rawPath), + route.test(normalizedRawPath), ); if (!isStaticRoute && !isExternalRewrite) { @@ -85,11 +88,12 @@ export default async function routingHandler( // We want to run this just before the dynamic route check internalEvent = handleFallbackFalse(internalEvent, PrerenderManifest); + normalizedRawPath = internalEvent.rawPath.replace(localeRegexp, "/"); const isDynamicRoute = !isExternalRewrite && dynamicRegexp.some((route) => - route.test((internalEvent as InternalEvent).rawPath), + route.test(normalizedRawPath), ); if (!isDynamicRoute && !isStaticRoute && !isExternalRewrite) { // Fallback rewrite to be applied @@ -110,6 +114,8 @@ export default async function routingHandler( const isRouteFoundBeforeAllRewrites = isStaticRoute || isDynamicRoute || isExternalRewrite; + + normalizedRawPath = internalEvent.rawPath.replace(localeRegexp, "/"); // If we still haven't found a route, we show the 404 page // We need to ensure that rewrites are applied before showing the 404 page @@ -118,10 +124,10 @@ export default async function routingHandler( !isApiRoute && // We need to check again once all rewrites have been applied !staticRegexp.some((route) => - route.test((internalEvent as InternalEvent).rawPath), + route.test(normalizedRawPath), ) && !dynamicRegexp.some((route) => - route.test((internalEvent as InternalEvent).rawPath), + route.test(normalizedRawPath), ) ) { internalEvent = { diff --git a/packages/open-next/src/types/next-types.ts b/packages/open-next/src/types/next-types.ts index f1f3f8077..d7f6570df 100644 --- a/packages/open-next/src/types/next-types.ts +++ b/packages/open-next/src/types/next-types.ts @@ -112,6 +112,9 @@ export interface RoutesManifest { | RewriteDefinition[]; redirects: RedirectDefinition[]; headers?: Header[]; + i18n?: { + locales: string[]; + }; } export interface MiddlewareInfo { From cb0249bf53e360d8d60d67eb68d78a7ba827ef14 Mon Sep 17 00:00:00 2001 From: Jan Stevens Date: Fri, 21 Jun 2024 13:55:58 +0200 Subject: [PATCH 2/8] fix: eslint issues --- packages/open-next/src/core/routingHandler.ts | 24 ++++++++----------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/packages/open-next/src/core/routingHandler.ts b/packages/open-next/src/core/routingHandler.ts index 5a35496df..46683f1c1 100644 --- a/packages/open-next/src/core/routingHandler.ts +++ b/packages/open-next/src/core/routingHandler.ts @@ -30,7 +30,11 @@ const dynamicRegexp = RoutesManifest.routes.dynamic.map( (route) => new RegExp(route.regex), ); -const localeRegexp = RoutesManifest.locales.length ? new RegExp(RoutesManifest.locales.map((locale) => `(^/${locale}?/)`).join("|")) : null; +const localeRegexp = RoutesManifest.locales.length + ? new RegExp( + RoutesManifest.locales.map((locale) => `(^/${locale}?/)`).join("|"), + ) + : null; export default async function routingHandler( event: InternalEvent, @@ -72,9 +76,7 @@ export default async function routingHandler( let normalizedRawPath = internalEvent.rawPath.replace(localeRegexp, "/"); const isStaticRoute = !isExternalRewrite && - staticRegexp.some((route) => - route.test(normalizedRawPath), - ); + staticRegexp.some((route) => route.test(normalizedRawPath)); if (!isStaticRoute && !isExternalRewrite) { // Second rewrite to be applied @@ -92,9 +94,7 @@ export default async function routingHandler( const isDynamicRoute = !isExternalRewrite && - dynamicRegexp.some((route) => - route.test(normalizedRawPath), - ); + dynamicRegexp.some((route) => route.test(normalizedRawPath)); if (!isDynamicRoute && !isStaticRoute && !isExternalRewrite) { // Fallback rewrite to be applied const fallbackRewrites = handleRewrites( @@ -114,7 +114,7 @@ export default async function routingHandler( const isRouteFoundBeforeAllRewrites = isStaticRoute || isDynamicRoute || isExternalRewrite; - + normalizedRawPath = internalEvent.rawPath.replace(localeRegexp, "/"); // If we still haven't found a route, we show the 404 page @@ -123,12 +123,8 @@ export default async function routingHandler( !isRouteFoundBeforeAllRewrites && !isApiRoute && // We need to check again once all rewrites have been applied - !staticRegexp.some((route) => - route.test(normalizedRawPath), - ) && - !dynamicRegexp.some((route) => - route.test(normalizedRawPath), - ) + !staticRegexp.some((route) => route.test(normalizedRawPath)) && + !dynamicRegexp.some((route) => route.test(normalizedRawPath)) ) { internalEvent = { ...internalEvent, From 5caa19493e5775107f6b332a6d4b86c17c68e6fc Mon Sep 17 00:00:00 2001 From: Jan Stevens Date: Fri, 21 Jun 2024 15:19:12 +0200 Subject: [PATCH 3/8] fix: refactor to enhance existing regex --- packages/open-next/src/core/routingHandler.ts | 36 ++++++++++--------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/packages/open-next/src/core/routingHandler.ts b/packages/open-next/src/core/routingHandler.ts index 46683f1c1..492b8b33b 100644 --- a/packages/open-next/src/core/routingHandler.ts +++ b/packages/open-next/src/core/routingHandler.ts @@ -22,20 +22,19 @@ export interface MiddlewareOutputEvent { origin: Origin | false; } +// Add the locale prefix to the regex so we correctly match the rawPath +const optionalLocalePrefixRegex = !!RoutesManifest.locales.length + ? `^/(?:${RoutesManifest.locales.map((locale) => `${locale}/`).join("|")})?` + : null; + const staticRegexp = RoutesManifest.routes.static.map( - (route) => new RegExp(route.regex), + (route) => new RegExp(route.regex.replace("^/", optionalLocalePrefixRegex)), ); const dynamicRegexp = RoutesManifest.routes.dynamic.map( - (route) => new RegExp(route.regex), + (route) => new RegExp(route.regex.replace("^/", optionalLocalePrefixRegex)), ); -const localeRegexp = RoutesManifest.locales.length - ? new RegExp( - RoutesManifest.locales.map((locale) => `(^/${locale}?/)`).join("|"), - ) - : null; - export default async function routingHandler( event: InternalEvent, ): Promise { @@ -73,10 +72,12 @@ export default async function routingHandler( internalEvent = beforeRewrites.internalEvent; isExternalRewrite = beforeRewrites.isExternalRewrite; } - let normalizedRawPath = internalEvent.rawPath.replace(localeRegexp, "/"); + const isStaticRoute = !isExternalRewrite && - staticRegexp.some((route) => route.test(normalizedRawPath)); + staticRegexp.some((route) => + route.test((internalEvent as InternalEvent).rawPath), + ); if (!isStaticRoute && !isExternalRewrite) { // Second rewrite to be applied @@ -90,11 +91,12 @@ export default async function routingHandler( // We want to run this just before the dynamic route check internalEvent = handleFallbackFalse(internalEvent, PrerenderManifest); - normalizedRawPath = internalEvent.rawPath.replace(localeRegexp, "/"); const isDynamicRoute = !isExternalRewrite && - dynamicRegexp.some((route) => route.test(normalizedRawPath)); + dynamicRegexp.some((route) => + route.test((internalEvent as InternalEvent).rawPath), + ); if (!isDynamicRoute && !isStaticRoute && !isExternalRewrite) { // Fallback rewrite to be applied const fallbackRewrites = handleRewrites( @@ -115,16 +117,18 @@ export default async function routingHandler( const isRouteFoundBeforeAllRewrites = isStaticRoute || isDynamicRoute || isExternalRewrite; - normalizedRawPath = internalEvent.rawPath.replace(localeRegexp, "/"); - // If we still haven't found a route, we show the 404 page // We need to ensure that rewrites are applied before showing the 404 page if ( !isRouteFoundBeforeAllRewrites && !isApiRoute && // We need to check again once all rewrites have been applied - !staticRegexp.some((route) => route.test(normalizedRawPath)) && - !dynamicRegexp.some((route) => route.test(normalizedRawPath)) + !staticRegexp.some((route) => + route.test((internalEvent as InternalEvent).rawPath), + ) && + !dynamicRegexp.some((route) => + route.test((internalEvent as InternalEvent).rawPath), + ) ) { internalEvent = { ...internalEvent, From b75ec2149168159f604fdb20e6b79a9e1e99487f Mon Sep 17 00:00:00 2001 From: Jan Stevens Date: Fri, 21 Jun 2024 16:19:03 +0200 Subject: [PATCH 4/8] chore: add changelog --- .changeset/tidy-mice-train.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/tidy-mice-train.md diff --git a/.changeset/tidy-mice-train.md b/.changeset/tidy-mice-train.md new file mode 100644 index 000000000..e7d0b8dc0 --- /dev/null +++ b/.changeset/tidy-mice-train.md @@ -0,0 +1,5 @@ +--- +"open-next": patch +--- + +fix 404 handeling with i18n routes From d76bea68220ba8df2f340c0c5c20d6bd500a56e1 Mon Sep 17 00:00:00 2001 From: Jan Stevens Date: Fri, 21 Jun 2024 16:26:59 +0200 Subject: [PATCH 5/8] fix: ensure root locale urls are captured --- packages/open-next/src/core/routingHandler.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/open-next/src/core/routingHandler.ts b/packages/open-next/src/core/routingHandler.ts index 492b8b33b..582ef2229 100644 --- a/packages/open-next/src/core/routingHandler.ts +++ b/packages/open-next/src/core/routingHandler.ts @@ -24,7 +24,7 @@ export interface MiddlewareOutputEvent { // Add the locale prefix to the regex so we correctly match the rawPath const optionalLocalePrefixRegex = !!RoutesManifest.locales.length - ? `^/(?:${RoutesManifest.locales.map((locale) => `${locale}/`).join("|")})?` + ? `^/(?:${RoutesManifest.locales.map((locale) => `${locale}/?`).join("|")})?` : null; const staticRegexp = RoutesManifest.routes.static.map( From b0973c28df6e60443be55e50f9c23e20a6031ba1 Mon Sep 17 00:00:00 2001 From: Jan Stevens Date: Mon, 24 Jun 2024 08:07:48 +0200 Subject: [PATCH 6/8] chore: add i18n configuration --- examples/pages-router/next.config.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/examples/pages-router/next.config.js b/examples/pages-router/next.config.js index a0a5044b8..5a647fcbc 100644 --- a/examples/pages-router/next.config.js +++ b/examples/pages-router/next.config.js @@ -1,6 +1,10 @@ /** @type {import('next').NextConfig} */ const nextConfig = { transpilePackages: ["@example/shared"], + i18n: { + locales: ['en', 'nl'], + defaultLocale: 'en', + }, cleanDistDir: true, reactStrictMode: true, output: "standalone", From b9118422733247219c03c35f7dc79d014c89cf8b Mon Sep 17 00:00:00 2001 From: Jan Stevens Date: Mon, 24 Jun 2024 08:10:09 +0200 Subject: [PATCH 7/8] fix: use double quotes --- examples/pages-router/next.config.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/examples/pages-router/next.config.js b/examples/pages-router/next.config.js index 5a647fcbc..2cb58d635 100644 --- a/examples/pages-router/next.config.js +++ b/examples/pages-router/next.config.js @@ -2,8 +2,8 @@ const nextConfig = { transpilePackages: ["@example/shared"], i18n: { - locales: ['en', 'nl'], - defaultLocale: 'en', + locales: ["en", "nl"], + defaultLocale: "en", }, cleanDistDir: true, reactStrictMode: true, From 8bb1728d0b6ca7cda456e4a080a6e35cb66d844d Mon Sep 17 00:00:00 2001 From: Dorseuil Nicolas Date: Fri, 28 Jun 2024 09:12:06 +0200 Subject: [PATCH 8/8] fix e2e --- examples/pages-router/next.config.js | 3 ++- packages/open-next/src/core/routingHandler.ts | 2 +- packages/tests-e2e/tests/pagesRouter/data.test.ts | 8 ++++---- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/examples/pages-router/next.config.js b/examples/pages-router/next.config.js index 2cb58d635..1eac7325e 100644 --- a/examples/pages-router/next.config.js +++ b/examples/pages-router/next.config.js @@ -13,10 +13,11 @@ const nextConfig = { ignoreDuringBuilds: true, }, rewrites: () => [ - { source: "/rewrite", destination: "/" }, + { source: "/rewrite", destination: "/", locale: false }, { source: "/rewriteUsingQuery", destination: "/:destination/", + locale: false, has: [ { type: "query", diff --git a/packages/open-next/src/core/routingHandler.ts b/packages/open-next/src/core/routingHandler.ts index 582ef2229..da35d9f84 100644 --- a/packages/open-next/src/core/routingHandler.ts +++ b/packages/open-next/src/core/routingHandler.ts @@ -25,7 +25,7 @@ export interface MiddlewareOutputEvent { // Add the locale prefix to the regex so we correctly match the rawPath const optionalLocalePrefixRegex = !!RoutesManifest.locales.length ? `^/(?:${RoutesManifest.locales.map((locale) => `${locale}/?`).join("|")})?` - : null; + : "^/"; const staticRegexp = RoutesManifest.routes.static.map( (route) => new RegExp(route.regex.replace("^/", optionalLocalePrefixRegex)), diff --git a/packages/tests-e2e/tests/pagesRouter/data.test.ts b/packages/tests-e2e/tests/pagesRouter/data.test.ts index eb5bf322b..919fcc6c1 100644 --- a/packages/tests-e2e/tests/pagesRouter/data.test.ts +++ b/packages/tests-e2e/tests/pagesRouter/data.test.ts @@ -3,18 +3,18 @@ import { expect, test } from "@playwright/test"; test("fix _next/data", async ({ page }) => { await page.goto("/"); - const isrJson = page.waitForResponse("/_next/data/*/isr.json"); + const isrJson = page.waitForResponse("/_next/data/*/en/isr.json"); await page.locator('[href="/isr/"]').click(); const response = await isrJson; expect(response.ok()).toBe(true); - expect(response.request().url()).toMatch(/\/_next\/data\/.*\/isr\.json$/); + expect(response.request().url()).toMatch(/\/_next\/data\/.*\/en\/isr\.json$/); await page.waitForURL("/isr/"); - const homeJson = page.waitForResponse("/_next/data/*/index.json"); + const homeJson = page.waitForResponse("/_next/data/*/en.json"); await page.locator('[href="/"]').click(); const response2 = await homeJson; expect(response2.ok()).toBe(true); - expect(response2.request().url()).toMatch(/\/_next\/data\/.*\/index\.json$/); + expect(response2.request().url()).toMatch(/\/_next\/data\/.*\/en\.json$/); await page.waitForURL("/"); const body = await response2.json(); expect(body).toEqual({ pageProps: { hello: "world" }, __N_SSG: true });