Skip to content

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

Merged
merged 8 commits into from
Dec 2, 2021
Merged

Conversation

ascorbic
Copy link
Contributor

@ascorbic ascorbic commented Dec 1, 2021

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

  1. TODO: create a test site with a static or fallback page at the root
  2. Visit the page with a browser locale of "es"
  3. Observe a redirect to /es/

Relevant links (GitHub issues, Notion docs, etc.) or a picture of cute animal

image

Fixes #859

Standard checks:

  • Check the Deploy Preview's Demo site for your PR's functionality
  • Add docs when necessary

🧪 Once merged, make sure to update the version if needed and that it was published correctly.

@netlify
Copy link

netlify bot commented Dec 1, 2021

✔️ 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

@github-actions github-actions bot added the type: bug code to address defects in shipped code label Dec 1, 2021
@ascorbic ascorbic requested a review from tiffafoo December 1, 2021 10:31
status: 301,
conditions: {
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore Incorrect type
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So many ignores

worried kermit gif

Copy link
Contributor Author

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ascorbic ascorbic force-pushed the mk/locale-detection branch from fb46877 to a1bb287 Compare December 1, 2021 12:58
@ascorbic ascorbic marked this pull request as ready for review December 1, 2021 14:32
@netlify
Copy link

netlify bot commented Dec 1, 2021

✔️ 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

@ascorbic ascorbic force-pushed the mk/locale-detection branch 4 times, most recently from b6d9ea4 to 35aaa3e Compare December 1, 2021 15:05
@ascorbic ascorbic requested a review from tiffafoo December 1, 2021 15:07
@ascorbic ascorbic force-pushed the mk/locale-detection branch from 35aaa3e to 02d2b9b Compare December 1, 2021 15:24
@ascorbic ascorbic force-pushed the mk/locale-detection branch from d8d124f to 3b832d1 Compare December 1, 2021 16:18
@ascorbic ascorbic force-pushed the mk/locale-detection branch 2 times, most recently from 7e26769 to 267af0d Compare December 1, 2021 16:38
@ascorbic ascorbic force-pushed the mk/locale-detection branch from 267af0d to 14fbaec Compare December 1, 2021 17:05
@tiffafoo tiffafoo self-requested a review December 2, 2021 08:49
@tiffafoo

This comment has been minimized.

@ascorbic
Copy link
Contributor Author

ascorbic commented Dec 2, 2021

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 ascorbic merged commit 964637b into main Dec 2, 2021
@ascorbic ascorbic deleted the mk/locale-detection branch December 2, 2021 16:02
@andykenward
Copy link

@ascorbic Is the next.js built in autodetection still not supported?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug code to address defects in shipped code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: handle locale autodetection in Netlify redirects
3 participants