diff --git a/src/build/functions/edge.ts b/src/build/functions/edge.ts index e8def1a11..d551afda5 100644 --- a/src/build/functions/edge.ts +++ b/src/build/functions/edge.ts @@ -36,13 +36,23 @@ const augmentMatchers = ( matchers: NextDefinition['matchers'], ctx: PluginContext, ): NextDefinition['matchers'] => { - if (!ctx.buildConfig.i18n) { + const i18NConfig = ctx.buildConfig.i18n + if (!i18NConfig) { return matchers } return matchers.flatMap((matcher) => { if (matcher.originalSource && matcher.locale !== false) { return [ - matcher, + matcher.regexp + ? { + ...matcher, + // https://github.com/vercel/next.js/blob/5e236c9909a768dc93856fdfad53d4f4adc2db99/packages/next/src/build/analysis/get-page-static-info.ts#L332-L336 + // Next is producing pretty broad matcher for i18n locale. Presumably rest of their infrastructure protects this broad matcher + // from matching on non-locale paths. For us this becomes request entry point, so we need to narrow it down to just defined locales + // otherwise users might get unexpected matches on paths like `/api*` + regexp: matcher.regexp.replace(/\[\^\/\.]+/g, `(${i18NConfig.locales.join('|')})`), + } + : matcher, { ...matcher, regexp: pathToRegexp(matcher.originalSource).source, diff --git a/tests/e2e/edge-middleware.test.ts b/tests/e2e/edge-middleware.test.ts index 8b32fbaa1..daea8e5d2 100644 --- a/tests/e2e/edge-middleware.test.ts +++ b/tests/e2e/edge-middleware.test.ts @@ -69,3 +69,150 @@ test('json data rewrite works', async ({ middlewarePages }) => { expect(data.pageProps.message).toBeDefined() }) + +// those tests use `fetch` instead of `page.goto` intentionally to avoid potential client rendering +// hiding any potential edge/server issues +test.describe('Middleware with i18n and excluded paths', () => { + const DEFAULT_LOCALE = 'en' + + /** helper function to extract JSON data from page rendering data with `
{JSON.stringify(data)}` */ + function extractDataFromHtml(html: string): Record
(?[^<]+)<\/pre>/) + if (!match || !match.groups?.rawInput) { + console.error(' not found in html input', { + html, + }) + throw new Error('Failed to extract data from HTML') + } + + const { rawInput } = match.groups + const unescapedInput = rawInput.replaceAll('"', '"') + try { + return JSON.parse(unescapedInput) + } catch (originalError) { + console.error('Failed to parse JSON', { + originalError, + rawInput, + unescapedInput, + }) + } + throw new Error('Failed to extract data from HTML') + } + + // those tests hit paths ending with `/json` which has special handling in middleware + // to return JSON response from middleware itself + test.describe('Middleware response path', () => { + test('should match on non-localized not excluded page path', async ({ + middlewareI18nExcludedPaths, + }) => { + const response = await fetch(`${middlewareI18nExcludedPaths.url}/json`) + + expect(response.headers.get('x-test-used-middleware')).toBe('true') + expect(response.status).toBe(200) + + const { nextUrlPathname, nextUrlLocale } = await response.json() + + expect(nextUrlPathname).toBe('/json') + expect(nextUrlLocale).toBe(DEFAULT_LOCALE) + }) + + test('should match on localized not excluded page path', async ({ + middlewareI18nExcludedPaths, + }) => { + const response = await fetch(`${middlewareI18nExcludedPaths.url}/fr/json`) + + expect(response.headers.get('x-test-used-middleware')).toBe('true') + expect(response.status).toBe(200) + + const { nextUrlPathname, nextUrlLocale } = await response.json() + + expect(nextUrlPathname).toBe('/json') + expect(nextUrlLocale).toBe('fr') + }) + }) + + // those tests hit paths that don't end with `/json` while still satisfying middleware matcher + // so middleware should pass them through to origin + test.describe('Middleware passthrough', () => { + test('should match on non-localized not excluded page path', async ({ + middlewareI18nExcludedPaths, + }) => { + const response = await fetch(`${middlewareI18nExcludedPaths.url}/html`) + + expect(response.headers.get('x-test-used-middleware')).toBe('true') + expect(response.status).toBe(200) + expect(response.headers.get('content-type')).toMatch(/text\/html/) + + const html = await response.text() + const { locale, params } = extractDataFromHtml(html) + + expect(params).toMatchObject({ catchall: ['html'] }) + expect(locale).toBe(DEFAULT_LOCALE) + }) + + test('should match on localized not excluded page path', async ({ + middlewareI18nExcludedPaths, + }) => { + const response = await fetch(`${middlewareI18nExcludedPaths.url}/fr/html`) + + expect(response.headers.get('x-test-used-middleware')).toBe('true') + expect(response.status).toBe(200) + expect(response.headers.get('content-type')).toMatch(/text\/html/) + + const html = await response.text() + const { locale, params } = extractDataFromHtml(html) + + expect(params).toMatchObject({ catchall: ['html'] }) + expect(locale).toBe('fr') + }) + }) + + // those tests hit paths that don't satisfy middleware matcher, so should go directly to origin + // without going through middleware + test.describe('Middleware skipping (paths not satisfying middleware matcher)', () => { + test('should NOT match on non-localized excluded API path', async ({ + middlewareI18nExcludedPaths, + }) => { + const response = await fetch(`${middlewareI18nExcludedPaths.url}/api/html`) + + expect(response.headers.get('x-test-used-middleware')).not.toBe('true') + expect(response.status).toBe(200) + + const { params } = await response.json() + + expect(params).toMatchObject({ catchall: ['html'] }) + }) + + test('should NOT match on non-localized excluded page path', async ({ + middlewareI18nExcludedPaths, + }) => { + const response = await fetch(`${middlewareI18nExcludedPaths.url}/excluded`) + + expect(response.headers.get('x-test-used-middleware')).not.toBe('true') + expect(response.status).toBe(200) + expect(response.headers.get('content-type')).toMatch(/text\/html/) + + const html = await response.text() + const { locale, params } = extractDataFromHtml(html) + + expect(params).toMatchObject({ catchall: ['excluded'] }) + expect(locale).toBe(DEFAULT_LOCALE) + }) + + test('should NOT match on localized excluded page path', async ({ + middlewareI18nExcludedPaths, + }) => { + const response = await fetch(`${middlewareI18nExcludedPaths.url}/fr/excluded`) + + expect(response.headers.get('x-test-used-middleware')).not.toBe('true') + expect(response.status).toBe(200) + expect(response.headers.get('content-type')).toMatch(/text\/html/) + + const html = await response.text() + const { locale, params } = extractDataFromHtml(html) + + expect(params).toMatchObject({ catchall: ['excluded'] }) + expect(locale).toBe('fr') + }) + }) +}) diff --git a/tests/fixtures/middleware-i18n-excluded-paths/middleware.ts b/tests/fixtures/middleware-i18n-excluded-paths/middleware.ts new file mode 100644 index 000000000..712f3648b --- /dev/null +++ b/tests/fixtures/middleware-i18n-excluded-paths/middleware.ts @@ -0,0 +1,36 @@ +import { NextResponse } from 'next/server' +import type { NextRequest } from 'next/server' + +export async function middleware(request: NextRequest) { + const url = request.nextUrl + + // if path ends with /json we create response in middleware, otherwise we pass it through + // to next server to get page or api response from it + const response = url.pathname.includes('/json') + ? NextResponse.json({ + requestUrlPathname: new URL(request.url).pathname, + nextUrlPathname: request.nextUrl.pathname, + nextUrlLocale: request.nextUrl.locale, + }) + : NextResponse.next() + + response.headers.set('x-test-used-middleware', 'true') + + return response +} + +// matcher copied from example in https://nextjs.org/docs/pages/building-your-application/routing/middleware#matcher +// with `excluded` segment added to exclusion +export const config = { + matcher: [ + /* + * Match all request paths except for the ones starting with: + * - api (API routes) + * - excluded (for testing localized routes and not just API routes) + * - _next/static (static files) + * - _next/image (image optimization files) + * - favicon.ico, sitemap.xml, robots.txt (metadata files) + */ + '/((?!api|excluded|_next/static|_next/image|favicon.ico|sitemap.xml|robots.txt).*)', + ], +} diff --git a/tests/fixtures/middleware-i18n-excluded-paths/next-env.d.ts b/tests/fixtures/middleware-i18n-excluded-paths/next-env.d.ts new file mode 100644 index 000000000..52e831b43 --- /dev/null +++ b/tests/fixtures/middleware-i18n-excluded-paths/next-env.d.ts @@ -0,0 +1,5 @@ +///+/// + +// NOTE: This file should not be edited +// see https://nextjs.org/docs/pages/api-reference/config/typescript for more information. diff --git a/tests/fixtures/middleware-i18n-excluded-paths/next.config.js b/tests/fixtures/middleware-i18n-excluded-paths/next.config.js new file mode 100644 index 000000000..a96ce45ff --- /dev/null +++ b/tests/fixtures/middleware-i18n-excluded-paths/next.config.js @@ -0,0 +1,10 @@ +module.exports = { + output: 'standalone', + eslint: { + ignoreDuringBuilds: true, + }, + i18n: { + locales: ['en', 'fr'], + defaultLocale: 'en', + }, +} diff --git a/tests/fixtures/middleware-i18n-excluded-paths/package.json b/tests/fixtures/middleware-i18n-excluded-paths/package.json new file mode 100644 index 000000000..3246e924f --- /dev/null +++ b/tests/fixtures/middleware-i18n-excluded-paths/package.json @@ -0,0 +1,20 @@ +{ + "name": "middleware-i18n-excluded-paths", + "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/node": "^17.0.12", + "@types/react": "18.2.47", + "typescript": "^5.2.2" + } +} diff --git a/tests/fixtures/middleware-i18n-excluded-paths/pages/[[...catchall]].tsx b/tests/fixtures/middleware-i18n-excluded-paths/pages/[[...catchall]].tsx new file mode 100644 index 000000000..811d8a6f2 --- /dev/null +++ b/tests/fixtures/middleware-i18n-excluded-paths/pages/[[...catchall]].tsx @@ -0,0 +1,21 @@ +import type { GetStaticPaths, GetStaticProps } from 'next' + +export default function CatchAll({ params, locale }) { + return {JSON.stringify({ params, locale }, null, 2)}+} + +export const getStaticPaths: GetStaticPaths = () => { + return { + paths: [], + fallback: 'blocking', + } +} + +export const getStaticProps: GetStaticProps = ({ params, locale }) => { + return { + props: { + params, + locale, + }, + } +} diff --git a/tests/fixtures/middleware-i18n-excluded-paths/pages/api/[[...catchall]].ts b/tests/fixtures/middleware-i18n-excluded-paths/pages/api/[[...catchall]].ts new file mode 100644 index 000000000..415f631cf --- /dev/null +++ b/tests/fixtures/middleware-i18n-excluded-paths/pages/api/[[...catchall]].ts @@ -0,0 +1,11 @@ +import type { NextApiRequest, NextApiResponse } from 'next' + +type ResponseData = { + params: { + catchall?: string[] + } +} + +export default function handler(req: NextApiRequest, res: NextApiResponse) { + res.status(200).json({ params: req.query }) +} diff --git a/tests/fixtures/middleware-i18n-excluded-paths/tsconfig.json b/tests/fixtures/middleware-i18n-excluded-paths/tsconfig.json new file mode 100644 index 000000000..1499fe59f --- /dev/null +++ b/tests/fixtures/middleware-i18n-excluded-paths/tsconfig.json @@ -0,0 +1,19 @@ +{ + "compilerOptions": { + "target": "ES2017", + "lib": ["dom", "dom.iterable", "esnext"], + "allowJs": true, + "skipLibCheck": true, + "strict": false, + "noEmit": true, + "incremental": true, + "module": "esnext", + "esModuleInterop": true, + "moduleResolution": "node", + "resolveJsonModule": true, + "isolatedModules": true, + "jsx": "preserve" + }, + "include": ["next-env.d.ts", "**/*.ts", "**/*.tsx"], + "exclude": ["node_modules"] +} diff --git a/tests/prepare.mjs b/tests/prepare.mjs index 3316fd238..072d08e6b 100644 --- a/tests/prepare.mjs +++ b/tests/prepare.mjs @@ -23,6 +23,7 @@ const e2eOnlyFixtures = new Set([ 'after', 'cli-before-regional-blobs-support', 'dist-dir', + 'middleware-i18n-excluded-paths', // There is also a bug on Windows on Node.js 18.20.6, that cause build failures on this fixture // see https://github.com/opennextjs/opennextjs-netlify/actions/runs/13268839161/job/37043172448?pr=2749#step:12:78 'middleware-og', diff --git a/tests/utils/create-e2e-fixture.ts b/tests/utils/create-e2e-fixture.ts index cc9c58849..6da96f044 100644 --- a/tests/utils/create-e2e-fixture.ts +++ b/tests/utils/create-e2e-fixture.ts @@ -333,6 +333,7 @@ export const fixtureFactories = { pnpm: () => createE2EFixture('pnpm', { packageManger: 'pnpm' }), bun: () => createE2EFixture('simple', { packageManger: 'bun' }), middleware: () => createE2EFixture('middleware'), + middlewareI18nExcludedPaths: () => createE2EFixture('middleware-i18n-excluded-paths'), middlewareOg: () => createE2EFixture('middleware-og'), middlewarePages: () => createE2EFixture('middleware-pages'), pageRouter: () => createE2EFixture('page-router'),