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

has field example seems wrong in next@canary #23415

Closed
gustavobini opened this issue Mar 26, 2021 · 5 comments · Fixed by #23588 or #23626
Closed

has field example seems wrong in next@canary #23415

gustavobini opened this issue Mar 26, 2021 · 5 comments · Fixed by #23588 or #23626
Assignees
Milestone

Comments

@gustavobini
Copy link

What version of Next.js are you using?

10.0.10-canary.11

What version of Node.js are you using?

12.2.0

What browser are you using?

Edge

What operating system are you using?

macOS

How are you deploying your application?

Not deploying

Describe the Bug

When following the recently added example provided in https://github.com/vercel/next.js/blob/canary/docs/api-reference/next.config.js/rewrites.md#header-cookie-and-query-matching for the has field with type query, the following error occurs when running next dev or next build:

`destination` has segments not in `source` (page) for route {"source":"/specific/:path*","has":[{"type":"query","key":"page","value":"home"}],"destination":"/:path*/:page"}

Expected Behavior

I am not entirely sure on this one. I would love if :page was accessible there but I believe the example is incorrect.

To Reproduce

npx create-next-app some-app

Create a next.config.js file in the root folder and add this content:

module.exports = {
  async rewrites() {
    return [
      {
        source: "/specific/:path*",
        has: [
          {
            type: "query",
            key: "page",
            value: "home",
          },
        ],
        destination: "/:path*/:page",
      },
    ];
  },
};

Run either next build or next start.

@gustavobini gustavobini added the bug Issue was opened via the bug report template. label Mar 26, 2021
@ijjk ijjk added kind: bug and removed bug Issue was opened via the bug report template. labels Mar 31, 2021
@ijjk ijjk self-assigned this Mar 31, 2021
@kodiakhq kodiakhq bot closed this as completed in #23588 Apr 1, 2021
@Timer Timer added this to the Iteration 18 milestone Apr 1, 2021
kodiakhq bot pushed a commit that referenced this issue Apr 1, 2021
This ensures we gather segments from the experimental has field when validating segments used in the destination to prevent the invalid segments in the destination error from showing incorrectly. This usage has been added to the custom-routes test suite to ensure the segments are passed correctly from the has field. 

Fixes: #23415

## Bug

- [x] Related issues linked using `fixes #number`
- [x] Integration tests added
@laugharn
Copy link
Contributor

laugharn commented Apr 1, 2021

#23588 doesn't appear to have fixed the example above that's also on https://nextjs.org/docs/api-reference/next.config.js/rewrites#header-cookie-and-query-matching

@gustavobini
Copy link
Author

thanks @ijjk this is a very exciting feature and I truly hope it gets to stable version

@laugharn
Copy link
Contributor

laugharn commented Apr 2, 2021

Yeah, I cannot wait for this!

@timneutkens timneutkens modified the milestones: Iteration 18, Iteration 19 Apr 8, 2021
@kodiakhq kodiakhq bot closed this as completed in #23626 Apr 13, 2021
kodiakhq bot pushed a commit that referenced this issue Apr 13, 2021
This is a follow-up to #23588 to update to use a regex lexer to gather the named regex groups instead of attempting to gather them through executing the regex since it can fail to gather the regex groups when they are using specific matching. This also ensures we don't pass the value as a segment when value is defined and it doesn't use a capture group. Additional tests are added to cover these cases and documentation updated to reflect this. 

Closes: #23415

## Bug

- [x] Related issues linked using `fixes #number`
- [x] Integration tests added

## Documentation / Examples

- [x] Make sure the linting passes
@ijjk
Copy link
Member

ijjk commented Apr 16, 2021

This should now be updated in v10.1.4-canary.9 of Next.js, please update and give it a try!

SokratisVidros pushed a commit to SokratisVidros/next.js that referenced this issue Apr 20, 2021
This ensures we gather segments from the experimental has field when validating segments used in the destination to prevent the invalid segments in the destination error from showing incorrectly. This usage has been added to the custom-routes test suite to ensure the segments are passed correctly from the has field. 

Fixes: vercel#23415

## Bug

- [x] Related issues linked using `fixes #number`
- [x] Integration tests added
SokratisVidros pushed a commit to SokratisVidros/next.js that referenced this issue Apr 20, 2021
This is a follow-up to vercel#23588 to update to use a regex lexer to gather the named regex groups instead of attempting to gather them through executing the regex since it can fail to gather the regex groups when they are using specific matching. This also ensures we don't pass the value as a segment when value is defined and it doesn't use a capture group. Additional tests are added to cover these cases and documentation updated to reflect this. 

Closes: vercel#23415

## Bug

- [x] Related issues linked using `fixes #number`
- [x] Integration tests added

## Documentation / Examples

- [x] Make sure the linting passes
flybayer pushed a commit to blitz-js/next.js that referenced this issue Apr 29, 2021
This ensures we gather segments from the experimental has field when validating segments used in the destination to prevent the invalid segments in the destination error from showing incorrectly. This usage has been added to the custom-routes test suite to ensure the segments are passed correctly from the has field. 

Fixes: vercel#23415

## Bug

- [x] Related issues linked using `fixes #number`
- [x] Integration tests added
@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
6 participants