Skip to content

Commit

Permalink
fix matcher not working with i18n (#518)
Browse files Browse the repository at this point in the history
* fix middleware matcher not working with i18n

* fix headers with i18n
remove unnecessary function

* add e2e test

* Create fifty-clocks-report.md
  • Loading branch information
conico974 authored Sep 22, 2024
1 parent 44adea2 commit 4ec9265
Show file tree
Hide file tree
Showing 6 changed files with 63 additions and 27 deletions.
5 changes: 5 additions & 0 deletions .changeset/fifty-clocks-report.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"open-next": patch
---

fix middleware and headers matcher not working properly with i18n
11 changes: 11 additions & 0 deletions examples/pages-router/next.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 },
{
Expand Down
13 changes: 13 additions & 0 deletions examples/pages-router/src/middleware.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { NextRequest, NextResponse } from "next/server";

export function middleware(request: NextRequest) {

Check warning on line 3 in examples/pages-router/src/middleware.ts

View workflow job for this annotation

GitHub Actions / Release

'request' is defined but never used. Allowed unused args must match /^_/u
return NextResponse.next({
headers: {
"x-from-middleware": "true",
},
});
}

export const config = {
matcher: ["/"],
};
16 changes: 13 additions & 3 deletions packages/open-next/src/core/routing/matcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,14 +144,24 @@ export function addNextConfigHeaders(

const requestHeaders: Record<string, string> = {};

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);
Expand Down
28 changes: 4 additions & 24 deletions packages/open-next/src/core/routing/middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -32,33 +33,12 @@ 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) {
// first item will be empty string from splitting at first char
const pathnameParts = pathname.split("/");
const locales = NextConfig.i18n?.locales;

(locales || []).some((locale) => {
if (
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 res.end() is return, the parent needs to return and not process next server
export async function handleMiddleware(
internalEvent: InternalEvent,
): Promise<MiddlewareOutputEvent | InternalResult> {
const { rawPath, query } = internalEvent;
const normalizedPath = normalizeLocalePath(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;
Expand All @@ -68,7 +48,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);
Expand Down
17 changes: 17 additions & 0 deletions packages/tests-e2e/tests/pagesRouter/i18n.test.ts
Original file line number Diff line number Diff line change
@@ -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");
});

0 comments on commit 4ec9265

Please sign in to comment.