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

13.3.0 Intercepting with grouped app does not work #48104

Closed
1 task done
khuezy opened this issue Apr 7, 2023 · 30 comments · Fixed by #48351
Closed
1 task done

13.3.0 Intercepting with grouped app does not work #48104

khuezy opened this issue Apr 7, 2023 · 30 comments · Fixed by #48351
Labels
bug Issue was opened via the bug report template.

Comments

@khuezy
Copy link
Contributor

khuezy commented Apr 7, 2023

Verify canary release

  • I verified that the issue exists in the latest Next.js canary release

Provide environment information

Operating System:
      Platform: darwin
      Arch: arm64
      Version: Darwin Kernel Version 22.3.0: Mon Jan 30 20:38:37 PST 2023; root:xnu-8792.81.3~2/RELEASE_ARM64_T6000
    Binaries:
      Node: 18.14.0
      npm: 9.3.1
      Yarn: 1.22.19
      pnpm: 7.27.0
    Relevant packages:
      next: 13.3.1-canary.2
      eslint-config-next: N/A
      react: 18.2.0
      react-dom: 18.2.0

Which area(s) of Next.js are affected? (leave empty if unsure)

No response

Link to the code that reproduces this issue

/~https://github.com/khuezy/next-intercept

To Reproduce

  • npm install
  • npm run dev
  • go to localhost:3000/shop
  • Click on Go to nested, route is stuck
  • go to localhost:3000/shop_no_group
  • Click on Go to nested, route is intercepted

Describe the Bug

When using group routes, interception does not work.
eg:

app/(group)/shop

Expected Behavior

When using grouped routes, interception should work.

Which browser are you using? (if relevant)

No response

How are you deploying your application? (if relevant)

No response

@khuezy khuezy added the bug Issue was opened via the bug report template. label Apr 7, 2023
@khuezy
Copy link
Contributor Author

khuezy commented Apr 7, 2023

I had to look over the e2e tests in nextjs to find out that you need default.tsx in order to fix the 404. Would be nice to not have to create a temp file though.

@khuezy khuezy closed this as completed Apr 7, 2023
@khuezy khuezy reopened this Apr 7, 2023
@khuezy khuezy changed the title 13.3.0 Intercepting nested dynamic pages results in 404 13.3.0 Intercepting with grouped app does not work Apr 7, 2023
@bob-obringer
Copy link

Running into the same problem. Sometimes it seems to sort of half work the first time, but otherwise, it's broken with route groups.

@borispoehland
Copy link

borispoehland commented Apr 9, 2023

Same here. Also, it would be lovely to configure that an interception happens on all routes of a specific group, as currently you would have to duplicate the interception path across all routes where we want interception to happen: #48157

@amlcodes
Copy link

same. ah, guess it's a grouped routes issue, not dynamic route issue.but repro here: /~https://github.com/amlcodes/amlcodes-nextgram-intercepting-dynamic-routes-issue

@feedthejim
Copy link
Contributor

looking!

@feedthejim
Copy link
Contributor

should be fixed by #48276

@khuezy
Copy link
Contributor Author

khuezy commented Apr 12, 2023

You the man @feedthejim !

@khuezy
Copy link
Contributor Author

khuezy commented Apr 12, 2023

@feedthejim :( I don't think the fix addresses interception w/ modal.
eg:

/photos/[author]/[id]/page.tsx // shows the individual photo by the author
/photos/page.tsx                      // shows all the photos
/photos/@modal/(.)[author]/[id]/page.tsx // doesn't appear to work (shows a modal of the individual photo on top of all the photos)

@feedthejim
Copy link
Contributor

@khuezy can you open a repro with the latest canary?

@Nickman87
Copy link

Nickman87 commented Apr 13, 2023

@feedthejim I think I have the same issue where intercepting routes is not working inside of a group folder (using 13.3.1-canary.6)

I built a small reproduction here: https://codesandbox.io/p/sandbox/nextjs-intercepting-routes-s9j6mo

I have a REAL group dashboard (using a (group) folder) and a FAKE group dashboard using exactly the same code and folder structure but inside a regular group folder (which means it is also like that in the path of the URL).

Testing the fake one, the modal content is loaded as expected, when you go to the real one, next will always navigate away.

Hope this helps you track down the issue :)

--- edit ---
I added a third option being a dashboard nested inside a group folder, because I thought it might be because the intercepting route ends inside the group folder, but that one also does not seem to work as it should.

@khuezy
Copy link
Contributor Author

khuezy commented Apr 13, 2023

@Nickman87 thanks for putting up the reproduction demo. @feedthejim the sandbox above is the same setup I want to use but currently doesn't work.

@feedthejim
Copy link
Contributor

@Nickman87 I looked at the repo and it seems that there's a bit of a misunderstanding: route groups are not part of the URL, so in the "real" route group, you're wrongly routing to the /group whereas you should route to /dashboard.

If you change it, it will fix it. Is that what you intended?

CleanShot 2023-04-13 at 17 58 57@2x

@Nickman87
Copy link

Nickman87 commented Apr 13, 2023

Hi @feedthejim,

This is exactly what I am trying to demonstrate, so it is intentional.

If I put the exact same code inside of a real path folder, then it works, but if I use the (group) naming, then the modal does not seem to work the way it should. Check out the differences between the two setups (the code inside is identical but the execution is not)

@khuezy
Copy link
Contributor Author

khuezy commented Apr 13, 2023

@feedthejim can you look at: https://codesandbox.io/p/sandbox/nostalgic-chaplygin-tfyi0p

Append "/photos" to the site url, eg: "csb.app/photos".

Clicking on the "Go to /photos/[author]/[id]" will navigate to the page, I expect it to intercept.
If you then click "Back" in the browser, the navigation seems to be stuck too.

Edit: 1 second please, I have typo in the sub folder.

@feedthejim
Copy link
Contributor

but if I use the (group) naming, then the modal does not seem to work the way it should

Yes, that's what I was saying to you in the previous comment. You are linking to a URL that links to /group/item/new in your route group. This will link to the second modal, the one that exists in the "real" folder. A route group does not impact the URL, so if you change the URL to point to /dashboard/item/new you will see the correct modal

@feedthejim
Copy link
Contributor

thanks for the repro @khuezy I can confirm it does not work as expected, let me investigate

@Nickman87
Copy link

Nickman87 commented Apr 13, 2023

@feedthejim You are right, I did make a mistake there, my bad :).
I changed the links to be correct now and the error still persists, The modal is not loaded but next redirects to the page.
Do you see any other issues with the setup now?

I think the issue is identical as in @khuezy's setup, only his is maybe easier to follow 😅 but I tried to rebuild the structure I have to reproduct it :)

@feedthejim
Copy link
Contributor

ohhhh okay I know what the issue is!

@khuezy
Copy link
Contributor Author

khuezy commented Apr 13, 2023

Would it be possible to make (.) optional, ie it should default to the current directory of the "@modal".
eg: @modal/(.)[author] and @modal/[author] are equivalent. You'd use (...) or (..) to go to root or up a level, respectively.

@feedthejim
Copy link
Contributor

Opened a PR for the route group support!

@khuezy no because that would disable the interception route! checkout the doc for both concepts https://beta.nextjs.org/docs/routing/parallel-routes

@khuezy
Copy link
Contributor Author

khuezy commented Apr 13, 2023

Gotcha, thanks for the quick turnaround!

@amlcodes
Copy link

you're the goat @feedthejim

feedthejim added a commit that referenced this issue Apr 13, 2023
fixes #48104 

This PR fixes route groups breaking interception routes. I hadn't
realised that route groups were actually part of the tree router, so we
were not stripping them out in the interception matcher. Fixed now.

<!-- Thanks for opening a PR! Your contribution is much appreciated.
To make sure your PR is handled as smoothly as possible we request that
you follow the checklist sections below.
Choose the right checklist for the change(s) that you're making:

## For Contributors

### Improving Documentation or adding/fixing Examples

- The "examples guidelines" are followed from our contributing doc
/~https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md
- Make sure the linting passes by running `pnpm build && pnpm lint`. See
/~https://github.com/vercel/next.js/blob/canary/contributing/repository/linting.md

### Fixing a bug

- Related issues linked using `fixes #number`
- Tests added. See:
/~https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs
- Errors have a helpful link attached, see
/~https://github.com/vercel/next.js/blob/canary/contributing.md

### Adding a feature

- Implements an existing feature request or RFC. Make sure the feature
request has been accepted for implementation before opening a PR. (A
discussion must be opened, see
/~https://github.com/vercel/next.js/discussions/new?category=ideas)
- Related issues/discussions are linked using `fixes #number`
- e2e tests added
(/~https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs
- Documentation added
- Telemetry added. In case of a feature if it's used or not.
- Errors have a helpful link attached, see
/~https://github.com/vercel/next.js/blob/canary/contributing.md



## For Maintainers

- Minimal description (aim for explaining to someone not on the team to
understand the PR)
- When linking to a Slack thread, you might want to share details of the
conclusion
- Link both the Linear (Fixes NEXT-xxx) and the GitHub issues
- Add review comments if necessary to explain to the reviewer the logic
behind a change

### What?

### Why?

### How?

Closes NEXT-
Fixes #

-->
@borispoehland
Copy link

Hey @feedthejim, I was trying to apply that in my Nextgram demo from a different issue and it does not work. Could you please take a look?
/~https://github.com/borispoehland/nextgram-demo
https://nextgram-demo.vercel.app/

I took the working example and broke it by putting the 2 routes / and /test into the (app) group

@resthedev
Copy link

On a related note, I am having issues getting intercept to work when there are multiple nested dynamic slugs .
For example, intercepting at (...)posts/[collectionId]/[postId] does not work. Only (...posts)/[postId] works.

@TrustyTechSG
Copy link

TrustyTechSG commented May 8, 2023

On a related note, I am having issues getting intercept to work when there are multiple nested dynamic slugs . For example, intercepting at (...)posts/[collectionId]/[postId] does not work. Only (...posts)/[postId] works.

I have exact the same issue on 13.4.1,

/[lang]/checkout/[businessCode]/[serviceId]/@calendar/(..)testpath **work**
/[lang]/checkout/[businessCode]/[serviceId]/@calendar/(..)(..)anothertestpath **not work** (always hard navigate to anothertestpath)
/[lang]/checkout/[businessCode]/[serviceId]/@calendar/(...)[lang]/thirdtestpath **not work** (always hard navigate to thirdtestpath)

@khuezy
Copy link
Contributor Author

khuezy commented May 9, 2023

@feedthejim Can you take a look at this ticket again please. Or should I open another ticket? As others have mentioned, dynamic routes don't seem to be interception. Here's a new repro: https://codesandbox.io/p/sandbox/youthful-butterfly-l44fsy?file=%2Fapp%2F%28main%29%2Fstatic%2Fpage.tsx%3A6%2C57

The photos app has dynamic routes of [author], which doesn't appear to be intercepted at all.
If you go to the /static, the interception works.

Thanks!
NOTE: I'm using the latest 13.4.2-canary.3

If I click on the "Go to /photos/[author]/[id]]":
Photos/(.)john/123 Page. You should only see me on page reload

If I refresh the /photos/john/123 page, it prints:
Photos/john/123 Page. You should only see me on page reload

Notice the missing "(.)"

@Nithur-M
Copy link

Hello @khuezy did you able to find a solution for this?

@khuezy
Copy link
Contributor Author

khuezy commented May 15, 2023

@Nithur-M I think one of the work around is to go to the root: (...)someroute.
I opened ticket: #49614

@Nithur-M
Copy link

@Nithur-M I think one of the work around is to go to the root: (...)someroute. I opened ticket: #49614

Thanks for this. Facing the same issue now.

@github-actions
Copy link
Contributor

This closed issue has been automatically locked because it had no new activity for a month. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue was opened via the bug report template.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants