Skip to content
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

[Next-9.5.4] Rewrites URLs are not masked properly and instead interpolated. #17810

Closed
umarsenpai opened this issue Oct 12, 2020 · 2 comments
Closed

Comments

@umarsenpai
Copy link
Contributor

Bug report

Describe the bug

Hi, I have a rewrite in my app for rewriting a slug based url to legacy id based urls like:

{
  source: "/blog/:slug",
  destination: "/page/[authorId]/[pageId]"
},

Before Next 9.5.3, I was using router.push like this to navigate to slug based URL:

router.push(
     "/page/[authorId]/[pageId]?authorId=123&pageId=456&slug=some-blog",
     "/blog/some-blog"
)

It works fine on 9.5.3 as well.

But, after moving to 9.5.4, the above rewrite gets interpolated and the URL on browser is displayed like: /page/123/456?slug=some-blog instead of /blog/some-blog.

Is there another way to navigate to /blog/some-blog? Or is it an issue with the interpolate?

Regression PR: #16774
Line which causes this: https://github.com/vercel/next.js/pull/16774/files#diff-5eff10c96fa6858c659318492c4cf7f7R637

To Reproduce

I have created two codesandbox, one with 9.5.2 and one with 9.5.4 to show the behavior:

Next 9.5.4

  1. Go to https://codesandbox.io/s/nextjs-next-forked-6nh2z
  2. Click on 'Go to blog page'
  3. The URL displayed is /page/123/456?slug=some-blog

Expected behavior

  1. Go to https://codesandbox.io/s/nextjs-next-forked-qq1tc
  2. Click on 'Go to blog page'
  3. The URL displayed is /blog/some-blog

System information

  • Version of Next.js: 9.5.4
@timneutkens timneutkens added good first issue Easy to fix issues, good for newcomers kind: bug labels Oct 12, 2020
@timneutkens timneutkens added this to the 9.x.x milestone Oct 12, 2020
@Timer Timer removed this from the 9.x.x milestone Oct 12, 2020
@ijjk
Copy link
Member

ijjk commented Oct 14, 2020

This doesn't seem to be expected 🤔 the rewrites feature isn't meant to map the dynamic route like this and instead should have the full URL already provided. The dynamic route interpolation is a client-side feature as shown in the docs here https://nextjs.org/docs/routing/introduction#linking-to-dynamic-paths

The above rewrite should be something like this instead:

{
  source: "/blog/:authorId/:slug",
  destination: "/page/:authorId/:slug"
},

If you're using unconventional routing like this href/as shouldn't have breaking changes as mentioned in the issue and can still be used, if I'm misunderstanding and href/as aren't working anymore this would be a bug and does need to be corrected otherwise this appears to be more a feature request.

@Timer Timer added this to the 10.x.x milestone Nov 1, 2020
@ijjk ijjk added type: needs investigation and removed good first issue Easy to fix issues, good for newcomers kind: bug labels Dec 1, 2020
@Timer Timer removed this from the 10.x.x milestone Feb 11, 2021
@balazsorban44
Copy link
Member

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.

@vercel vercel locked as resolved and limited conversation to collaborators Jan 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants