Should we invoke onStateChange with _all_ the state?

[kentcdodds]

• downshift version: 1.0.0-rc.3

Relevant code or config

class App extends React.Component {
  state = {inputValue: ''}
  handleStateChange = changes => {
    if (changes.hasOwnProperty('inputValue')) {
      this.setState({inputValue: changes.inputValue})
    }
  }
  render() {
    const {inputValue} = this.state
    return (
      <Div
        css={{
          display: 'flex',
          justifyContent: 'center',
          flexDirection: 'column',
          textAlign: 'center',
        }}
      >
        <h2>basic example</h2>
        <SemanticUIDownshift
          items={items}
          inputValue={inputValue}
          onStateChange={this.handleStateChange}
          onChange={() => {}}
          itemToString={i => (i ? i.name : '')}
          className={css({width: 250, margin: 'auto'})}
        />
      </Div>
    )
  }
}

Reproduction:

https://codesandbox.io/s/5yJ3G59jA

Problem description:

In my handleStateChange handler, I have to check whether inputValue is included in the changes if I want to set the state.

Suggested solution:

What if we just always pass all the state to your onStateChange callback? Then you wouldn't have to check whether inputValue (and other state) exists in the changes.

As I was creating this, I was thinking that maybe we should leave it as it is, because the alternative wouldn't enable you to know what changed, just that something changed.

But what if we did both? So it'd be: this.props.onStateChange(changes, allState)? I think that'd be a good compromise, so I'm going to do that 😄

[the-simian]

I would prefer that, myself. I noticed in the function that you passed most of what I'd be looking for, but why not show it all? Someone therefore can fire a side effect of any mutation they need at this point, and you're allowing folks to access the plumbing more easily.

Ultimately the big picture for this project seems to be that it is an api that enables folks to build any ui, so exposing more is not out of touch with the design direction of the component.

[kentcdodds]

Cool! I've almost got that done. Thanks for the validation.

Also, I'm improving things slightly. So now your onStateChange will only ever be called when state actually changes (not when setState is called) and it's only called with the actual changes (in addition to the second parameter which includes all the state). I think that's what we always wanted, but it wasn't doing that. So I guess you could call this a bug fix 😅

[the-simian]

Ok I'll update my examples with this in mind, I plan on adding more features to it when this version is published also.

[kentcdodds]

It's published as the latest rc 😄