opennextjs / opennextjs-aws

Open-source Next.js adapter for AWS
https://open-next.js.org
MIT License
3.9k stars 119 forks source link

Matcher-based middleware is not invoked when i18n is configured #516

Closed sladkoff closed 6 days ago

sladkoff commented 1 week ago

Versions

Setup

I have this middleware.ts which works in the standalone build of nextjs:

import { MiddlewareConfig, NextRequest, NextResponse } from 'next/server'

export async function middleware(req: NextRequest) {
  console.log('MIDDLEWARE')

  let url = req.nextUrl.clone()

  const baseUrl = "https://example.com/".split('://')

  url.protocol = baseUrl[0]
  url.hostname = baseUrl[1]
  url.port = baseUrl[0] === 'https' ? '443' : '80'
  url.pathname = url.pathname.replace(/^\/api\/test/, '')

  console.log(`Proxying ${req.url} to ${url.toString()}`)

  return NextResponse.rewrite(url)
}

export const config: MiddlewareConfig = {
  matcher: '/api/test/:path*',
}

I can go to localhost:3001/api/test/ in the browser and reach https://example.com/.

Problem

I have created this local open-next.local.config.ts:

// open-next.local.config.ts -
// A good practice would be to use a different name so that it doesn't conflict
// with your existing open-next.config.ts i.e. open-next.local.config.ts
import type { OpenNextConfig } from 'open-next/types/open-next.d.ts'

const config = {
  default: {
    override:{
      // We use a custom wrapper so that we can use static assets and image optimization locally
      wrapper: () => import('./dev/wrapper').then(m => m.default),
      // ISR and SSG won't work properly locally without this - Remove if you only need SSR
      // incrementalCache: () => import('./dev/incrementalCache').then(m => m.default),
      // ISR requires a queue to work properly - Remove if you only need SSR or SSG
      // queue: () => import('./dev/queue').then(m => m.default),
      converter: 'node',
    }
  },
  // middleware: {
  //   external: true,
  // },
  // You don't need this part if you don't use image optimization or don't need it in your test
  imageOptimization: {
    // Image optimization only work on linux, and you have to use the correct architecture for your system
    arch: 'x64',
    override: {
      wrapper: 'node',
      converter: 'node',
    }
    // If you need to test with local assets, you'll have to override the imageLoader as well
  },

  dangerous: {
    // We disable the cache tags as it will usually not be needed locally for testing
    // It's only used for next/cache revalidateTag and revalidatePath
    // If you need it you'll have to override the tagCache as well
    disableTagCache: true,

    // You can uncomment this line if you only need to test SSR
    //disableIncrementalCache: true,
  },
  // You can override the build command so that you don't have to rebuild the app every time
  // You need to have run the default build command at least once
  // buildCommand: 'echo "no build command"',
  edgeExternals: ['./dev/wrapper'],
} satisfies OpenNextConfig

export default config

I build and start the local open-next server and go to http://localhost:3001/api/test/ -> I receive the 404 page of my nextjs project and the server logs never show the line "MIDDLEWARE".

The same happens when the open-next app is deployed to AWS.

sladkoff commented 1 week ago

I was able to produce my expected result (JSON response from middleware) by manually changing my local open-next code:

  1. middleware.ts

Remove lines: https://github.com/sst/open-next/blob/main/packages/open-next/src/core/routing/middleware.ts#L63-L66

  1. edgeFunctionHandler.ts

Replace lines https://github.com/sst/open-next/blob/main/packages/open-next/src/core/edgeFunctionHandler.ts#L60-L62 with dummy implementation const correspondingRoutes = routes[0]

With these hard coded changes the middleware is executed properly which leads me to believe that something is wrong with the way that the middleware routes are resolved from the regex expressions.

The way I understand, a middleware-manifest.json is created by the next build:

{
  "version": 3,
  "middleware": {
    "/": {
      "files": [
        "server/edge-runtime-webpack.js",
        "server/src/middleware.js"
      ],
      "name": "src/middleware",
      "page": "/",
      "matchers": [
        {
          "regexp": "^(?:\\/(_next\\/data\\/[^/]{1,}))?(?:\\/((?!_next\\/)[^/.]{1,}))\\/api\\/test(?:\\/((?:[^\\/#\\?]+?)(?:\\/(?:[^\\/#\\?]+?))*))?(.json)?[\\/#\\?]?$",
          "originalSource": "/api/test/:path*"
        }
      ],
      "wasm": [],
      "assets": [],
      "env": {
        "__NEXT_BUILD_ID": "aSDe_tjGG58HlC_u6NljI",
        "__NEXT_PREVIEW_MODE_ID": "2cd693bc647e8af1c8d317da3ac52c78",
        "...": "..."
      }
    }
  },
  "functions": {},
  "sortedMiddleware": [
    "/"
  ]
}

I recognize originalSource=/api/test/:path* from my middleware but I have a hard time figuring out the cryptic regex that was created for it. I entered it into https://regex101.com/ and was not able to make the regex match to the normalizedPath that is being passed to https://github.com/sst/open-next/blob/main/packages/open-next/src/core/routing/middleware.ts#L63 (/api/tests/comments/).

Anyway, this manifest is picked up by open-next when resolving requested paths and choosing when to execute a middleware. This doesn't seem to work in my case.

khuezy commented 1 week ago

That isn't valid regex, it's a route syntax via https://github.com/pillarjs/path-to-regexp

sladkoff commented 1 week ago

That isn't valid regex, it's a route syntax via https://github.com/pillarjs/path-to-regexp

The middleware supports these expressions according to the Next docs on middleware matchers.

Do you mean that open-next doesn't support these and I have to use regex in the matcher configuration instead?

conico974 commented 1 week ago

What's in your next config ?

sladkoff commented 1 week ago

What's in your next config ?

import i18n from './next-i18next.config.js'

import mdx from '@next/mdx'

const withMDX = mdx({
  extension: /\.mdx?$/,
  options: {
    remarkPlugins: [],
    rehypePlugins: [],
    // If you use `MDXProvider`, uncomment the following line.
    providerImportSource: '@mdx-js/react',
  },
})

/** @type {import('next').NextConfig} */
const nextConfig = withMDX({
  reactStrictMode: true,
  compiler: {
    emotion: true,
  },
  pageExtensions: ['ts', 'tsx', 'js', 'jsx', 'md', 'mdx'],
  i18n: i18n.i18n,
  images: {
    domains: ['redacted.fra1.cdn.digitaloceanspaces.com'],
    // These are the modified default device sizes that match with redacted-images pre-optimization
    // See also https://nextjs.org/docs/app/api-reference/components/image#devicesizes
    // deviceSizes: [640, 750, 828, 1080, 1200, 1920, 2048, 3840],
    // imageSizes: [16, 32, 48, 64, 96, 128, 256, 384],
    deviceSizes: [640, 828, 1200, 1920, 3840],
    imageSizes: [128, 384],
  },
  webpack: (config) => {
    // This is to allow us to load fonts in PDF
    config.module.rules.push({
      test: /\.(woff(2)?|ttf|eot|svg)(\?v=\d+\.\d+\.\d+)?$/,
      use: [
        {
          loader: 'file-loader',
          options: {
            name: '[name].[ext]',
            outputPath: 'public/fonts/',
          },
        },
      ],
    })
    return config
  },
  async redirects() {
    return [
      {
        source: '/app',
        destination: '/app/home',
        permanent: true,
      },
      {
        source: '/portal',
        destination: '/portal/blog',
        permanent: true,
      },
      {
        source: '/:lang(.+)/signup',
        destination: '/api/auth/login?signup=true&language=:lang',
        locale: false,
        permanent: false,
      },
      {
        source: '/invite',
        destination: '/app/get-started/accept-invite',
        permanent: true,
      }
    ]
  },
  async headers() {
    return [
      {
        source: '/logout',
        headers: [
          {
            key: 'Set-Cookie',
            value: 'appSession=logout; Max-Age=-1',
          },
        ],
      },
      {
        source: '/.well-known/apple-app-site-association',
        headers: [{ key: 'content-type', value: 'application/json' }],
      },
    ]
  },
})

export default nextConfig
conico974 commented 1 week ago

I see, it's likely a bug because of i18n. Can you test if it works without it?

khuezy commented 1 week ago

That isn't valid regex, it's a route syntax via https://github.com/pillarjs/path-to-regexp

The middleware supports these expressions according to the Next docs on middleware matchers.

Do you mean that open-next doesn't support these and I have to use regex in the matcher configuration instead?

It supports it, I was just mentioning why it wasn't valid syntax. Nextjs does the path to regex under the hood via that library.

sladkoff commented 1 week ago

I see, it's likely a bug because of i18n. Can you test if it works without it?

I removed all i18n related from this app which resulted in a different regexp in the middleware-manifest.json:

- "^(?:\\/(_next\\/data\\/[^/]{1,}))?(?:\\/((?!_next\\/)[^/.]{1,}))\\/api\\/test(?:\\/((?:[^\\/#\\?]+?)(?:\\/(?:[^\\/#\\?]+?))*))?(.json)?[\\/#\\?]?$"
+ "^(?:\\/(_next\\/data\\/[^/]{1,}))?\\/api\\/test(?:\\/((?:[^\\/#\\?]+?)(?:\\/(?:[^\\/#\\?]+?))*))?(.json)?[\\/#\\?]?$"

My middleware works with this.

Is there a workaround to get this to work with i18n or does this have to be fixed in the open-next core?

conico974 commented 1 week ago

Just remove the matcher and do the matching in code inside the middleware

sladkoff commented 1 week ago

Just remove the matcher and do the matching in code inside the middleware

Damn, I haven't considered that the matcher config is optional 😞 Thank you for this straight-forward workaround.

Should I leave this issue open? I guess a note in the docs could help if someone is using i18n with middleware matchers 🤷