-
Notifications
You must be signed in to change notification settings - Fork 88
fix: move locale detection to netlify redirects #861
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-nx-monorepo-demo ready! 🔨 Explore the source changes: 2e65907 🔍 Inspect the deploy log: https://app.netlify.com/sites/netlify-plugin-nextjs-nx-monorepo-demo/deploys/61a8eb1895451900082a7687 😎 Browse the preview: https://deploy-preview-861--netlify-plugin-nextjs-nx-monorepo-demo.netlify.app |
src/helpers/redirects.ts
Outdated
status: 301, | ||
conditions: { | ||
// eslint-disable-next-line @typescript-eslint/ban-ts-comment | ||
// @ts-ignore Incorrect type |
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.
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.
I'm going to fix the @netlify/build
types
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.
fb46877
to
a1bb287
Compare
✔️ Deploy Preview for netlify-plugin-nextjs-static-root-demo ready! 🔨 Explore the source changes: 2e65907 🔍 Inspect the deploy log: https://app.netlify.com/sites/netlify-plugin-nextjs-static-root-demo/deploys/61a8eb180ff53f0007ed8811 😎 Browse the preview: https://deploy-preview-861--netlify-plugin-nextjs-static-root-demo.netlify.app |
b6d9ea4
to
35aaa3e
Compare
35aaa3e
to
02d2b9b
Compare
d8d124f
to
3b832d1
Compare
7e26769
to
267af0d
Compare
267af0d
to
14fbaec
Compare
This comment has been minimized.
This comment has been minimized.
I'm going to merge this, because even though the libredirect issue means the autodetection based on browser header isn;t working, it's better than the current behaviour |
@ascorbic Is the next.js built in autodetection still not supported? |
Summary
Currently, locale detection requires the request to reach the handler function, which means it breaks for static or ODB routes. This PR moves the detection into Netlify redirects, so it happens before other redirects are processed.
This PR also refactors the redirects into a different file, because it was getting unwieldy keeping it inside the config helper
Test plan
/es/
Relevant links (GitHub issues, Notion docs, etc.) or a picture of cute animal
Fixes #859
Standard checks:
🧪 Once merged, make sure to update the version if needed and that it was published correctly.