From 1607ffffaee282e865af263b95a87ea38225be4d Mon Sep 17 00:00:00 2001 From: Norton Wong Date: Thu, 3 Oct 2019 14:23:58 -0400 Subject: [PATCH] fix: migrate unsafe lifecycle (#40) * fix(uncontrollable): Migrate unsafe lifecycle Fixes https://github.com/jquense/uncontrollable/issues/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. --- package.json | 5 ++-- src/uncontrollable.js | 43 +++++++++++++++++++---------------- test/test.js | 53 ++++++++++++++++++++++++++++++++++++++++--- 3 files changed, 77 insertions(+), 24 deletions(-) diff --git a/package.json b/package.json index 4938be2..2b1ffc4 100644 --- a/package.json +++ b/package.json @@ -66,7 +66,8 @@ }, "dependencies": { "@babel/runtime": "^7.4.5", - "invariant": "^2.2.4" + "invariant": "^2.2.4", + "react-lifecycles-compat": "^3.0.4" }, "release": { "pkgRoot": "lib", @@ -74,4 +75,4 @@ "@4c/semantic-release-config" ] } -} \ No newline at end of file +} diff --git a/src/uncontrollable.js b/src/uncontrollable.js index 20cf453..e4f32c0 100644 --- a/src/uncontrollable.js +++ b/src/uncontrollable.js @@ -1,4 +1,5 @@ import React from 'react' +import { polyfill } from 'react-lifecycles-compat' import invariant from 'invariant' import * as Utils from './utils' @@ -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 }) @@ -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() { @@ -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, { @@ -103,6 +106,8 @@ export default function uncontrollable(Component, controlledValues, methods = [] } } + polyfill(UncontrolledComponent) + UncontrolledComponent.displayName = `Uncontrolled(${displayName})` UncontrolledComponent.propTypes = { diff --git a/test/test.js b/test/test.js index ee6ab47..dadd603 100644 --- a/test/test.js +++ b/test/test.js @@ -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', () => { @@ -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', () => { @@ -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', () => { @@ -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 ( + this.setState({ value })} + /> + ) + } + } + + var inst = mount(); + + 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 = {} @@ -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') }) }) })