Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Even when setState returns null, it calls cDU lifecycle. #1783

Closed
Ailrun opened this issue Aug 21, 2018 · 11 comments
Closed

Even when setState returns null, it calls cDU lifecycle. #1783

Ailrun opened this issue Aug 21, 2018 · 11 comments

Comments

@Ailrun
Copy link

Ailrun commented Aug 21, 2018

Describe the bug
In React, if setState returns null, cDU is not called and so following does not make infinite loop even after cDU is called. (You can check it on this codesandbox by typing text on the input. It prints only one console log at a time.)

export class TestComponent extends Component {
  constructor(props) {
    super(props);

    this.state = {};
  }

  componentDidUpdate() {
    this.setState(() => {
      return null;
    });
  }

  render() {
    return null;
  }
}

However, in enzyme, when I update component, i.e., invoking cDU with above component, will gives me infinite loop like

      at withSetStateAllowed (node_modules/enzyme/build/Utils.js:296:21)
      at ShallowWrapper.<anonymous> (node_modules/enzyme/build/ShallowWrapper.js:511:42)
      at ShallowWrapper.single (node_modules/enzyme/build/ShallowWrapper.js:1735:25)
      at ShallowWrapper.setState (node_modules/enzyme/build/ShallowWrapper.js:510:14)
      at TestComponent.ShallowWrapper.instance.setState (node_modules/enzyme/build/ShallowWrapper.js:190:35)
      at TestComponent.setState [as componentDidUpdate] (TestComponent.js:11:10)
      at node_modules/enzyme/build/ShallowWrapper.js:548:28
      at withSetStateAllowed (node_modules/enzyme/build/Utils.js:300:3)
      at ShallowWrapper.<anonymous> (node_modules/enzyme/build/ShallowWrapper.js:511:42)
      at ShallowWrapper.single (node_modules/enzyme/build/ShallowWrapper.js:1735:25)
      at ShallowWrapper.setState (node_modules/enzyme/build/ShallowWrapper.js:510:14)
      at TestComponent.ShallowWrapper.instance.setState (node_modules/enzyme/build/ShallowWrapper.js:190:35)
      at TestComponent.setState [as componentDidUpdate] (TestComponent.js:11:10)
      at node_modules/enzyme/build/ShallowWrapper.js:548:28
      // and repeat from line 1 to line 7

To Reproduce

  1. git clone https://github.com/Ailrun/enzyme-issue1783
  2. npm i
  3. npm test

Expected behavior
Does not give infinite loop.

Desktop (please complete the following information):

@ljharb
Copy link
Member

ljharb commented Aug 21, 2018

I had no idea the return value of setState mattered at all. cc @koba04

Thanks, I’ll get a fix for this in soon.

@ljharb
Copy link
Member

ljharb commented Aug 21, 2018

@Ailrun in that code sandbox, when i type a single letter, i get a single console log - but i get the same even if i comment out the return null line inside the setState callback. However, when I add return {} or return false, it infinitely loops - indicating that this is the correct behavior - except for null/undefined.

Thanks for the report.

@Ailrun
Copy link
Author

Ailrun commented Aug 21, 2018

@ljharb yes, it also ignores undefined too. I missed that part. Thank you.

@koba04
Copy link
Contributor

koba04 commented Aug 21, 2018

Thanks to open the issue! I’ll working on this.

@ljharb
Copy link
Member

ljharb commented Aug 21, 2018

@koba04 i'm about to restructure all the lifecycle method tests, so you may want to hold off for an hour or so :-)

@Ailrun
Copy link
Author

Ailrun commented Aug 22, 2018

@ljharb That sounds great! I cannot wait to use fixed lifecycles 😃

@ljharb
Copy link
Member

ljharb commented Aug 22, 2018

@koba04 k, go for it :-D

@koba04
Copy link
Contributor

koba04 commented Aug 22, 2018

I've created a PR to fix this. #1785

@Ailrun
Copy link
Author

Ailrun commented Aug 22, 2018

@ljharb Can I know when will this be released?

@ljharb
Copy link
Member

ljharb commented Aug 22, 2018

I'll probably release a v3.5.0 later this week.

@Ailrun
Copy link
Author

Ailrun commented Aug 22, 2018

@ljharb Really thank you for this fast response!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants