Skip to content

Commit

Permalink
Migrate away from unsafe componentWillReceiveProps. (#26)
Browse files Browse the repository at this point in the history
  • Loading branch information
cdeutsch authored and torleifhalseth committed Oct 11, 2019
1 parent cc64339 commit 983f2d7
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 12 deletions.
13 changes: 8 additions & 5 deletions src/components/Collapse.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,29 +24,32 @@ class Collapse extends PureComponent {
}
}

componentWillReceiveProps(nextProps) {
componentDidUpdate(prevProps) {
if (!this.content) {
return;
}

// If the transition is changed lets update it
if (nextProps.transition !== this.props.transition) {
this.setState({ transition: nextProps.transition });
if (this.props.transition !== prevProps.transition) {
// eslint-disable-next-line react/no-did-update-set-state
this.setState({ transition: this.props.transition });
}

// expand
if (!this.props.isOpen && nextProps.isOpen) {
if (!prevProps.isOpen && this.props.isOpen) {
// have the element transition to the height of its inner content
// eslint-disable-next-line react/no-did-update-set-state
this.setState({
height: `${this.getHeight()}px`,
visibility: 'visible',
});
}

// collapse
if (this.props.isOpen && !nextProps.isOpen) {
if (prevProps.isOpen && !this.props.isOpen) {
// explicitly set the element's height to its current pixel height, so we
// aren't transitioning out of 'auto'
// eslint-disable-next-line react/no-did-update-set-state
this.setState({ height: `${this.getHeight()}px` });
util.requestAnimationFrame(() => {
// "pausing" the JavaScript execution to let the rendering threads catch up
Expand Down
17 changes: 10 additions & 7 deletions src/components/Collapse.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ describe('<Collapse />', () => {
let requestAnimationFrameSpy;
let setExpandedSpy;
let componentDidMountSpy;
let componentWillReceivePropsSpy;
let componentDidUpdateSpy;

beforeAll(() => {
fakeRaf.use();
Expand All @@ -19,17 +19,17 @@ describe('<Collapse />', () => {
requestAnimationFrameSpy = jest.spyOn(util, 'requestAnimationFrame');
setExpandedSpy = jest.spyOn(Collapse.prototype, 'setExpanded');
componentDidMountSpy = jest.spyOn(Collapse.prototype, 'componentDidMount');
componentWillReceivePropsSpy = jest.spyOn(
componentDidUpdateSpy = jest.spyOn(
Collapse.prototype,
'componentWillReceiveProps',
'componentDidUpdate',
);
});

afterEach(() => {
requestAnimationFrameSpy.mockRestore();
setExpandedSpy.mockRestore();
componentDidMountSpy.mockRestore();
componentWillReceivePropsSpy.mockRestore();
componentDidUpdateSpy.mockRestore();
});

afterAll(() => {
Expand Down Expand Up @@ -96,14 +96,16 @@ describe('<Collapse />', () => {
it('should update height when isOpen prop is changed to true', () => {
const wrapper = mount(makeWrapper());
wrapper.setProps({ isOpen: true });
expect(componentWillReceivePropsSpy).toHaveBeenCalled();
expect(componentDidUpdateSpy).toHaveBeenCalled();
wrapper.update();
expect(wrapper.find('div').prop('style').height).toEqual('20px');
});

it('should update visibility when isOpen prop is changed to true', () => {
const wrapper = mount(makeWrapper());
wrapper.setProps({ isOpen: true });
expect(componentWillReceivePropsSpy).toHaveBeenCalled();
expect(componentDidUpdateSpy).toHaveBeenCalled();
wrapper.update();
expect(wrapper.find('div').prop('style').visibility).toEqual('visible');
});

Expand All @@ -115,7 +117,8 @@ describe('<Collapse />', () => {
expect(styles.overflow).toEqual('visible');

wrapper.setProps({ isOpen: false });
expect(componentWillReceivePropsSpy).toHaveBeenCalled();
expect(componentDidUpdateSpy).toHaveBeenCalled();
wrapper.update();
styles = wrapper.find('div').prop('style');
wrapper.update();

Expand Down

0 comments on commit 983f2d7

Please sign in to comment.