-
-
Notifications
You must be signed in to change notification settings - Fork 840
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
Fix usePartFields hook #7868
Fix usePartFields hook #7868
Conversation
✅ Deploy Preview for inventree-web-pui-preview ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
@@ -13,6 +13,8 @@ export function usePartFields({ | |||
}: { | |||
create?: boolean; | |||
}): ApiFormFieldSet { | |||
const settings = useGlobalSettingsState.getState(); |
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.
Are you sure, that this works?
What was your initial issue, that you tried to solve with this? That the useMemo was not retriggered when the global state updated?
Isn't the Zustand syntax something like this in a reactive way then:
const settings = useGlobalSettingsState.getState(); | |
const settings = useGlobalSettingsState(); |
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.
The initial issue was that the call to settings was buried inside the lower useMemo
call which was only updated when create
changed. So, AFAIK, settings
value was never being re-evaluated
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.
No, it should be reevaluated, on the initial render of the component and if create would have changed.
Using .getState is used to get the state outside of the component. E.g. in a hook. Ref: https://github.com/pmndrs/zustand?tab=readme-ov-file#readingwriting-state-and-reacting-to-changes-outside-of-components
If you want to subscribe to the state and bind it to a react state, just call the store, the store is a hook. But I'm not sure if this whole change is really necessary.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #7868 +/- ##
==========================================
- Coverage 83.48% 83.36% -0.12%
==========================================
Files 1128 1128
Lines 50264 50264
Branches 1733 1733
==========================================
- Hits 41962 41904 -58
- Misses 7860 7929 +69
+ Partials 442 431 -11
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
No description provided.