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

Hot Reload Full Refresh in v13.4.8 #53614

Closed
1 task done
frankfka opened this issue Aug 5, 2023 · 8 comments · Fixed by #54690
Closed
1 task done

Hot Reload Full Refresh in v13.4.8 #53614

frankfka opened this issue Aug 5, 2023 · 8 comments · Fixed by #54690
Assignees
Labels
bug Issue was opened via the bug report template. linear: next Confirmed issue that is tracked by the Next.js team. locked

Comments

@frankfka
Copy link

frankfka commented Aug 5, 2023

Verify canary release

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

Provide environment information

Next.js v13.4.8 with pages directory routing & Tailwind:

Operating System:
      Platform: darwin
      Arch: x64
      Version: Darwin Kernel Version 22.5.0: Thu Jun  8 22:22:22 PDT 2023; root:xnu-8796.121.3~7/RELEASE_X86_64
    Binaries:
      Node: 16.18.1
      npm: 9.3.1
      Yarn: 1.22.19
      pnpm: 7.5.0
    Relevant Packages:
      next: 13.4.8
      eslint-config-next: 13.4.8
      react: 18.2.0
      react-dom: 18.2.0
      typescript: 5.1.6
    Next.js Config:
      output: N/A

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

No response

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

https://github.com/frankfka/nextjs-full-refresh-bug

To Reproduce

See README in https://github.com/frankfka/nextjs-full-refresh-bug

Describe the Bug

We noticed in our production app that an upgrade from v13.4.7 -> v13.4.8 breaks hot reload in our dialog components (we use Radix Dialogs). Hot reload still works in normal pages, but will trigger a full refresh if a dialog component is edited.

I see a number of updates to hot reload in 13.4.8, perhaps one of them could have triggered this change?

Expected Behavior

Hot reload works when editing dialog components

Which browser are you using? (if relevant)

No response

How are you deploying your application? (if relevant)

No response

NEXT-1568

@frankfka frankfka added the bug Issue was opened via the bug report template. label Aug 5, 2023
@jamesedmonston
Copy link

I'm seeing this too. Editing my header and footer components which are children of _app.tsx causes the entire page to reload.

@Tadimsky
Copy link

Tadimsky commented Aug 17, 2023

I also went version by version and found out that it was definitely introduced in this version.
The hash of _document.js changes and that's triggering the reload.

I debugged the hot-reloader server and this is where we're telling the client to reload the page:
https://github.com/vercel/next.js/blame/93259caac47a820422bb3a37198c115c37613164/packages/next/src/server/dev/hot-reloader.ts#L1223

This is not new, but the client only recently started listening to that message and reloading the page.
That was introduced with this PR (which was part of the 13.4.8 version):
https://github.com/vercel/next.js/pull/51516/files#diff-2b9bd2ade12f637cc8192719277f9a2238c996e2d8ef88f024b773866d32956bR67

@huozhi is this something you could maybe take a look at?

@pronovaso
Copy link

From version of 13.4.8 I have this behaviour too and is it a little anoing. If I make some changes on dialog componet and in some tabs, I must go all again.

@huozhi huozhi added the please add a complete reproduction The issue lacks information for further investigation label Aug 21, 2023
@github-actions
Copy link
Contributor

We cannot recreate the issue with the provided information. Please add a reproduction in order for us to be able to investigate.

Why was this issue marked with the please add a complete reproduction label?

To be able to investigate, we need access to a reproduction to identify what triggered the issue. We prefer a link to a public GitHub repository (template for App Router, template for Pages Router), but you can also use these templates: CodeSandbox: App Router or CodeSandbox: Pages Router.

To make sure the issue is resolved as quickly as possible, please make sure that the reproduction is as minimal as possible. This means that you should remove unnecessary code, files, and dependencies that do not contribute to the issue. Ensure your reproduction does not depend on secrets, 3rd party registries, private dependencies, or any other data that cannot be made public. Avoid a reproduction including a whole monorepo (unless relevant to the issue). The easier it is to reproduce the issue, the quicker we can help.

Please test your reproduction against the latest version of Next.js (next@canary) to make sure your issue has not already been fixed.

If you cannot create a clean reproduction, another way you can help the maintainers' job is to pinpoint the canary version of next that introduced the issue. Check out our releases, and try to find the first canary release that introduced the issue. This will help us narrow down the scope of the issue, and possibly point to the PR/code change that introduced it. You can install a specific version of next by running npm install next@<version>.

I added a link, why was it still marked?

Ensure the link is pointing to a codebase that is accessible (e.g. not a private repository). "example.com", "n/a", "will add later", etc. are not acceptable links -- we need to see a public codebase. See the above section for accepted links.

What happens if I don't provide a sufficient minimal reproduction?

Issues with the please add a complete reproduction label that receives no meaningful activity (e.g. new comments with a reproduction link) are automatically closed and locked after 30 days.

If your issue has not been resolved in that time and it has been closed/locked, please open a new issue with the required reproduction.

I did not open this issue, but it is relevant to me, what can I do to help?

Anyone experiencing the same issue is welcome to provide a minimal reproduction following the above steps. Furthermore, you can upvote the issue using the 👍 reaction on the topmost comment (please do not comment "I have the same issue" without reproduction steps). Then, we can sort issues by votes to prioritize.

I think my reproduction is good enough, why aren't you looking into it quicker?

We look into every Next.js issue and constantly monitor open issues for new comments.

However, sometimes we might miss one or two due to the popularity/high traffic of the repository. We apologize, and kindly ask you to refrain from tagging core maintainers, as that will usually not result in increased priority.

Upvoting issues to show your interest will help us prioritize and address them as quickly as possible. That said, every issue is important to us, and if an issue gets closed by accident, we encourage you to open a new one linking to the old issue and we will look into it.

Useful Resources

@frankfka
Copy link
Author

@huozhi I added a repo link with a reproduction example

@huozhi huozhi removed the please add a complete reproduction The issue lacks information for further investigation label Aug 21, 2023
@jamesedmonston
Copy link

@huozhi I'm happy to supply a repo privately if it's helpful.

@nathandallaire
Copy link

Hey all, I noticed this several weeks ago and thought I was losing my mind.

I found this by creating a new next app: npx create-next-app@latest (disable app router for this - I didn't test on that).

If you edit anything within _app.js you'll see the page run a full refresh. I even noticed this on components created and imported into _app.js - as long as it is rendered within the hierarchy of _app.js, it will run a full refresh.

That being said, if there is content rendered through something that is not _app.js (ie. through pages/index.js), then it will hot reload as usual. I can confirm I triple checked all the components are named (not anonymous) and their corresponding files are pascal cased.

@huozhi huozhi added linear: next Confirmed issue that is tracked by the Next.js team. and removed type: needs triage labels Aug 28, 2023
@huozhi huozhi self-assigned this Aug 28, 2023
timneutkens pushed a commit that referenced this issue Aug 30, 2023
### What
When you only have `pages/` folder and custom `_app.js`, editing the
`_app.js` file when unexpectedly lead to full page reload. This is
because the `pages/_app` chunk is included into `pages/_document` chunk
as they all pass through the `next-route-loader` which forms the router
module and other userland modules including `/_app` and `/_document`.
That leads to a change: whenever you modify `_app.js`, chunk hash of
`/_document` will always be updated as it's built as a new chunk.

Found this difference by looking at the document chunk with nextjs in
early 13.4.x releases.

### How

We'll check at the entries creation point to filter out `/_app` and
`/_document` so that we won't have them both bundled for `_document`
chunk. Then when we won't trigger the hmr path to reload the page since
_document chunk hash is updated.

Closes NEXT-1568
Fixes #53614
@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 Sep 13, 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. 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.

6 participants