fix: pass only first language to next-server to match platform redirects support #2138
Conversation
✅ 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 nextjs-plugin-custom-routes-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. |
✅ 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 next-i18next-demo 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-export-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
| // https://docs.netlify.com/routing/redirects/redirect-options/#redirect-by-country-or-language | ||
| // > Language-based redirects always match against the first language reported by the browser in the Accept-Language header regardless of quality value weighting. | ||
| // If we wouldn't keep just first language, it's possible for `next-server` to generate locale redirect that could be cached by ODB | ||
| event.headers['accept-language'] = event.headers['accept-language'].replace(/\s*,.*$/, '') |
There was a problem hiding this comment.
maybe this should be done just for odb mode, and let ssr keep all languages, but it feels weird to change behavior between 2 of them?
There was a problem hiding this comment.
That's a good point, but worse, if we change it for SSR too it's going to be a breaking change because it will stop Next.js handling non-primary locales, meaning users with secondary/tertiary languages in their accept-language header will no longer see content in their language on SSR pages 😬
There was a problem hiding this comment.
Having different behaviour between SSR/ODB feels wrong, and I guess it's because what we're doing is actually trying to workaround our lack of support by imposing the same limitation on Next, which would be a regression.
There was a problem hiding this comment.
Yeah, to me it's picking one of 2 bad options and figuring it which one is worse/better. I do agree it's probably better to keep SSR pages behaving as they are (so supporting matching on all languages on the header, even if there are multiple source that can produce redirect) and limit the workaround to just ODB (it does technically make things worse to match on lowest common denominator to avoid caching wrong things, so we want to limit area we make ~worse).
In any case 4c85c21 limits the workaround just to ODB
pieh
force-pushed
the
fix/non-first-language-locale-redirect
branch
from
f884088 to
d4ea6e6
Compare
Description
Netlify locale redirects match only on first language listed in
Accept-Languageheader.next-serverhandles all of them.https://docs.netlify.com/routing/redirects/redirect-options/#redirect-by-country-or-language
This means some requests might skip Netlify language redirects (when Automatic Locale Detection in next.js is not disabled) and hit lambda that could produce locale redirect. For SSR this might not a problem, but this is a problem for ODB, because that locale redirect gets cached for everyone.
This PR basically tricks
next-serverto only handle first language (to match platform capability) by removing all languages except of first one inAccept-Languageheader before passing it tonext-serverDocumentation
Tests
Added cypress test, but can be also verified manually like this:
Examine status code / response headers of requests to
/with__prerender_bypasscookie (/is using ODB, so we want to ensure we execute it) with specifically crafterAccept-Languagewhere first language on the list is not in i18n config, but there is a language in the list overall that is in i18n config - for exampleAccept-Language: pl, fr-CH;q=0.9, fr;q=0.9, en;q=0.8, de;q=0.7, *;q=0.5(plnot in i18n,fris in i18n)mainbranchCurl command:
Result (truncated for readability):
Above is the problem because this would be cached for everyone if request like that happens before ODB cache is hydrated or after it expired. Note - 307 is redirect from
next-server, our are 301s (separate discussion if those should be 301 or 302)Preview deploy for this PR:
Curl command:
Result:
Relevant links (GitHub issues, etc.) or a picture of cute animal