Fix redirect url for prefixing the default locale #33762
Merged
+3
−1
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We want to prefix the default locale to the current path (`/path`), not the current href (`http://domain.tld/path`).
lancechentw
requested review from
huozhi,
ijjk,
leerob,
padmaia,
shuding,
styfle and
timneutkens
as code owners
styfle
reviewed
Feb 4, 2022
| @@ -177,7 +177,7 @@ export function middleware(request: NextRequest) { | |||
| request.nextUrl.locale === 'default' | |||
|
|
|||
| return shouldHandleLocale | |||
| ? NextResponse.redirect(`/en${request.nextUrl.href}`) | |||
| ? NextResponse.redirect(`/en${request.nextUrl.pathname}`) | |||
There was a problem hiding this comment.
What about when there is a query string?
I think it should be something like
const url = new URL(request.nextUrl)
url.pathname = `/en${url.pathname}`
NextResponse.redirect(url)There was a problem hiding this comment.
Good catch @styfle! I am thinking about not to create a new object, but not sure if there is any pitfall doing this, something like
request.nextUrl.pathname = `/en${request.nextUrl.pathname}`
NextResponse.redirect(`/en${request.nextUrl.href}`)The previous fix redirect to the correct default locale url, but would drop the query string. This commit preserve the original query string by redirecting to the full href.
kodiakhq bot
pushed a commit
that referenced
this pull request
Feb 6, 2022
The recommended code for `_middleware.ts` does not work in prod, but does work locally. You need to use `request.nextUrl.pathname` to properly redirect from what I can tell. You also need to have a quick helper function to strip off the `/default` locale at the start of the pathname as we are providing `/en` as a fallback locale. FWIW - I am pretty new to NextJS. Someone with more experience should probably review this suggestion before merging it. What I can tell you however is that the code as it is in `_middleware.ts` works locally but breaks in prod. To test this out use the code and navigate to `https//www.mysite.com` - it will work as expected on the root url, as this matches `nextUrl.href`. Now try navigating to `https//www.mysite.com/about` and you will be redirected to `https://www.mysite.com/en/https://www.mysite.com/about`. ## Documentation / Examples - [ ] Make sure the linting passes by running `yarn lint` Closes: #33762 Co-authored-by: JJ Kasper <22380829+ijjk@users.noreply.github.com>
# Conflicts: # docs/advanced-features/i18n-routing.md
natew
pushed a commit
to natew/next.js
that referenced
this pull request
Feb 16, 2022
The recommended code for `_middleware.ts` does not work in prod, but does work locally. You need to use `request.nextUrl.pathname` to properly redirect from what I can tell. You also need to have a quick helper function to strip off the `/default` locale at the start of the pathname as we are providing `/en` as a fallback locale. FWIW - I am pretty new to NextJS. Someone with more experience should probably review this suggestion before merging it. What I can tell you however is that the code as it is in `_middleware.ts` works locally but breaks in prod. To test this out use the code and navigate to `https//www.mysite.com` - it will work as expected on the root url, as this matches `nextUrl.href`. Now try navigating to `https//www.mysite.com/about` and you will be redirected to `https://www.mysite.com/en/https://www.mysite.com/about`. ## Documentation / Examples - [ ] Make sure the linting passes by running `yarn lint` Closes: vercel#33762 Co-authored-by: JJ Kasper <22380829+ijjk@users.noreply.github.com>
natew
pushed a commit
to natew/next.js
that referenced
this pull request
Feb 16, 2022
We want to prefix the default locale to the current path (`/path`), not the current href (`http://domain.tld/path`). ## Documentation / Examples - [x] Make sure the linting passes by running `yarn lint` Co-authored-by: JJ Kasper <22380829+ijjk@users.noreply.github.com>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We want to prefix the default locale to the current path (
/path), not the current href (http://domain.tld/path).Documentation / Examples
yarn lint