Vite crash when switching between two significantly different branches - RangeError: Set maximum size exceeded #12062
Comments
|
So weird that we exceed |
|
actually yeah, @sapphi-red isn't this a bug? vite/packages/vite/src/node/server/hmr.ts Lines 144 to 147 in 167753d vite/packages/vite/src/node/server/hmr.ts Lines 246 to 249 in 167753d Set are not smart enough to deduplicate objects based on their property, for this to work as intended, the entries we want deduplicated would have to reference the same variable |
|
By digging down the history, it seems this I think it's a bug. It needs to be something like: const boundaries = new Map<ModuleNode, Set<ModuleNode>>()
const s = boundaries.get(boundary) ?? new Set()
s.add(acceptedVia)
boundaries.set(boundary, s) |
sapphi-red
added
contribution welcome
p3-minor-bug
|
Hello @fc. We like your proposal/feedback and would appreciate a contribution via a Pull Request by you or another community member. We thank you in advance for your contribution and are looking forward to reviewing it! |
|
removed |
Hi @fc I made a pr #12125 that uses a hash to compare duplicated modules. Let me know if that fixes it |
|
I made a mistake in my earlier testing so you can ignore my comment above yours. Then I went through the steps again and enabled Looks like I need to drop in some console.log statements to see where it is getting stuck.. Maybe this is related to the issue you reported -- #12087 |
|
Yeah I have this feeling it may be a cascade of errors for us too. Fixing one reveals another 🥲 |
|
I put in some log statements to see what files it is trying to run at. At a glance, it looks like Using your branch, in my logs it is basically hitting this call over-and-over again: vite/packages/vite/src/node/server/hmr.ts Line 313 in 167753d I re-read through the stack trace and I was actually missing in my stack trace walkthrough/explanation that it was calling that line above for 20-ish times before it did the final call to |
|
You may be on to something @fc there's the block of code above that is supposed to return early in the event of the circular dependency: vite/packages/vite/src/node/server/hmr.ts Lines 308 to 311 in 167753d But again, like for sets, this won't work if the module node is not the same reference. Maybe it needs to be: if (currentChain.some((node: ModuleNode) => node.id === importer.id)) {
// circular deps is considered dead end
return true
} |
|
Assuming it is a bug in |
@fc okay I just pushed some updates to my branch implementing what I proposed earlier. Let's see if that helps |
|
@benatshippabo I did a test but it still looks like some kind of infinite loop. I re-added my logs and it just showed that it was still calling this line a lot. I'm going to see if I can figure exactly what args/params are being initially sent to Edit: Sweet, it's possible to set breakpoints in vite with |
|
@fc you are right, the test isn't comprehensive enough. Could you also share the parameters being passed to |
|
@benatshippabo I've been looking at this tonight. The data is just too massive and extremely nested when I look at it in the debugger. I'm not super sure how I can even copy the data out. If there are 1000s of importers and then one of those importers has 1000s of importers it will then call |
|
I will add that |
Fixes #727 as best we can w/o requiring major changes. HMR works best w/ functional components and that's too big of a change to switch everything to functional components just for HMR. Added an eslint rule which will warn about things that will almost certainly invalidate HMR. Fixed the warnings it emitted. Tested locally by changing some displayed text values in some panels and seeing if the page triggered a full reload. I didn't have any specific files/cases that triggered full reloads previously and it seems Vite 4 has made it better on its own. If we start running into cases. Saving `GridRenderer` doesn't trigger a full page reload (didn't before either), but massively slows the page (also had this behavior prior to this change). The change eventually propagates and refreshes We should keep an eye on vitejs/vite#12062 which will likely also fix the slow HMR issues on some components. There seems to be duplication of modules in the update list and it can explode at times (like GridRenderer triggers 14 unique modules, but 20k updates consisting of just those 14) BREAKING CHANGE: Renamed `renderFileListItem` to `FileListItem`. Renamed `RenderFileListItemProps` to `FileListItemProps`. Removed exports for `ConsolePlugin.assertIsConsolePluginProps`, `GridPlugin.SUPPORTED_TYPES`, `FileList.getPathFromItem`, `FileList.DRAG_HOVER_TIMEOUT`, `FileList.getItemIcon`, `Grid.directionForKey`, `GotoRow.isIrisGridProxyModel`, and `Aggregations.SELECTABLE_OPTIONS`. These were all only being consumed within their own file and are not consumed in enterprise
|
This clearly need a repro showing how to get Vite graph module getting duplicated modules (or a least showing than updates is called more times than file change events) |
|
I can provide our repo with the issue if you don't mind a non-minimal repro I placed some breakpoints in the HMR code and here's some observations
|
|
If you can provide a repo where git clone + (p)npm i + name of the file to edit to trigger the behaviour I can look at it. |
|
I made a branch that doesn't require any services to run. Here is the link https://github.com/mattrunyon/web-client-ui/tree/vite-hmr-demo
Then navigate to You can save |
|
One other thing I noticed is it might be related to index files which re-export many modules in a monorepo. So in my repo above, Other cases which I thought may trigger it would be saving something like |
|
Oh yeah barrels exports is really bad for Vite HMR model. |
|
Noted. The barrel exports are useful for keeping our imports non-breaking since it can be consumed as We have the eslint rule recommended by the react-swc plugin and AFAIK all of our files are valid under that rule. So usually we don't run into any invalidations. Just another point I noticed is that class components are not self accepting but functional components are. I know functional components are better for HMR, but class components seem to work ok (even if they're just remounted and state is discarded). In our case we don't really have the resources to convert our class components to functional components and are fine with losing the state if the component is remounted. In the |
|
You could setup something to have Even without the state update, the file is a boundary so that's keep HMR quick and I understand that keeping state is not worth the rewrite. Ok this makes sense. I'm checking it now but if that's that I can fix that |
|
Thanks. The propagateUpdates function is very quick on the server either way. It's the update list containing thousands of duplicates that makes the client slow. So the client plugin assumes that every item in the list needs to be updated. But fixing propagateUpdates recrawling nodes should probably fix both the HMR issue and the issue on this ticket of overflowing the call stack with a bunch of changes. If that is indeed the case |
|
would this plugin help with the barrel export issue? |
|
This sounds a lot of additional tooling just to avoid writing explicit import in the first place. And most people often combine barrels export with |
|
Closing this issue as I guess it's fixed. There's two reports that confirmed #12658 fixed their issues (#12658 (comment), #12087 (comment)). |
Describe the bug
In our case, by running vite in local dev and switching between our release branches with
git checkoutthen we see the shell/vite crash and the browser die. It seems like HMR is not able to keep up with so many files changing so quickly.Examination of the stack trace
It seems like since so many files change at the same time, it triggers this event handler many times:
vite/packages/vite/src/node/server/index.ts
Lines 506 to 511 in 8c87af7
The event handler is calling
handleFileAddUnlink:vite/packages/vite/src/node/server/hmr.ts
Line 195 in 167753d
Which subsequently calls
updateModules:vite/packages/vite/src/node/server/hmr.ts
Line 204 in 167753d
Calls
propagateUpdateat this line over-and-over again (10+ times according to stack trace):vite/packages/vite/src/node/server/hmr.ts
Line 313 in 167753d
Then calls
propagateUpdateone more time before hitting theRangeError:vite/packages/vite/src/node/server/hmr.ts
Line 148 in 167753d
Finally, the exception/crash is triggered at this point:
vite/packages/vite/src/node/server/hmr.ts
Line 246 in 167753d
Reproduction
not yet...
Steps to reproduce
System Info
Used Package Manager
yarn
Logs
I think the browser crash is possibly due to how HMR tries to do a full reload of the page but because the vite server crashed it shows Chrome as unavailable:
Possibly related stack trace that doesn't cause total crash
The stack trace below can happen but in an overlay and appears to be more recoverable.
When a lot of files change but not as many as when switching between our release branches, then it doesn't lead to a complete vite crash but it has this stack trace that displays in the vite overlay. I suspect that the stack trace below and the one above have a related cause (i.e., lots of files change):
Validations
The text was updated successfully, but these errors were encountered: