Skip to content

Commit

Permalink
fix(internalSetState): dont change references inside setState updater (
Browse files Browse the repository at this point in the history
…#423)

* fix(internalSetState): dont change references inside setState updater function

* fix(internalSetState): remove change to arg passed to updater function

* fix(internalSetState): add failing test case
  • Loading branch information
drobannx authored and Kent C. Dodds committed Apr 18, 2018
1 parent 8695998 commit 8105906
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 16 deletions.
9 changes: 9 additions & 0 deletions .all-contributorsrc
Original file line number Diff line number Diff line change
Expand Up @@ -738,6 +738,15 @@
"contributions": [
"example"
]
},
{
"login": "drobannx",
"name": "Brandon Clemons",
"avatar_url": "https://avatars2.githubusercontent.com/u/6361167?v=4",
"profile": "https://github.com/drobannx",
"contributions": [
"code"
]
}
]
}
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ autocomplete/dropdown/select/combobox components</p>
[![downloads][downloads-badge]][npmcharts] [![version][version-badge]][package]
[![MIT License][license-badge]][license]

[![All Contributors](https://img.shields.io/badge/all_contributors-74-orange.svg?style=flat-square)](#contributors)
[![All Contributors](https://img.shields.io/badge/all_contributors-75-orange.svg?style=flat-square)](#contributors)
[![PRs Welcome][prs-badge]][prs] [![Chat][chat-badge]][chat]
[![Code of Conduct][coc-badge]][coc]

Expand Down Expand Up @@ -956,7 +956,7 @@ Thanks goes to these people ([emoji key][emojis]):
| [<img src="https://avatars2.githubusercontent.com/u/1556430?v=4" width="100px;"/><br /><sub><b>Pete Redmond</b></sub>](https://httpete.com)<br />[🐛](https://github.com/paypal/downshift/issues?q=author%3Ahttpete-ire "Bug reports") | [<img src="https://avatars2.githubusercontent.com/u/1706342?v=4" width="100px;"/><br /><sub><b>Nick Lavin</b></sub>](https://github.com/Zashy)<br />[🐛](https://github.com/paypal/downshift/issues?q=author%3AZashy "Bug reports") [💻](https://github.com/paypal/downshift/commits?author=Zashy "Code") [⚠️](https://github.com/paypal/downshift/commits?author=Zashy "Tests") | [<img src="https://avatars2.githubusercontent.com/u/17031?v=4" width="100px;"/><br /><sub><b>James Long</b></sub>](http://jlongster.com)<br />[🐛](https://github.com/paypal/downshift/issues?q=author%3Ajlongster "Bug reports") [💻](https://github.com/paypal/downshift/commits?author=jlongster "Code") | [<img src="https://avatars0.githubusercontent.com/u/1505907?v=4" width="100px;"/><br /><sub><b>Michael Ball</b></sub>](http://michaelball.co)<br />[🐛](https://github.com/paypal/downshift/issues?q=author%3Acycomachead "Bug reports") [💻](https://github.com/paypal/downshift/commits?author=cycomachead "Code") | [<img src="https://avatars0.githubusercontent.com/u/8990614?v=4" width="100px;"/><br /><sub><b>CAVALEIRO Julien</b></sub>](https://github.com/Julienng)<br />[💡](#example-Julienng "Examples") | [<img src="https://avatars1.githubusercontent.com/u/3421067?v=4" width="100px;"/><br /><sub><b>Kim Grönqvist</b></sub>](http://www.kimgronqvist.se)<br />[💻](https://github.com/paypal/downshift/commits?author=kimgronqvist "Code") [⚠️](https://github.com/paypal/downshift/commits?author=kimgronqvist "Tests") | [<img src="https://avatars2.githubusercontent.com/u/3675602?v=4" width="100px;"/><br /><sub><b>Sijie</b></sub>](http://sijietian.com)<br />[🐛](https://github.com/paypal/downshift/issues?q=author%3Atiansijie "Bug reports") [💻](https://github.com/paypal/downshift/commits?author=tiansijie "Code") |
| [<img src="https://avatars0.githubusercontent.com/u/410792?v=4" width="100px;"/><br /><sub><b>Dony Sukardi</b></sub>](http://dsds.io)<br />[💡](#example-donysukardi "Examples") [💬](#question-donysukardi "Answering Questions") [💻](https://github.com/paypal/downshift/commits?author=donysukardi "Code") [⚠️](https://github.com/paypal/downshift/commits?author=donysukardi "Tests") | [<img src="https://avatars1.githubusercontent.com/u/2755722?v=4" width="100px;"/><br /><sub><b>Dillon Mulroy</b></sub>](https://dillonmulroy.com)<br />[📖](https://github.com/paypal/downshift/commits?author=dmmulroy "Documentation") | [<img src="https://avatars3.githubusercontent.com/u/12440573?v=4" width="100px;"/><br /><sub><b>Curtis Tate Wilkinson</b></sub>](https://twitter.com/curtytate)<br />[💻](https://github.com/paypal/downshift/commits?author=curtiswilkinson "Code") | [<img src="https://avatars3.githubusercontent.com/u/383212?v=4" width="100px;"/><br /><sub><b>Brice BERNARD</b></sub>](https://github.com/brikou)<br />[🐛](https://github.com/paypal/downshift/issues?q=author%3Abrikou "Bug reports") [💻](https://github.com/paypal/downshift/commits?author=brikou "Code") | [<img src="https://avatars3.githubusercontent.com/u/14304503?v=4" width="100px;"/><br /><sub><b>Tony Xu</b></sub>](https://github.com/xutopia)<br />[💻](https://github.com/paypal/downshift/commits?author=xutopia "Code") | [<img src="https://avatars1.githubusercontent.com/u/14035529?v=4" width="100px;"/><br /><sub><b>Anthony Ng</b></sub>](http://anthonyng.me)<br />[📖](https://github.com/paypal/downshift/commits?author=newyork-anthonyng "Documentation") | [<img src="https://avatars2.githubusercontent.com/u/11996139?v=4" width="100px;"/><br /><sub><b>S S</b></sub>](https://github.com/notruth)<br />[💬](#question-notruth "Answering Questions") [💻](https://github.com/paypal/downshift/commits?author=notruth "Code") [📖](https://github.com/paypal/downshift/commits?author=notruth "Documentation") [🤔](#ideas-notruth "Ideas, Planning, & Feedback") [⚠️](https://github.com/paypal/downshift/commits?author=notruth "Tests") |
| [<img src="https://avatars0.githubusercontent.com/u/29493001?v=4" width="100px;"/><br /><sub><b>Austin Tackaberry</b></sub>](http://austintackaberry.co)<br />[💬](#question-austintackaberry "Answering Questions") [💻](https://github.com/paypal/downshift/commits?author=austintackaberry "Code") [📖](https://github.com/paypal/downshift/commits?author=austintackaberry "Documentation") [🐛](https://github.com/paypal/downshift/issues?q=author%3Aaustintackaberry "Bug reports") [💡](#example-austintackaberry "Examples") [🤔](#ideas-austintackaberry "Ideas, Planning, & Feedback") [👀](#review-austintackaberry "Reviewed Pull Requests") [⚠️](https://github.com/paypal/downshift/commits?author=austintackaberry "Tests") | [<img src="https://avatars3.githubusercontent.com/u/4168055?v=4" width="100px;"/><br /><sub><b>Jean Duthon</b></sub>](https://github.com/jduthon)<br />[🐛](https://github.com/paypal/downshift/issues?q=author%3Ajduthon "Bug reports") [💻](https://github.com/paypal/downshift/commits?author=jduthon "Code") | [<img src="https://avatars3.githubusercontent.com/u/3889580?v=4" width="100px;"/><br /><sub><b>Anton Telesh</b></sub>](http://antontelesh.github.io)<br />[🐛](https://github.com/paypal/downshift/issues?q=author%3AAntontelesh "Bug reports") [💻](https://github.com/paypal/downshift/commits?author=Antontelesh "Code") | [<img src="https://avatars3.githubusercontent.com/u/1060669?v=4" width="100px;"/><br /><sub><b>Eric Edem</b></sub>](https://github.com/ericedem)<br />[💻](https://github.com/paypal/downshift/commits?author=ericedem "Code") [📖](https://github.com/paypal/downshift/commits?author=ericedem "Documentation") [🤔](#ideas-ericedem "Ideas, Planning, & Feedback") [⚠️](https://github.com/paypal/downshift/commits?author=ericedem "Tests") | [<img src="https://avatars3.githubusercontent.com/u/3409645?v=4" width="100px;"/><br /><sub><b>Austin Wood</b></sub>](https://github.com/indiesquidge)<br />[💬](#question-indiesquidge "Answering Questions") [📖](https://github.com/paypal/downshift/commits?author=indiesquidge "Documentation") [👀](#review-indiesquidge "Reviewed Pull Requests") | [<img src="https://avatars3.githubusercontent.com/u/14275790?v=4" width="100px;"/><br /><sub><b>Mark Murray</b></sub>](https://github.com/mmmurray)<br />[🚇](#infra-mmmurray "Infrastructure (Hosting, Build-Tools, etc)") | [<img src="https://avatars0.githubusercontent.com/u/1862172?v=4" width="100px;"/><br /><sub><b>Gianmarco</b></sub>](https://github.com/gsimone)<br />[🐛](https://github.com/paypal/downshift/issues?q=author%3Agsimone "Bug reports") [💻](https://github.com/paypal/downshift/commits?author=gsimone "Code") |
| [<img src="https://avatars2.githubusercontent.com/u/6838136?v=4" width="100px;"/><br /><sub><b>Emmanuel Pastor</b></sub>](https://github.com/pastr)<br />[💡](#example-pastr "Examples") | [<img src="https://avatars2.githubusercontent.com/u/10345034?v=4" width="100px;"/><br /><sub><b>dalehurwitz</b></sub>](https://github.com/dalehurwitz)<br />[💻](https://github.com/paypal/downshift/commits?author=dalehurwitz "Code") | [<img src="https://avatars1.githubusercontent.com/u/4813007?v=4" width="100px;"/><br /><sub><b>Bogdan Lobor</b></sub>](https://github.com/blobor)<br />[🐛](https://github.com/paypal/downshift/issues?q=author%3Ablobor "Bug reports") [💻](https://github.com/paypal/downshift/commits?author=blobor "Code") | [<img src="https://avatars0.githubusercontent.com/u/1127238?v=4" width="100px;"/><br /><sub><b>Luke Herrington</b></sub>](https://github.com/infiniteluke)<br />[💡](#example-infiniteluke "Examples") |
| [<img src="https://avatars2.githubusercontent.com/u/6838136?v=4" width="100px;"/><br /><sub><b>Emmanuel Pastor</b></sub>](https://github.com/pastr)<br />[💡](#example-pastr "Examples") | [<img src="https://avatars2.githubusercontent.com/u/10345034?v=4" width="100px;"/><br /><sub><b>dalehurwitz</b></sub>](https://github.com/dalehurwitz)<br />[💻](https://github.com/paypal/downshift/commits?author=dalehurwitz "Code") | [<img src="https://avatars1.githubusercontent.com/u/4813007?v=4" width="100px;"/><br /><sub><b>Bogdan Lobor</b></sub>](https://github.com/blobor)<br />[🐛](https://github.com/paypal/downshift/issues?q=author%3Ablobor "Bug reports") [💻](https://github.com/paypal/downshift/commits?author=blobor "Code") | [<img src="https://avatars0.githubusercontent.com/u/1127238?v=4" width="100px;"/><br /><sub><b>Luke Herrington</b></sub>](https://github.com/infiniteluke)<br />[💡](#example-infiniteluke "Examples") | [<img src="https://avatars2.githubusercontent.com/u/6361167?v=4" width="100px;"/><br /><sub><b>Brandon Clemons</b></sub>](https://github.com/drobannx)<br />[💻](https://github.com/paypal/downshift/commits?author=drobannx "Code") |

<!-- ALL-CONTRIBUTORS-LIST:END -->

Expand Down
16 changes: 16 additions & 0 deletions src/__tests__/downshift.misc.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,22 @@ test('can use children instead of render prop', () => {
expect(childrenSpy).toHaveBeenCalledTimes(1)
})

test('should not throw error during strict mode during reset', () => {
let renderArg
const renderFn = () => <div />
const renderSpy = jest.fn(controllerArg => {
renderArg = controllerArg
return renderFn(controllerArg)
})
render(
<React.StrictMode>
<Downshift render={renderSpy} />
</React.StrictMode>,
)

renderArg.reset()
})

test('can use setState for ultimate power', () => {
const {renderSpy, setState} = setup()
renderSpy.mockClear()
Expand Down
37 changes: 23 additions & 14 deletions src/downshift.js
Original file line number Diff line number Diff line change
Expand Up @@ -318,32 +318,38 @@ class Downshift extends Component {
return this.setState(
state => {
state = this.getState(state)
stateToSet = isStateToSetFunction ? stateToSet(state) : stateToSet
let newStateToSet = isStateToSetFunction
? stateToSet(state)
: stateToSet

// Your own function that could modify the state that will be set.
stateToSet = this.props.stateReducer(state, stateToSet)
newStateToSet = this.props.stateReducer(state, newStateToSet)

// checks if an item is selected, regardless of if it's different from
// what was selected before
// used to determine if onSelect and onChange callbacks should be called
isItemSelected = stateToSet.hasOwnProperty('selectedItem')
isItemSelected = newStateToSet.hasOwnProperty('selectedItem')
// this keeps track of the object we want to call with setState
const nextState = {}
// this is just used to tell whether the state changed
const nextFullState = {}
// we need to call on change if the outside world is controlling any of our state
// and we're trying to update that state. OR if the selection has changed and we're
// trying to update the selection
if (isItemSelected && stateToSet.selectedItem !== state.selectedItem) {
onChangeArg = stateToSet.selectedItem
if (
isItemSelected &&
newStateToSet.selectedItem !== state.selectedItem
) {
onChangeArg = newStateToSet.selectedItem
}
stateToSet.type = stateToSet.type || Downshift.stateChangeTypes.unknown
newStateToSet.type =
newStateToSet.type || Downshift.stateChangeTypes.unknown

Object.keys(stateToSet).forEach(key => {
Object.keys(newStateToSet).forEach(key => {
// onStateChangeArg should only have the state that is
// actually changing
if (state[key] !== stateToSet[key]) {
onStateChangeArg[key] = stateToSet[key]
if (state[key] !== newStateToSet[key]) {
onStateChangeArg[key] = newStateToSet[key]
}
// the type is useful for the onStateChangeArg
// but we don't actually want to set it in internal state.
Expand All @@ -354,19 +360,22 @@ class Downshift extends Component {
if (key === 'type') {
return
}
nextFullState[key] = stateToSet[key]
nextFullState[key] = newStateToSet[key]
// if it's coming from props, then we don't care to set it internally
if (!this.isControlledProp(key)) {
nextState[key] = stateToSet[key]
nextState[key] = newStateToSet[key]
}
})

// if stateToSet is a function, then we weren't able to call onInputValueChange
// earlier, so we'll call it now that we know what the inputValue state will be.
if (isStateToSetFunction && stateToSet.hasOwnProperty('inputValue')) {
this.props.onInputValueChange(stateToSet.inputValue, {
if (
isStateToSetFunction &&
newStateToSet.hasOwnProperty('inputValue')
) {
this.props.onInputValueChange(newStateToSet.inputValue, {
...this.getStateAndHelpers(),
...stateToSet,
...newStateToSet,
})
}

Expand Down

0 comments on commit 8105906

Please sign in to comment.