-
Notifications
You must be signed in to change notification settings - Fork 31
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
Improve HMR with Vite #727
Comments
mattrunyon
added
enhancement
New feature or request
triage
Issue requires triage
labels
Aug 23, 2022
https://github.com/ArnaudBarre/eslint-plugin-react-refresh This might be a useful plugin for eslint to help spot some potential issues |
Upgrading to Vite 4 provides a message in your console telling you when certain files cannot fast refresh. We should try to at least fix or keep a list of the known files. The dashboard core plugins seem to be invalid for fast refresh. |
mattrunyon
added a commit
that referenced
this issue
Mar 17, 2023
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Switching from CRA to Vite also moves us off of Webpack. Webpack possibly does some heuristics to make HMR function in different cases where Vite does not HMR properly.
Vite does not HMR if there are items it cannot determine are side-effect free. Some items like shortcuts possibly break this with the current singleton implementation and should use a class instead.
Also, HOC may break fast refresh. I am not sure if our current redux setup breaks this since it uses HOC.
Any imports such as
import 'myScript.js'
orimport 'myStyle.css'
will also be considered as side-effects most likely since the only reason an import like that exists is to execute something in the file. Not sure if sass triggers this w/ Vite, but it wouldn't surprise meOur log package could be an issue b/c in all the components we usually have a setup log line in the top level of the file which might be marked as a side-effect
vitejs/vite#4577 (comment)
vitejs/vite-plugin-react#21
The text was updated successfully, but these errors were encountered: