Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The matcher in middleware.ts is incompatible with presence of i18n in config #164

Closed
patrik-simunic-cz opened this issue Nov 12, 2022 · 6 comments

Comments

@patrik-simunic-cz
Copy link

patrik-simunic-cz commented Nov 12, 2022

Please see this issue: vercel/next.js#42795 (After 2 days of dealing with this, I really don't have the strength deal with this anymore.)
In short, you platforms demo is utterly incompatible with the very presence of i18n in next.config.js. Once you add i18n config, the behaviour of middleware changes and the matcher is no longer valid. Index page (and only index page) starts ending on 404 and it's truly, truly pain in the ass to debug this. Especially since you expect the official example to be correct and compatible with such a basic as is internationalisation. Only when I even considered that it might be a F-up on your part I was able to solve this. I had to just throw the official matcher to the bin and handle exceptions to rewrite programatically within the middleware itself.

(Mention or warning in the official guide would also be nice...... :))))

@rklubenspies
Copy link

@patrik-simunic-cz Your issues helped us discover and solve this same perplexing challenge. In our case, we could not get any index page routes to work on Vercel and on all pages' Tailwind CSS assets would be broken on load.

You mentioned solving this by handling exceptions to rewrite programmatically within the middleware itself. Is there a minimum reproduction of this logic that you would be willing to share on the issue here? I think a lot of other internationalization-minded developers would appreciate a starting point if they also find this issue like I did.

@steven-tey
Copy link
Contributor

This example does not account for i18n since it's an advanced feature – the full list of features are stated here: https://github.com/vercel/platforms#template-features

Also, this is a starter kit, not a full-fledged SaaS application, which means that it's not production ready by any means.

That being said, I'm going to take a look at how I can make i18n work with middleware, thank you for the feedback.

@patrik-simunic-cz
Copy link
Author

@rklubenspies I don't have a minimal reproducible repo, but in short: I just removed the regex matcher in exported config (as is in the official docs) and replaced it with this:

import type { NextURL } from 'next/dist/server/web/next-url'
import { NextRequest, NextResponse } from 'next/server'

import { config } from './config'

const isException = (url: NextURL) => {
    if (/^\/(_next|api|admin|locales|images|fonts)/.test(url.pathname)) {
        return true
    }

    return false
}

export default (request: NextRequest) => {
    const url = request.nextUrl

    if (isException(url)) {
        return
    }

    const hostname = request.headers.get('host') || config.defaultDomains[0]

    url.pathname = `/tenant/${hostname}${url.pathname}`
    return NextResponse.rewrite(url)
}

Basically, just NextResponse.rewrite everything, except all request.nextUrl.pathname that you manually check for in the middleware itself (instead of relying on the config).

@patrik-simunic-cz
Copy link
Author

patrik-simunic-cz commented Nov 24, 2022

@steven-tey Well, you may call it an advanced feature, but it's really not. Especially considering that we're talking about multi-tenant solution - if you're dealing with multi-tenant app, it's more than likely you're also dealing with internationalisation.

The logic goes like this:
(Multi-tenant app is very likely also internationalised app.) Multi-tenant app requires middleware rewrites. I18n changes behaviour of middleware matcher in a very non-obvious way. Ergo docs for multi-tenant app should have a waring about this fact.

Regardless, whether it's „advanced feature“ or not, it doesn't really change anything. The point is that enabling/disabling i18n changes the behaviour of the middleware matcher in a way that is far from obvious. If it's disabled the / page is not matched as an exception, if it's enabled the / page is matched as an exception for the middleware. Something changes inside the Next.js routing implementation without any obvious change from the outside that could alert you to this fact.

Don't take me wrong, I really appreciate how Next.js handles the internationalisation out-of-the-box. The problem is that it's a black box and black boxes need to be documented very well, otherwise they cause more problems because it is truly difficult to debug them. So if you're showcasing the middleware rewrite feature, there is supposed to be a big red warning right next to it shouting „Be ware!! If you do X (enable i18n), the routing behaviour inside this black box changes in a way that you need to take into account!“

@steven-tey
Copy link
Contributor

@patrik-simunic-cz agreed, added a caveat here: vercel/next.js#36563

Also, since we're in the process of migrating to the /app directory in Next.js 13 (which does not currently support i18n), I'm going to go ahead and close this issue for now – we will revisit this when i18n is supported in Next.js 13.

@steven-tey
Copy link
Contributor

Hey @patrik-simunic-cz! We just released this example that uses i18n with middleware (instead of delineating it in next.config.js)

I'm planning to integrate this into the new version of Platforms Starter Kit but before then, feel free to give it a try!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants