Skip to content

Conversation

@conico974
Copy link
Contributor

This PR fix #766

They were 2 different issues described there:

  • Query params defined in the destination of a rewrite were not forwarded as expected
  • Internal destination that are api route should be stripped from the appended locale. (If locale is not set to false, next will always transform in rewrite with an appended locale, but it expects the api route to be not locale prefixed)

@nicholas-c Could you confirm that it fixes your issue

@changeset-bot
Copy link

changeset-bot bot commented Mar 4, 2025

🦋 Changeset detected

Latest commit: 460ccc6

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@opennextjs/aws Patch
app-pages-router Patch
app-router Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@pkg-pr-new
Copy link

pkg-pr-new bot commented Mar 4, 2025

Open in Stackblitz

pnpm add https://pkg.pr.new/@opennextjs/aws@767

commit: 460ccc6

@nicholas-c
Copy link

@nicholas-c Could you confirm that it fixes your issue

Absolutely, I'll patch my test link in an hour or two - Will update here when done

Copy link
Contributor

@khuezy khuezy left a comment

Choose a reason for hiding this comment

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

LGTM! SHIP IT after @nicholas-c confirms the fix.

@khuezy
Copy link
Contributor

khuezy commented Mar 4, 2025

cc: @sommeeeer
Should we probably add an i18n app for e2e to catch these things.

@conico974
Copy link
Contributor Author

@khuezy page router already has some i18n config.
We just didn't test these specific cases

@sommeeeer
Copy link
Contributor

cc: @sommeeeer Should we probably add an i18n app for e2e to catch these things.

yeah, most definitely. we have discussed having some sort of logic that builds different next apps with different configurations (basePath, i18n etc) and then tests them in a Github Action locally. without deploying the actual apps. since most of it can probably be tested that way, and dont really need a deploy. perhaps, until we have that up and going, perhaps we should consider adding an app with some i18n and locales?

@nicholas-c
Copy link

@conico974 working perfectly 👌

API route working:
https://next-15-en.nich-carter-qza3ng.gymshark-sandbox.tools/account/register

i18n config working:
https://next-15-es.nich-carter-qza3ng.gymshark-sandbox.tools/account/register
https://next-15-es.nich-carter-qza3ng.gymshark-sandbox.tools/fr/account/register

Thanks again!

@conico974 conico974 merged commit 068ce66 into opennextjs:main Mar 4, 2025
3 checks passed
@github-actions github-actions bot mentioned this pull request Mar 4, 2025
@conico974 conico974 deleted the fix/rewrite-query branch March 14, 2025 12:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

next.config rewrites don't get handled properly

4 participants