jquense / uncontrollable

Wrap a controlled react component, to allow specific prop/handler pairs to be uncontrolled
MIT License
199 stars 33 forks source link

Reset prop value if defaultValue changes #14

Closed awseward closed 7 years ago

awseward commented 8 years ago

Proposed Change

If the value being passed into the default${propName} prop changes, then the actual value should change to the newly passed in value.

Why?

One example of a problem this solves comes up when trying to use the defaultValue prop of two comboboxes, and the selection in one affects the data prop in the other. I've provided a brief illustration to clarify.

Current Behavior

When I select a new option in the left Combobox, the right Combobox gets a new data prop, and its defaultValue prop is reset to undefined. This is done because the data populating the right Combobox's options is dependent on the selected item in the left Combobox.

Unfortunately, as the gif shows, Uncontrollable currently prevents this change from getting to the actual Combobox, so it doesn't render as we'd like it to. dependent-comboboxes-noreset

New Behavior

With this change applied, the Combobox re-renders as we expect when the value passed into its defaultValue is changed. dependent-comboboxes-reset

Concerns

We haven't seen anything break in our app since this change went live in our fork back in late January, but it's still probably worth considering whether this might have otherwise breaking changes for other use cases.

jquense commented 8 years ago

Hi! thanks for taking the time to work on this, and for contributing. I see the appeal for this sort of behavior, Ultimately though such a change, i think defeats the purpose to defaultValue in the first place. The contract with controlled/uncontrolled inputs is "you control the state" or "I control the state". This change places that somewhere in the middle instead, which while convenient at times is gonna add more complications than its worth (imo). I do appreciate that this proposed change is convenient, and I think for your app(s) it may be a perfectly valid approach. That being said, uncontrollable is a workhorse module that underpins many diverse components, which makes it preferable for it to be a straightforward and simple in its opinions. The behavior already diverges from the react inputs (which don't allow switching between modes) and that causes tension, this would make that even more divergent.

For what its worth, the "correct" way to handle your example is to not to use uncontrolled inputs, instead controlling value directly. If you really don't want to wire up the controls that way, setting a key on the second combobox to value of the first will effectively give you this behavior by forcing a remount of the second combobox.

final PS, if you do want to maintain this behavior, I think you can get a way with using createUncontrollable directly instead of maintaining a fork.