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 duplicated modules because of query params mismatch #2255

Closed
3 tasks done
rixo opened this issue Feb 25, 2021 · 14 comments · Fixed by #9773
Closed
3 tasks done

HMR duplicated modules because of query params mismatch #2255

rixo opened this issue Feb 25, 2021 · 14 comments · Fixed by #9773
Labels
feat: hmr p4-important Violate documented behavior or significantly improves performance (priority)

Comments

@rixo
Copy link
Contributor

rixo commented Feb 25, 2021

⚠️ IMPORTANT ⚠️ Please check the following list before proceeding. If you ignore this issue template, your issue will be directly closed.

  • Read the docs.
  • Use Vite >=2.0. (1.x is no longer supported)
  • If the issue is related to 1.x -> 2.0 upgrade, read the Migration Guide first.

我只能希望我正确地遵循了中文说明

Describe the bug

HMR sometimes reload the same version of a given module with different query parameters, resulting in duplicated copies of this module in the browser. Different importers get different runtime copies of the same module, which will cause trouble if the duplicated module is stateful.

Here, the module a.js at 1614293358773 is incorrectly duplicated:

image

I believe I also witnessed a situation in casual usage where both params were present, but not in the same order (?t=...&import and ?import&t...), resulting in a similar problem. But I don't have a reproduction for this one.

Why is this import query param needed, by the way?

Reproduction

https://github.com/rixo/esm-hmr-spec

The failing test is this one: importing-reloaded.spec.js. The failing assertion is here.

Run the failing test with:

yarn test --vite --filter reloaded

In order to open the Playwright browser and keep it open, to be able to inspect the network tab after the test has completed, run it with:

yarn test --vite --filter reloaded --open --keep

(I also recommend yarn test --vite --filter reloaded --inspect-brk, if you ever need to put a breakpoint in the Node code.)

System Info

  • vite version: 2.0.3
  • Operating System: linux-x64
  • Node version: node-v12.16.3
  • Package manager (npm/yarn/pnpm) and version: yarn 1.22.10
@yyx990803 yyx990803 added bug: upstream Bug in a dependency of Vite bug and removed pending triage bug: upstream Bug in a dependency of Vite labels Feb 26, 2021
@patak-dev patak-dev added feat: hmr p4-important Violate documented behavior or significantly improves performance (priority) and removed bug: hmr labels Mar 27, 2021
@bluwy
Copy link
Member

bluwy commented Mar 5, 2022

I've tested this with Vite 2.8.6 and can still reproduce the issue.

@Amareis
Copy link

Amareis commented Sep 9, 2022

image
I think it's same issue?..

@Amareis
Copy link

Amareis commented Sep 9, 2022

Or maybe not the same issue, but really similar - sometimes HMR somehow mess with modules, so it ends with error (in my case it's TDZ error - Cannot access SomeClass before initializing), and whole build became broken until next HMR or server restart.

@nielslanting
Copy link

I'm also getting Cannot access SomeClass before initializing when using any version of Vite above 3.1.0-beta.1 for a Vue project. It seems that #9773 breaks HMR for the Vue project, and it seems to do so because isExplicitImportRequired will return false for .vue files.

@Amareis
Copy link

Amareis commented Sep 13, 2022

That's not specific for Vue, I have just regular react project

@Amareis
Copy link

Amareis commented Sep 13, 2022

Yep, that's definitely caused by changing the file while hmr in process. Some race conditions, I think.

@Amareis
Copy link

Amareis commented Sep 13, 2022

So, basically, I get big-enough project, then:

  1. Change file (just print 1 somewhere in JSX), important: this change should invoke full page reload
  2. Ctrl+S
  3. Immediately change file again (print 2)
  4. Ctrl+S
  5. After page loading, there is duplicating JS imports with different timestamps and whole build is broken until next HMR:
    image
    That is picture after page reloading - there is two versions of my module. Since it imports stateful modules, I got my custom validation error, so this inconsistency became visible.

I think there is even falling test for it at https://github.com/vitejs/vite/blob/main/playground/ssr-vue/__tests__/ssr-vue.spec.ts#L166

@patak-dev
Copy link
Member

@Amareis would you create a new issue with a minimal reproduction against latest?

@Amareis
Copy link

Amareis commented Sep 13, 2022

@Amareis would you create a new issue with a minimal reproduction against latest?

I cannot, really, this is somehow based on big and complicated module graph I think. But reproducing just stable - change file, change file again while rebuild in progress and voila, module graph is broken - some importers imports older version and some - newer. Actual content of both versions is same, of course, it's only different timestamps in urls.

@Amareis
Copy link

Amareis commented Sep 13, 2022

I think for such cases we really needed tool to anonimize big projects -_- Just keep file structures and imports/exports. I'll try to do something similar now.

@Amareis
Copy link

Amareis commented Sep 13, 2022

Okay, there is my (anonimized) project: https://github.com/Amareis/vite-bug-example
And there is sceencast:
Screen recording 13.09.2022 20:36:15.webm

I edit file src/reviews/short-review.

As I notice, react plugin make it much easier to create such error. I think it's because of slower transpilation times - but in rare cases such error can be caused even without it.

@Amareis
Copy link

Amareis commented Sep 13, 2022

@patak-dev if you can dig it down, it will be just nice - it's pretty damn annoing error, even if it's not breaking dev flow completely.

@patak-dev
Copy link
Member

@Amareis please create a new issue with this information linking to this one, so this is properly tracked and all collaborators are aware of it. Thanks!

@Amareis
Copy link

Amareis commented Sep 14, 2022

@patak-dev done! #10118

@github-actions github-actions bot locked and limited conversation to collaborators Sep 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feat: hmr p4-important Violate documented behavior or significantly improves performance (priority)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants