material-ui
material-ui copied to clipboard
[material-ui][Badge] Fix stale values being used when badge is invisible
Fixes: https://github.com/mui/material-ui/issues/43081
To see the error:
- Go to the before/after demo
- Inspect the
.MuiBadge-badgeelement - Change the value back and forth between 8 and 0
- Observe the
.MuiBadge-badgeelement's value
Before: https://stackblitz.com/edit/react-qkjzbj?file=Demo.tsx After: Waiting for CI build
I think the use of usePreviousProps is incorrect, as badgeContent, max and displayValue shouldn't be coupled with the invisible value. They should always be updated regardless. It was originally added for another use case: https://github.com/mnajdova/material-ui/commit/7572c905d57a5e44630dc1980ea1395424dd0947#diff-26877d58ee4ebaa271700aae39d84f0ce250e8d01c677024bbf0d53f46159b1b, so it might've stuck around inadvertently.
I fixed in both mui-material and mui-base for consistency.
Netlify deploy preview
https://deploy-preview-43083--material-ui.netlify.app/
Bundle size report
Details of bundle changes (Toolpad) Details of bundle changes
Generated by :no_entry_sign: dangerJS against 6a98b91d2549b1480704237608105f7489cc59bd
I think the use of usePreviousProps is incorrect, as badgeContent, max and displayValue shouldn't be coupled with the invisible value. They should always be updated regardless. It was originally added for another use case: https://github.com/mnajdova/material-ui/commit/7572c905d57a5e44630dc1980ea1395424dd0947#diff-26877d58ee4ebaa271700aae39d84f0ce250e8d01c677024bbf0d53f46159b1b, so it might've stuck around inadvertently.
I would recommend when going trough history to search for the PR that did the fix, as it is usually how you can understand the motivation for the code change, e.g. https://github.com/mui/material-ui/pull/21557 has a description & explanation. Also, changing a test should be an indication that something will be broken/behavior will change.