Run stateReducer over stateToSet passed to onInputValueChange #830
Conversation
aaronnuu
force-pushed
the
fix/on-input-value-change-state-reducer
branch
2 times, most recently
from
55dc091 to
ba4f826
Compare
…lueChange function
aaronnuu
force-pushed
the
fix/on-input-value-change-state-reducer
branch
from
ba4f826 to
3da6bae
Compare
|
It took me too much, but arrived at this issue and aiming to close it. @aaronnuu are you still interested in merging this? If so I have a few comments, if not i will create a PR myself. issue makes sense, so we should merge the reviewed changes. |
|
I haven't been following the hooks development very closely so if it makes more sense to open a new PR I'm not against it, otherwise yeah still interested in helping out. |
src/__tests__/downshift.lifecycle.js
Outdated
| onInputValueChange: onInputValueChangeSpy, | ||
| }) | ||
| selectItem('bar') | ||
| expect(onInputValueChangeSpy).toHaveBeenCalledTimes(0) |
There was a problem hiding this comment.
.not.toHaveBeenCalled()
| ...stateToSet, | ||
| }) | ||
| if (!isStateToSetFunction) { | ||
| const newStateToSet = this.props.stateReducer(this.state, stateToSet) |
There was a problem hiding this comment.
no reason to call this unless newStateToSet.hasOwnProperty('inputValue') so I would leave the condition as it was and call this before calling onInputValueChange
There was a problem hiding this comment.
I had it this way originally but ran into some issues since the user can add or remove inputValue with the stateReducer so this ensures that onInputValueChange will be called (or not called) if they add / remove it
There was a problem hiding this comment.
I might add a comment for this because it's a bit of a gotcha
|
@silviuaavram Sorry it took a couple of weeks, have made those changes and re-run the snapshot. Wasn't sure how to replicate the helper object that gets sent as the second param to the |
What: Fixes #819
Why: Right now depending on how
internalSetStateis being called (as a state object or function) thestateAndHelpersparam has either been run through thestateReduceror it hasn't, this PR addresses this and makes both implementations work the same.How: I run the
stateToSetthrough thestateReducerand pass that state to theonInputValueChangefunction, exactly how it's being done within the functionalsetStatejust below.Checklist:
I have added a couple of tests that make sure the bugs that I identified aren't a problem anymore, not sure if they're done in the best way or not though so any feedback on that one would be appreciated.