-
Notifications
You must be signed in to change notification settings - Fork 27k
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
[i18n] non-default locale root level routes (i.e. /fr /es) get 307 redirects to themselves? #21943
Comments
Hi, in the above lines in |
@ijjk hey thanks for the response :). i think that at this point, parsedUrl.pathname is still that said, it seems normalizeLocalePath is the primary suspect. from within normalizeLocalePath:
for a pathname i'm v confused how this isnt an issue on |
update (since i was double checking the value of parsedUrl.pathname after the req.url format call because parsedUrl.pathname is what's passed to normalizeLocalePath): i'm not sure whereabouts in the source this particular line in the bundle comes from (line 5714 ^).... but this is definitely the culprit in terms of parsedUrl.pathname becoming |
@ijjk hey jj, sorry for the ping. able to offer any thoughts on this quickly? much appreciated 🙏 |
Hi, just double checked and in I logged out the values after these lines for better visibility running against the $ node --trace-deprecation packages/next/dist/bin/next test/integration/i18n-support/
ready - started server on 0.0.0.0:3000, url: http://localhost:3000
info - Using external babel configuration from /Users/jj/dev/vercel/next.js/test/.babelrc
{
pathname: '/nl',
detectedLocale: 'nl',
shouldAddLocalePrefix: false,
parsedUrlPathname: '/nl',
denormalizedPagePath: '/nl',
detectedDefaultLocale: false,
shouldStripDefaultLocale: false
} It looks like this value is being updated in the serverless handler here and used here which is causing the behavior noted above, correcting that should resolve the issue. Thanks for helping investigate this, feel free to send a PR updating this and we can get this updated! |
… serverelss handler
This issue has been automatically locked due to no recent activity. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you. |
What version of Next.js are you using?
10.0.6
What version of Node.js are you using?
14.12.0
What browser are you using?
Chrome
What operating system are you using?
macOS
How are you deploying your application?
Other platform
Describe the Bug
i18n + target: serverless
this line is true only for non-default locale root routes (aka
/pl
or/fr
etc). whenshouldAddLocalePrefix
is true, it passes this condition (which therefore fails for non-root routes like/pl/static
) and sets up a 307 to/pl
, which can cause infinite redirects.i'm looking to better understand why this logic exists for root non-default locale pages in i18n. specifically, this line:
const shouldAddLocalePrefix = !detectedDefaultLocale && denormalizedPagePath === '/'
what is the purpose of this condition?
denormalizedPagePath
becomes/
when req.url is/fr
or/pl
. so like i said, it will eventually get to the res.setHeader and formatUrl using this value${basePath || ''}/${detectedLocale}
which becomes/fr
or/pl
. the problem with this is if you're serving next page (.next/serverless/pages/index.js
) for/${locale}
, the 307 will hit/${locale}
again, reprocess the next page, re-hit the 307, etc etc.i traced this work back to @ijjk so i assume they'll have the most insight :)
Expected Behavior
no 307s from
/${locale}
to/${locale}
To Reproduce
this is mainly just a question to understand so no need to repro. as answered previously though, yes, this is observed on a platform that is not vercel. my hope is that next maintainers are still diligent about helping those who don't or can't host on vercel! 🙏
thanks so much for your time and help! really appreciate it <3
The text was updated successfully, but these errors were encountered: