-
Notifications
You must be signed in to change notification settings - Fork 60
Hot Refresh Support #1148
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
Hot Refresh Support #1148
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Love that this persistent issue is finally being fixed, we left it here for far to long imo.
Two small things:
- We use pascal case for files so
useModalManager.tsx
should beUseModalManager.tsx
. - Small merge conflicts with
dev
.
219ff55
to
8e856d4
Compare
Noted! Updated the file nomenclature. I tried to merge the conflicts but the new Jolt module seems to be giving me problems. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noted! Updated the file nomenclature. I tried to merge the conflicts but the new Jolt module seems to be giving me problems.
Make sure to run npm install
again (or whatever package manager you use). If your still having issues let me know specifics and I can help resolve them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like there is still two exports within ThemeContext.tsx
.
And here:
Thanks for the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the
useTheme
catch.ThemeProvider
, however, should stay because it's the React component. I realized that theuseModelControlContext
andusePanelControlContext
hooks were both still inModelContext
andPanelContext
and did some further component isolation.
Seems reasonable, just got to make sure the workflows pass and we can get this in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Worked perfectly, I was able to edit the main hud and have it hot reload without any errors or unintended behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amazing
Refactors
ThemeContext
,ModalContext
, andPanelContext
to follow React Fast Refresh guidelines by exporting only components/hooks. Moves helpers to separate.ts
files to avoid full reloads on UI changes. More info in docs.