From 93eb3ee65664a8a67fb80561a543a35fd9274df2 Mon Sep 17 00:00:00 2001 From: Dorseuil Nicolas Date: Tue, 17 Sep 2024 13:04:43 +0200 Subject: [PATCH 1/4] fix middleware matcher not working with i18n --- .../open-next/src/core/routing/middleware.ts | 30 ++++++++++--------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/packages/open-next/src/core/routing/middleware.ts b/packages/open-next/src/core/routing/middleware.ts index 5a04e2066..0567e3574 100644 --- a/packages/open-next/src/core/routing/middleware.ts +++ b/packages/open-next/src/core/routing/middleware.ts @@ -32,33 +32,35 @@ type MiddlewareOutputEvent = InternalEvent & { // and res.body prior to processing the next-server. // @returns undefined | res.end() -// NOTE: We need to normalize the locale path before passing it to the middleware -// See https://github.com/vercel/next.js/blob/39589ff35003ba73f92b7f7b349b3fdd3458819f/packages/next/src/shared/lib/i18n/normalize-locale-path.ts#L15 -function normalizeLocalePath(pathname: string) { +// NOTE: We need to be sure that the locale path is there for the matcher +function ensureLocalizedPath(pathname: string) { + // If there are no i18n, we don't need to do anything + if (!NextConfig.i18n) return pathname; // first item will be empty string from splitting at first char const pathnameParts = pathname.split("/"); const locales = NextConfig.i18n?.locales; - (locales || []).some((locale) => { - if ( + const hasLocaleAlready = (locales || []).some((locale) => { + return !!( pathnameParts[1] && pathnameParts[1].toLowerCase() === locale.toLowerCase() - ) { - pathnameParts.splice(1, 1); - pathname = pathnameParts.join("/") || "/"; - return true; - } - return false; + ); }); - return locales && !pathname.endsWith("/") ? `${pathname}/` : pathname; + if (hasLocaleAlready) { + return pathname; + } + + const defaultLocale = NextConfig.i18n.defaultLocale; + + return [`/${defaultLocale}`, ...pathnameParts.slice(1)].join("/"); } // if res.end() is return, the parent needs to return and not process next server export async function handleMiddleware( internalEvent: InternalEvent, ): Promise { const { rawPath, query } = internalEvent; - const normalizedPath = normalizeLocalePath(rawPath); + const normalizedPath = ensureLocalizedPath(rawPath); // We only need the normalizedPath to check if the middleware should run const hasMatch = middleMatch.some((r) => r.test(normalizedPath)); if (!hasMatch) return internalEvent; @@ -68,7 +70,7 @@ export async function handleMiddleware( const host = internalEvent.headers.host ? `https://${internalEvent.headers.host}` : "http://localhost:3000"; - const initialUrl = new URL(rawPath, host); + const initialUrl = new URL(normalizedPath, host); initialUrl.search = convertToQueryString(query); const url = initialUrl.toString(); // console.log("url", url, normalizedPath); From 0d518d71009c8db9a11c18652f4569727f2c2d14 Mon Sep 17 00:00:00 2001 From: Dorseuil Nicolas Date: Fri, 20 Sep 2024 16:14:36 +0200 Subject: [PATCH 2/4] fix headers with i18n remove unnecessary function --- .../open-next/src/core/routing/matcher.ts | 16 +++++++++-- .../open-next/src/core/routing/middleware.ts | 28 ++----------------- 2 files changed, 16 insertions(+), 28 deletions(-) diff --git a/packages/open-next/src/core/routing/matcher.ts b/packages/open-next/src/core/routing/matcher.ts index 245629aeb..249526edc 100644 --- a/packages/open-next/src/core/routing/matcher.ts +++ b/packages/open-next/src/core/routing/matcher.ts @@ -144,14 +144,24 @@ export function addNextConfigHeaders( const requestHeaders: Record = {}; - for (const { headers, has, missing, regex, source } of configHeaders) { + const localizedRawPath = localizePath(event); + + for (const { + headers, + has, + missing, + regex, + source, + locale, + } of configHeaders) { + const path = locale === false ? rawPath : localizedRawPath; if ( - new RegExp(regex).test(rawPath) && + new RegExp(regex).test(path) && checkHas(matcher, has) && checkHas(matcher, missing, true) ) { const fromSource = match(source); - const _match = fromSource(rawPath); + const _match = fromSource(path); headers.forEach((h) => { try { const key = convertMatch(_match, compile(h.key), h.key); diff --git a/packages/open-next/src/core/routing/middleware.ts b/packages/open-next/src/core/routing/middleware.ts index 0567e3574..ebdf1bf5a 100644 --- a/packages/open-next/src/core/routing/middleware.ts +++ b/packages/open-next/src/core/routing/middleware.ts @@ -4,6 +4,7 @@ import { MiddlewareManifest, NextConfig } from "config/index.js"; import { InternalEvent, InternalResult } from "types/open-next.js"; import { emptyReadableStream } from "utils/stream.js"; +import { localizePath } from "./i18n/index.js"; //NOTE: we should try to avoid importing stuff from next as much as possible // every release of next could break this // const { run } = require("next/dist/server/web/sandbox"); @@ -32,35 +33,12 @@ type MiddlewareOutputEvent = InternalEvent & { // and res.body prior to processing the next-server. // @returns undefined | res.end() -// NOTE: We need to be sure that the locale path is there for the matcher -function ensureLocalizedPath(pathname: string) { - // If there are no i18n, we don't need to do anything - if (!NextConfig.i18n) return pathname; - // first item will be empty string from splitting at first char - const pathnameParts = pathname.split("/"); - const locales = NextConfig.i18n?.locales; - - const hasLocaleAlready = (locales || []).some((locale) => { - return !!( - pathnameParts[1] && - pathnameParts[1].toLowerCase() === locale.toLowerCase() - ); - }); - - if (hasLocaleAlready) { - return pathname; - } - - const defaultLocale = NextConfig.i18n.defaultLocale; - - return [`/${defaultLocale}`, ...pathnameParts.slice(1)].join("/"); -} // if res.end() is return, the parent needs to return and not process next server export async function handleMiddleware( internalEvent: InternalEvent, ): Promise { - const { rawPath, query } = internalEvent; - const normalizedPath = ensureLocalizedPath(rawPath); + const { query } = internalEvent; + const normalizedPath = localizePath(internalEvent); // We only need the normalizedPath to check if the middleware should run const hasMatch = middleMatch.some((r) => r.test(normalizedPath)); if (!hasMatch) return internalEvent; From 0d19aebb8bfb88a2a0476992ce80cde6cf31e2a4 Mon Sep 17 00:00:00 2001 From: Dorseuil Nicolas Date: Fri, 20 Sep 2024 18:02:16 +0200 Subject: [PATCH 3/4] add e2e test --- examples/pages-router/next.config.js | 11 +++++++++++ examples/pages-router/src/middleware.ts | 13 +++++++++++++ .../tests-e2e/tests/pagesRouter/i18n.test.ts | 17 +++++++++++++++++ 3 files changed, 41 insertions(+) create mode 100644 examples/pages-router/src/middleware.ts create mode 100644 packages/tests-e2e/tests/pagesRouter/i18n.test.ts diff --git a/examples/pages-router/next.config.js b/examples/pages-router/next.config.js index 48fea847d..98e1bb953 100644 --- a/examples/pages-router/next.config.js +++ b/examples/pages-router/next.config.js @@ -12,6 +12,17 @@ const nextConfig = { eslint: { ignoreDuringBuilds: true, }, + headers: () => [ + { + source: "/", + headers: [ + { + key: "x-custom-header", + value: "my custom header value", + }, + ], + }, + ], rewrites: () => [ { source: "/rewrite", destination: "/", locale: false }, { diff --git a/examples/pages-router/src/middleware.ts b/examples/pages-router/src/middleware.ts new file mode 100644 index 000000000..77a0fc4fa --- /dev/null +++ b/examples/pages-router/src/middleware.ts @@ -0,0 +1,13 @@ +import { NextRequest, NextResponse } from "next/server"; + +export function middleware(request: NextRequest) { + return NextResponse.next({ + headers: { + "x-from-middleware": "true", + }, + }); +} + +export const config = { + matcher: ["/"], +}; diff --git a/packages/tests-e2e/tests/pagesRouter/i18n.test.ts b/packages/tests-e2e/tests/pagesRouter/i18n.test.ts new file mode 100644 index 000000000..4271f93d5 --- /dev/null +++ b/packages/tests-e2e/tests/pagesRouter/i18n.test.ts @@ -0,0 +1,17 @@ +import { expect, test } from "@playwright/test"; + +test("Next config headers with i18n", async ({ page }) => { + const responsePromise = page.waitForResponse((response) => { + return response.status() === 200; + }); + await page.goto("/"); + + const response = await responsePromise; + // Response header should be set + const headers = response.headers(); + // Headers from next.config.js should be set + expect(headers["x-custom-header"]).toEqual("my custom header value"); + + // Headers from middleware should be set + expect(headers["x-from-middleware"]).toEqual("true"); +}); From 661ef9801737a189aa86d39cc491c310e556587e Mon Sep 17 00:00:00 2001 From: conico974 Date: Sun, 22 Sep 2024 20:11:33 +0200 Subject: [PATCH 4/4] Create fifty-clocks-report.md --- .changeset/fifty-clocks-report.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/fifty-clocks-report.md diff --git a/.changeset/fifty-clocks-report.md b/.changeset/fifty-clocks-report.md new file mode 100644 index 000000000..8f200c814 --- /dev/null +++ b/.changeset/fifty-clocks-report.md @@ -0,0 +1,5 @@ +--- +"open-next": patch +--- + +fix middleware and headers matcher not working properly with i18n