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-1337] Parallel routes keep refreshing in DEV in next 13.4.7 #51951

Closed
1 task done
Fredkiss3 opened this issue Jun 28, 2023 · 12 comments · Fixed by #52061
Closed
1 task done

[NEXT-1337] Parallel routes keep refreshing in DEV in next 13.4.7 #51951

Fredkiss3 opened this issue Jun 28, 2023 · 12 comments · Fixed by #52061
Labels
area: app App directory (appDir: true) bug Issue was opened via the bug report template. linear: next Confirmed issue that is tracked by the Next.js team. locked

Comments

@Fredkiss3
Copy link
Contributor

Fredkiss3 commented Jun 28, 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.5.0: Thu Jun  8 22:22:19 PDT 2023; root:xnu-8796.121.3~7/RELEASE_ARM64_T8103
    Binaries:
      Node: 18.16.0
      npm: 9.5.1
      Yarn: 1.22.19
      pnpm: 8.6.2
    Relevant Packages:
      next: 13.4.8-canary.8
      eslint-config-next: N/A
      react: 18.2.0
      react-dom: 18.2.0
      typescript: 4.9.4
    Next.js Config:
      output: N/A

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

App directory (appDir: true)

Link to the code that reproduces this issue or a replay of the bug

https://github.com/Fredkiss3/infinite-reload-parallel-routes-bug

To Reproduce

  1. Run the DEV server and go to http://localhost:3000
  2. Watch as the color of the text keep changing every couple of seconds

Describe the Bug

In the latest version of next (13.4.7) and even on the latest canary, when we use parallel routes, they get refreshed every couple of seconds without doing any action in DEV, i've noticed that everytime the websocket used by next for HMR does does a ping, next refresh the page and the server code is reexecuted.

I've tested with the previous version (13.4.6) and it worked fine.

Expected Behavior

The parallel route should not refresh every couple of seconds.

Which browser are you using? (if relevant)

No response

How are you deploying your application? (if relevant)

No response

NEXT-1337

@Fredkiss3 Fredkiss3 added the bug Issue was opened via the bug report template. label Jun 28, 2023
@github-actions github-actions bot added the area: app App directory (appDir: true) label Jun 28, 2023
@nicksan222
Copy link

As described in the documentation

Parallel routes are created using named slots. Slots are defined with the @folder convention, and are passed to the same-level layout as props.
Slots are not route segments and do not affect the URL structure. The file path /@team/members would be accessible at /members.

Maybe you should try to move the layout to the folder level as the @parallel

Might be wrong let me know if that works since i never used parallels directly

@m10rten
Copy link

m10rten commented Jun 30, 2023

Moving the layout won't fix it, perhaps making some "use client";
And I have no idea what that randomColor thing does, but try removing that?

@Fredkiss3
Copy link
Contributor Author

Fredkiss3 commented Jun 30, 2023

@m10rten randomColor does not do anything particular, it is just a package that generates a random rgb color, it is there for demo purposes to see the bug more clearly.

The bug is that the page will refresh (not reload as in the browser reload, but more like a refresh happening in the cases of an HMR update) without the developper doing anything, so to reproduce you just have to wait and you will see the color of the text changing every couple of seconds. While normally it shouldn't and only refresh if the user reload the browser's tab or change the code to cause an HMR refresh.

@nicksan222
Copy link

So i fixed the issue.

The things i did were:

  • Created a page.tsx file on app folder
// page.tsx

export default function Page() {
    return <div></div>
}
  • Modified the layout.tsx like this
import * as React from "react";

export default function RootLayout({
  children,
  parallel
}: {
  children: React.ReactNode,
  parallel: React.ReactNode;
}) {
  return (
    <html>
      <head />
      <body>
        <h2>ROOT LAYOUT</h2>
        {parallel}
      </body>
    </html>
  );
}

I guess children must always be the first argument, kind of a reserved keywork

@m10rten
Copy link

m10rten commented Jun 30, 2023

not having a root page should not have been the issue though.

@Fredkiss3
Copy link
Contributor Author

Fredkiss3 commented Jun 30, 2023

Thank you @nicksan222 , i've tested and the fix is to simply add a page.tsx at the same level as the layout, no need to add children to the layout.tsx file :

// app/page.tsx

export default function Page() {
  return null;
}

The weird thing is that this did not happen with next@13.4.6 and only with the latest version. If this is an evolution and it is needed in new versions, it should be documented in my opinion.

However i followed the guide on the docs which did not mention the need of a root page, and since it worked in the versions before, i am more inclined to say this is a bug, i may be wrong though.

@feedthejim
Copy link
Contributor

will take a look on Monday, I think I broke it in #51604

@feedthejim feedthejim added the linear: next Confirmed issue that is tracked by the Next.js team. label Jun 30, 2023
@feedthejim feedthejim changed the title Parallel routes keep refreshing in DEV in next 13.4.7 [NEXT-1337] Parallel routes keep refreshing in DEV in next 13.4.7 Jun 30, 2023
timneutkens pushed a commit that referenced this issue Jul 4, 2023
…#52061)

### What?
When there's a parallel route adjacent to a tree that has no page
component, it's treated as an invalid entry in `handleAppPing` during
dev HMR, which causes an infinite refresh cycle

### Why?
In #51413, an update was made to `next-app-loader` to support layout
files in parallel routes. Part of this change updated the parallel
segment matching logic to mark the parallel page entry as `[
'@parallel', [ 'page$' ] ]` rather than `[ '@parallel', 'page$' ]`.

This resulted in `handleAppPing` looking for the corresponding page
entry at `client@app@/@parallel/page$/page` (note the `PAGE_SEGMENT`
marker) rather than `client@app@/@parallel/page`, causing the path to be
marked invalid on HMR pings, and triggering an endless fastRefresh.

### How?
A simple patch to fix this would fix this is to update `getEntryKey` to
replace any `PAGE_SEGMENT`'s that leak into the entry which I did in
59a972f.

The other option that's currently implemented here is to only insert
PAGE_SEGMENT as an array in the scenario where there isn't a page
adjacent to the parallel segment. This is to ensure that the
`parallelKey` is `children` rather than the `@parallel` slot when in
[`createSubtreePropsFromSegmentPath`](https://github.com/vercel/next.js/blob/59a972f53339cf6e444e3bf5be45bf115a24c31a/packages/next/src/build/webpack/loaders/next-app-loader.ts#L298).
This seems to not cause any regressions with the issue being fixed in
51413, and also solves this case, but I'm just not 100% sure if this
might break another scenario that I'm not thinking of.

Closes NEXT-1337
Fixes #51951
@Fredkiss3
Copy link
Contributor Author

cc @feedthejim , this issue is still not totally fixed, there are some edge cases like here : #52061 (comment) i'm still hitting this one.

shuding pushed a commit that referenced this issue Jul 8, 2023
…#52061)

### What?
When there's a parallel route adjacent to a tree that has no page
component, it's treated as an invalid entry in `handleAppPing` during
dev HMR, which causes an infinite refresh cycle

### Why?
In #51413, an update was made to `next-app-loader` to support layout
files in parallel routes. Part of this change updated the parallel
segment matching logic to mark the parallel page entry as `[
'@parallel', [ 'page$' ] ]` rather than `[ '@parallel', 'page$' ]`.

This resulted in `handleAppPing` looking for the corresponding page
entry at `client@app@/@parallel/page$/page` (note the `PAGE_SEGMENT`
marker) rather than `client@app@/@parallel/page`, causing the path to be
marked invalid on HMR pings, and triggering an endless fastRefresh.

### How?
A simple patch to fix this would fix this is to update `getEntryKey` to
replace any `PAGE_SEGMENT`'s that leak into the entry which I did in
59a972f.

The other option that's currently implemented here is to only insert
PAGE_SEGMENT as an array in the scenario where there isn't a page
adjacent to the parallel segment. This is to ensure that the
`parallelKey` is `children` rather than the `@parallel` slot when in
[`createSubtreePropsFromSegmentPath`](https://github.com/vercel/next.js/blob/59a972f53339cf6e444e3bf5be45bf115a24c31a/packages/next/src/build/webpack/loaders/next-app-loader.ts#L298).
This seems to not cause any regressions with the issue being fixed in
51413, and also solves this case, but I'm just not 100% sure if this
might break another scenario that I'm not thinking of.

Closes NEXT-1337
Fixes #51951
@MostafaKMilly
Copy link

I still have the issue on 13.4.9
Screenshot 2023-07-09 at 6 59 14 AM

@timneutkens
Copy link
Member

@MostafaKMilly can you provide a reproduction?

@sinafath
Copy link

I still have the issue on 13.4.9 Screenshot 2023-07-09 at 6 59 14 AM

delete .next folder. It fixed for me

@github-actions
Copy link
Contributor

This closed issue has been automatically locked because it had no new activity for 2 weeks. 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 Aug 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: app App directory (appDir: true) bug Issue was opened via the bug report template. linear: next Confirmed issue that is tracked by the Next.js team. locked
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants