-
Notifications
You must be signed in to change notification settings - Fork 86
fix: custom headers for root route when using i18n #1893
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
Conversation
✅ Deploy Preview for netlify-plugin-nextjs-export-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for netlify-plugin-nextjs-nx-monorepo-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for netlify-plugin-nextjs-static-root-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for next-plugin-edge-middleware ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for next-plugin-canary ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for netlify-plugin-nextjs-next-auth-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for nextjs-plugin-custom-routes-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for next-i18next-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for netlify-plugin-nextjs-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great. For future reference, we also have escapeStringRegexp
available in next-utils
for edge functions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the fix @orinokai! 🚢
Summary
When generating custom headers, the Runtime reads Next.js config rules and translates them into the required format for the TOML file. Part of that translation involves converting the
:nextInternalLocale
placeholder to a single localised rule for each locale.However, the condition for the conversion was not being matched when one or more of the locales included a special regex character because Next.js escapes the locale names before writing the routes-manifest.json file and we were not doing the same before matching.
This PR lifts the
escapeStringRegexp
function from Next.js and uses it to escape the locales before creating the path, in the same way that Next.js does.Note: in this case the problematic character was a
-
, which isn't a special character in regex, but one that Next.js escapes regardless.Test plan
Relevant links (GitHub issues, Notion docs, etc.) or a picture of cute animal
Fixes #1883