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

Fix HMR issue after patching the client module #43819

Merged
merged 2 commits into from Dec 7, 2022
Merged

Fix HMR issue after patching the client module #43819

merged 2 commits into from Dec 7, 2022

Conversation

shuding
Copy link
Member

@shuding shuding commented Dec 7, 2022

Follow-up to #43778, this PR implements the logic of adding a timestamp query to all client module reference's IDs. Note that this only applies to flight renders during dev, which only affects HMR.

The reason of changing the module ID for hot reloading is, after changing a client component to a server component, the module is completely removed in the client chunk. And Webpack disposes it along with all the React refresh registered callbacks. And when changing it back to a server component, a new module instance is generated and loaded by Webpack, but the Flight client still holds the reference to the old module instance. This is basically a way to force flight to reload the modules from Webpack.

NEXT-30

Bug

  • Related issues linked using fixes #number
  • Integration tests added
  • Errors have a helpful link attached, see contributing.md

Feature

  • Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
  • Related issues linked using fixes #number
  • e2e tests added
  • Documentation added
  • Telemetry added. In case of a feature if it's used or not.
  • Errors have a helpful link attached, see contributing.md

Documentation / Examples

  • Make sure the linting passes by running pnpm build && pnpm lint
  • The "examples guidelines" are followed from our contributing doc

timneutkens
timneutkens previously approved these changes Dec 7, 2022
@timneutkens timneutkens merged commit 476bb09 into canary Dec 7, 2022
@timneutkens timneutkens deleted the shu/g51w branch December 7, 2022 16:34
kodiakhq bot pushed a commit that referenced this pull request Dec 8, 2022
… development (#43823)

As @timneutkens pointed out [here](#43819 (comment)), changing the key will make Flight's module cache grow indefinitely. While I've thought about updating Flight's logic to clean the cache at some point, but that's tricky to do correctly as everything is asynchronous and we have to trigger clean up from outside (Webpack).

So currently, a better way I can think of is to just give Flight a proxied object as `moduleExports`. Even if Flight is getting the same object from cache, it will always require the latest module from Webpack.

## Bug

- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Errors have a helpful link attached, see [`contributing.md`](https://github.com/vercel/next.js/blob/canary/contributing.md)

## Feature

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

## Documentation / Examples

- [ ] Make sure the linting passes by running `pnpm build && pnpm lint`
- [ ] The "examples guidelines" are followed from [our contributing doc](https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md)
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants