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

[9.5.3 - Rewrites] Navigating route with dynamic routes no longer work properly #16825

Closed
nghiepdev opened this issue Sep 3, 2020 · 8 comments · Fixed by #16860
Closed

[9.5.3 - Rewrites] Navigating route with dynamic routes no longer work properly #16825

nghiepdev opened this issue Sep 3, 2020 · 8 comments · Fixed by #16860
Assignees
Milestone

Comments

@nghiepdev
Copy link
Contributor

nghiepdev commented Sep 3, 2020

Describe the bug

// next.config.js
module.exports = {
  rewrites() {
    return [
      {
        source: "/my-post/:id",
        destination: "/post"
      }
    ];
  }
};

Navigating via Link or router.push

      <Link
        href={{
          pathname: "/post",
          query: {
            id: 1,
            another: "another"
          }
        }}
        as="/my-post/1?another=another"
      >
        <a>Post 1</a>
      </Link>

When custom the as path with params then router.query.id no longer true.

Actual: id=1?another=another
Expected: id=1

To Reproduce

https://codesandbox.io/s/aged-sky-nzs6j?file=/pages/index.js

@nghiepdev nghiepdev changed the title [9.5.3 - Rewrites] Navigating custom as no longer work propely [9.5.3 - Rewrites] Navigating route with dynamic routes no longer work properly Sep 3, 2020
@MrEfrem
Copy link

MrEfrem commented Sep 4, 2020

I also updated and all's broken now.

@ijjk
Copy link
Member

ijjk commented Sep 4, 2020

Hi, you should no longer need to map the href and as manually for custom-routes and next/link as they are being resolved automatically and the above snippet works ok if you only provide the href with the destination URL e.g.

<Link href="/my-post/1?another=another">
  <a>Post 1</a>
</Link>

I opened a PR to ensure this is being correctly resolved when mapping manually with href and as and automatically with only href here

@kodiakhq kodiakhq bot closed this as completed in #16860 Sep 4, 2020
kodiakhq bot pushed a commit that referenced this issue Sep 4, 2020
This makes sure we only pass the as value's `pathname` instead of the full value so that we don't accidentally include `query` values while resolving the rewrites. This also adds tests to ensure the rewrites are resolved with the correct query values when only providing `href` and when manually mapping them with `href` and `as`

Fixes: #16825
@Timer Timer modified the milestones: 9.x.x, iteration 8 Sep 4, 2020
@ijjk
Copy link
Member

ijjk commented Sep 5, 2020

This should be fixed in v9.5.4-canary.3 of Next.js please upgrade and try it out!

@MrEfrem
Copy link

MrEfrem commented Sep 5, 2020

@ijjk I've /en/software.jsx and /en/[post].jsx files. When I navigated to /en/software/ via Router.push('/en/software/') in v9.5.2 it resolved to /en/software.jsx correctly. But in v9.5.3 and v9.5.4-canary.3 it resolves to /en/[post].jsx. Is this intentional change? And why in the minor releases do you change so things?

@ijjk
Copy link
Member

ijjk commented Sep 5, 2020

@MrEfrem I opened a new issue to track the specific issue you are encountering here

@nghiepdev
Copy link
Contributor Author

Hi, you should no longer need to map the href and as manually for custom-routes and next/link as they are being resolved automatically and the above snippet works ok if you only provide the href with the destination URL e.g.

<Link href="/my-post/1?another=another">
  <a>Post 1</a>
</Link>

I had tried this, but throw error 404 and auto reload page.
Link example: https://codesandbox.io/s/hungry-breeze-675gf?file=/pages/index.js

@Timer
Copy link
Member

Timer commented Sep 10, 2020

@nghiepit can you please try [email protected]?

HitoriSensei pushed a commit to HitoriSensei/next.js that referenced this issue Sep 26, 2020
This makes sure we only pass the as value's `pathname` instead of the full value so that we don't accidentally include `query` values while resolving the rewrites. This also adds tests to ensure the rewrites are resolved with the correct query values when only providing `href` and when manually mapping them with `href` and `as`

Fixes: vercel#16825
@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 29, 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

Successfully merging a pull request may close this issue.

5 participants