Skip to content

Commit

Permalink
fix: migrate unsafe lifecycle (#40)
Browse files Browse the repository at this point in the history
* fix(uncontrollable):  Migrate unsafe lifecycle

Fixes #32.

* Add more test statements for defaultValue.

* Revert change in favor of legacy prefix.

* Migrate to getDerivedStateFromProps.

* Fix tests.

* Bump minimum peerDependency.

* Use polyfill to avoid version bump.
  • Loading branch information
nortonwong authored and jquense committed Oct 3, 2019
1 parent df2cbdc commit 1607fff
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 24 deletions.
5 changes: 3 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -66,12 +66,13 @@
},
"dependencies": {
"@babel/runtime": "^7.4.5",
"invariant": "^2.2.4"
"invariant": "^2.2.4",
"react-lifecycles-compat": "^3.0.4"
},
"release": {
"pkgRoot": "lib",
"extends": [
"@4c/semantic-release-config"
]
}
}
}
43 changes: 24 additions & 19 deletions src/uncontrollable.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import React from 'react'
import { polyfill } from 'react-lifecycles-compat'
import invariant from 'invariant'
import * as Utils from './utils'

Expand Down Expand Up @@ -35,8 +36,10 @@ export default function uncontrollable(Component, controlledValues, methods = []
this._notifying = false
}

this._values[propName] = value
if (!this.unmounted) this.forceUpdate()
if (!this.unmounted)
this.setState(({ values }) => ({
values: Object.assign(Object.create(null), values, { [propName]: value })
}))
}
this.handlers[handlerName] = handleChange
})
Expand All @@ -45,35 +48,35 @@ export default function uncontrollable(Component, controlledValues, methods = []
this.attachRef = ref => {
this.inner = ref
}
}

shouldComponentUpdate() {
//let the forceUpdate trigger the update
return !this._notifying
}

componentWillMount() {
let props = this.props

this._values = Object.create(null)

const values = Object.create(null)
controlledProps.forEach(key => {
this._values[key] = props[Utils.defaultKey(key)]
values[key] = this.props[Utils.defaultKey(key)]
})
this.state = { values, prevProps: {} };
}

componentWillReceiveProps(nextProps) {
let props = this.props
shouldComponentUpdate() {
//let setState trigger the update
return !this._notifying
}

static getDerivedStateFromProps(props, { values, prevProps }) {
const nextState = {
values: Object.assign(Object.create(null), values),
prevProps: {},
}
controlledProps.forEach(key => {
/**
* If a prop switches from controlled to Uncontrolled
* reset its value to the defaultValue
*/
if (!Utils.isProp(nextProps, key) && Utils.isProp(props, key)) {
this._values[key] = nextProps[Utils.defaultKey(key)]
nextState.prevProps[key] = props[key]
if (!Utils.isProp(props, key) && Utils.isProp(prevProps, key)) {
nextState.values[key] = props[Utils.defaultKey(key)]
}
})
return nextState
}

componentWillUnmount() {
Expand All @@ -91,7 +94,7 @@ export default function uncontrollable(Component, controlledValues, methods = []
controlledProps.forEach(propName => {
let propValue = this.props[propName]
newProps[propName] =
propValue !== undefined ? propValue : this._values[propName]
propValue !== undefined ? propValue : this.state.values[propName]
})

return React.createElement(Component, {
Expand All @@ -103,6 +106,8 @@ export default function uncontrollable(Component, controlledValues, methods = []
}
}

polyfill(UncontrolledComponent)

UncontrolledComponent.displayName = `Uncontrolled(${displayName})`

UncontrolledComponent.propTypes = {
Expand Down
53 changes: 50 additions & 3 deletions test/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ describe('uncontrollable', () => {
.first()
.simulate('change', { value: 42 })

expect(inst.children().instance()._values.value).toEqual(42)
expect(inst.children().instance().state.values.value).toEqual(42)
})

it('should allow for defaultProp', () => {
Expand All @@ -226,7 +226,7 @@ describe('uncontrollable', () => {
.tap(inst => expect(inst.getDOMNode().value).toEqual('10'))
.simulate('change', { value: 42 })

expect(inst.children().instance()._values.value).toEqual(42)
expect(inst.children().instance().state.values.value).toEqual(42)
})

it('should not forward default props through', () => {
Expand Down Expand Up @@ -264,7 +264,7 @@ describe('uncontrollable', () => {
expect(() => base.handleChange(42)).not.toThrow()

expect(spy).toHaveBeenCalledTimes(1)
expect(inst.children().instance()._values.value).toEqual(42)
expect(inst.children().instance().state.values.value).toEqual(42)
})

it('should update in the right order when controlled', () => {
Expand Down Expand Up @@ -327,6 +327,45 @@ describe('uncontrollable', () => {
expect(spy.mock.calls[0][0].value).toEqual(84)
})

it('should revert to defaultProp when switched to uncontrollable', () => {
var Control = uncontrollable(Base, { value: 'onChange' }),
spy = jest.fn()

class Parent extends React.Component {
state = { value: 5, defaultValue: 6 }
render() {
return (
<Control
onRender={spy}
value={this.state.value}
defaultValue={this.state.defaultValue}
onChange={value => this.setState({ value })}
/>
)
}
}

var inst = mount(<Parent />);

expect(spy.mock.calls.length).toEqual(1)
expect(spy.mock.calls[0][0].value).toEqual(5)

inst.setState({ value: undefined })

expect(spy.mock.calls.length).toEqual(2)
expect(spy.mock.calls[1][0].value).toEqual(6)

inst.setState({ value: 63 })

expect(spy.mock.calls.length).toEqual(3)
expect(spy.mock.calls[2][0].value).toEqual(63)

inst.setState({ value: undefined, defaultValue: 52 })

expect(spy.mock.calls.length).toEqual(4)
expect(spy.mock.calls[3][0].value).toEqual(52)
})

describe('hook', () => {
it('should track internally if not specified', () => {
let ref = {}
Expand Down Expand Up @@ -414,6 +453,14 @@ describe('uncontrollable', () => {
inst.setProps({ value: undefined })

expect(ref.current.value).toEqual('foo')

inst.setProps({ value: 'bar' })

expect(ref.current.value).toEqual('bar')

inst.setProps({ value: undefined, defaultValue: 'baz' })

expect(ref.current.value).toEqual('baz')
})
})
})

0 comments on commit 1607fff

Please sign in to comment.