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

HMR causes modules to be re-evaluated due to circular imports #2314

Closed
2 tasks done
nowylie opened this issue Feb 28, 2021 · 5 comments
Closed
2 tasks done

HMR causes modules to be re-evaluated due to circular imports #2314

nowylie opened this issue Feb 28, 2021 · 5 comments

Comments

@nowylie
Copy link

nowylie commented Feb 28, 2021

⚠️ IMPORTANT ⚠️ Please do not ignore this template. If you do, your issue will be closed immediately.

Describe the bug

In Vite 1, when a module is updated only that module is re-evaluated in the browser.

In Vite 2, when a module is updated all modules that depend on the updated module are invalidated and have their import path updated, causing the browser to re-evaluate the modules if they're in the dependency tree of the updated module (due to circular dependencies). Marking modules with import.meta.hot.decline() doesn't appear to make any difference.

Specifically the issue is at

mod.importers.forEach((importer) => invalidate(importer, timestamp, seen))

It would be nice if this behaviour was configurable.

Reproduction

https://github.com/nowylie/vite-2-repro

npm ci
npm run vite
  1. Open the repro page in the browser
  2. Open the JS console in the dev tools
  3. Save src/dep.js to trigger a HMR update
  4. Note that main.js is re-evaluated

System Info

  • vite version: 2.0.4
  • Operating System: macOS 11.2
  • Node version: 14.16.0
  • Package manager (npm/yarn/pnpm) and version: npm 6.14.11
@yyx990803
Copy link
Member

yyx990803 commented Mar 2, 2021

This is expected behavior. If there are circular imports, the only way to ensure correctness is to invalidate all modules in the chain. If you want to avoid it, consider refactor your code to avoid circular imports.

@nowylie
Copy link
Author

nowylie commented Mar 3, 2021

Thanks @yyx990803 for taking the time to look at this issue! I can understand that this is expected behaviour but had hoped it might be possible for there to be a configuration option to disable it.

For a bit more context, I used Vite 1 to develop some basic HMR support for Overture.js which we use to develop the front-end for Fastmail.

Overture makes reasonably heavy usage of Classes and Inheritance. This makes supporting HMR a bit difficult since updated modules export new class definitions and break instanceof checks. I worked around this by patching the prototype of the original class definition when new modules are evaluated. It's not perfect but it works in the general case. The other solution might be to proxy module exports but that would be a fair bit more complicated to implement.

Within our app, we have a central router module that handles routing and dynamically loading other parts of the app. The router module also exports some global state that other modules reference. This is where our circular dependencies come from. I think this is a sound design.

However this doesn't work well with Vite's new behaviour. Since parent modules are invalidated, an update in one of our View modules causes the router module to be invalidated and from there a large portion of the app is invalidated (since the router module is so central).

Is there any chance that this behaviour could be made configurable so that we can continue to use Vite?

@anncwb
Copy link
Contributor

anncwb commented Mar 8, 2021

@nowylie
Do you have a solution?
The same problem, circular dependencies are easy to appear after the project is complicated. And there is no way to investigate, because the dependence may be very deep.

@nowylie
Copy link
Author

nowylie commented Mar 8, 2021

@anncwb no solution yet.

As I mentioned, our circular dependencies are by design.

For accidental/unnecessary circular dependencies, I've had success in the past using madge to track them down.

@anncwb
Copy link
Contributor

anncwb commented Mar 11, 2021

@anncwb no solution yet.

As I mentioned, our circular dependencies are by design.

For accidental/unnecessary circular dependencies, I've had success in the past using madge to track them down.

I reopened an issue, wait to see the author's reply

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants