[base-ui] useControllableReducer warns when controlled props become uncontrolled (and vice versa) #39096
Conversation
Netlify deploy previewhttps://deploy-preview-39096--material-ui.netlify.app/ Bundle size reportDetails of bundle changes (Toolpad) |
mj12albert
force-pushed
the
use-controllable-reducer-warning
branch
from
35f31d6 to
4d684e4
Compare
|
This is just a naive implementation, but useControllableReducer should have a warning like this similar to useControlled right? @michaldudak I noticed it could be a prerequisite for #38972 CC @DiegoAndai |
| (controlledProps as Record<string, unknown>)[key] === undefined | ||
| ) { | ||
| console.error( | ||
| 'MUI: useControllableReducer is changing a controlled prop to be uncontrolled', |
There was a problem hiding this comment.
Should we add the prop name here?
There was a problem hiding this comment.
Yeah 👍 I will also pass in the component name too, as useControllableReducer is likely to be unhelpful for the user that needs to see the error ~
Why is this? 🤔 |
The v5 Select uses (Though this is a just a feature for dev mode so it's not necessarily a huge blocker ~) |
mj12albert
force-pushed
the
use-controllable-reducer-warning
branch
3 times, most recently
from
e10408c to
8e6949f
Compare
|
Getting some browser test errors after trying to pass variables to the error message 🧐 |
mj12albert
changed the title
github-actions
bot
added
the
PR: out-of-date
mj12albert
force-pushed
the
use-controllable-reducer-warning
branch
from
8e6949f to
bb1e0b8
Compare
github-actions
bot
removed
the
PR: out-of-date
mj12albert
force-pushed
the
use-controllable-reducer-warning
branch
3 times, most recently
from
4babb7c to
c4221ed
Compare
mj12albert
force-pushed
the
use-controllable-reducer-warning
branch
from
c4221ed to
ab90257
Compare
|
Finally got it to work by leaving it for a while, then the mysterious failing test just stopped failing 😓 Open to suggestions for a more ergonomic way to pass the |
Totally agree it's not the best name but there was kind of already a precedent 🙈 https://github.com/mui/material-ui/blob/master/packages/mui-base/src/useAutocomplete/useAutocomplete.d.ts#L98C2-L101 😅 @DiegoAndai |
There was a problem hiding this comment.
@mj12albert lets go with componentName then 😅
Left a question
|
CC @michaldudak to have a look and see if I missed anything 🙏 |
|
I've added more commits to fix other tests that were failing |
|
I will merge this first as Michal is off for a while, and so I don't have to keep fixing tests 😅 |
…ncontrolled (and vice versa) (mui#39096)
…ncontrolled (and vice versa) (mui#39096)
…ncontrolled (and vice versa) (mui#39096)
From #36119 (comment), #38723 (comment), could be a prerequisite for #38972
Components (or hooks) that use
useControllableReduceraccept controlled props, which should remain controlled (or uncontrolled) for its lifetime, but currently does not throw an error when it happens like inuseControlledThis PR adds an equivalent warning by "watching" all the keys in
controlledProps