Skip to content
This repository was archived by the owner on Mar 4, 2020. It is now read-only.

fix(AutoControlledComponent): fix behaviour when undefined it passed #499

Merged
merged 3 commits into from
Nov 20, 2018
Merged

fix(AutoControlledComponent): fix behaviour when undefined it passed #499

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
30d86cc
fix(AutoControlledComponent): fix behaviour when undefined it passed
layershifter Nov 20, 2018
06605e8
docs(CHANGELOG): add entry
layershifter Nov 20, 2018
01a928e
Merge branch 'master' into fix/acc-default
layershifter Nov 20, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm

## [Unreleased]

### Fixes
- Fix the behaviour of `AutoControlledComponent` when `undefined` is passed as a prop value @layershifter ([#499](https://github.com/stardust-ui/react/pull/499))

<!--------------------------------[ v0.12.0 ]------------------------------- -->
## [v0.12.0](https://github.com/stardust-ui/react/tree/v0.12.0) (2018-11-19)
[Compare changes](https://github.com/stardust-ui/react/compare/v0.11.0...v0.12.0)
Expand Down
7 changes: 2 additions & 5 deletions src/lib/AutoControlledComponent.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,13 +171,10 @@ export default class AutoControlledComponent<P = {}, S = {}> extends UIComponent

// Solve the next state for autoControlledProps
const newState = autoControlledProps.reduce((acc, prop) => {
const isNextUndefined = _.isUndefined(nextProps[prop])
const propWasRemoved = !_.isUndefined(this.props[prop]) && isNextUndefined
const isNextDefined = !_.isUndefined(nextProps[prop])

// if next is defined then use its value
if (!isNextUndefined) acc[prop] = nextProps[prop]
// reinitialize state for props just removed / set undefined
else if (propWasRemoved) acc[prop] = getAutoControlledStateValue(prop, nextProps)
if (isNextDefined) acc[prop] = nextProps[prop]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In fact, we don't need to do something with undefined at all. We should update the state only for defined values from props


return acc
}, {})
Expand Down
22 changes: 2 additions & 20 deletions test/specs/lib/AutoControlledComponent-test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ describe('extending AutoControlledComponent', () => {
expect(wrapper.state()).not.toHaveProperty(randomDefaultProp, randomValue)
})

test('does not return state to default props when setting props undefined', () => {
test('keeps current state value when setting props undefined', () => {
consoleUtil.disableOnce()

const autoControlledProps = ['foo']
Expand All @@ -333,25 +333,7 @@ describe('extending AutoControlledComponent', () => {
expect(wrapper.state()).toHaveProperty('foo', 'initial')

wrapper.setProps({ foo: undefined })

expect(wrapper.state()).toHaveProperty('foo', undefined)
})

test('sets state to undefined for props passed as undefined by the parent', () => {
consoleUtil.disableOnce()
const props = makeProps()
const autoControlledProps = _.keys(props)

const randomProp = _.sample(autoControlledProps)

TestClass = createTestClass({ autoControlledProps, state: {} })
const wrapper = shallow(<TestClass {...props} />)

expect(wrapper.state()).toHaveProperty(randomProp)

wrapper.setProps({ [randomProp]: undefined })

expect(wrapper.state()).toHaveProperty(randomProp, undefined)
expect(wrapper.state()).toHaveProperty('foo', 'initial')
})

test('does not set state for props passed as null by the parent', () => {
Expand Down