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

Callback of setState is called at not expected time. #556

Closed
windyGex opened this issue Feb 22, 2017 · 17 comments · Fixed by #3806
Closed

Callback of setState is called at not expected time. #556

windyGex opened this issue Feb 22, 2017 · 17 comments · Fixed by #3806

Comments

@windyGex
Copy link
Contributor

See the following code:

class A extends React.Component {
  componentWillMount() {
    this.setState({
        a: 'a'
    }, () => {
      console.log('setState');
    })
  }
  render() {
    return <B></B>
  }
}

class B extends React.Component{
  render() {
    return <div>B</div>;
  }
  componentDidMount() {
    console.log('didMount');
  }
}


ReactDOM.render(<A/>, mountNode);

Preact will log:

setState
didMount

React will log:

didMount
setState

I'm not sure if this is a bug?It will be error when access domNode of child component in callback.

@developit
Copy link
Member

developit commented Feb 22, 2017

Preact folds pre-mount setState() calls into the initial render by design. I hadn't thought of the callback issue though, that's cumbersome. Maybe it could be addressed similarly to #506, not sure.

@windyGex
Copy link
Contributor Author

windyGex commented Feb 23, 2017

What's your opinion? @robertknight

Look into this code:

https://github.com/developit/preact/blob/master/src/vdom/component.js#L193

Maybe removing the call flushMount conditions can solve this problem, I'm not sure if cause other problem. @developit

@robertknight
Copy link
Member

I can confirm this happens in Preact 10: https://codesandbox.io/s/lp5xyqq4vz

@edimitchel
Copy link

edimitchel commented Jun 26, 2019

@developit And now, with preact x beta, callback is no more called !
https://codesandbox.io/s/556-nop2h

@JoviDeCroock
Copy link
Member

JoviDeCroock commented Jun 26, 2019

Hey @edimitchel, I can confirm that it works in componentDidMount and not in componentWillMount. I'll try to look into this later, for the time being you could use didMount instead.

@edimitchel
Copy link

edimitchel commented Jun 26, 2019

Hey @edimitchel, I can confirm that it works in componentDidMount and not in componentWillMount. I'll try to look into this later, for the time being you could use didMount instead.

I used on componentDidMount on my personal project and this not working. Actually, you're right, on the demo, it works like expected. But for now, on my project, it doesn't.

I refactor my code and now, the callback is called as expected.. I execute another setState inside the callback (yes, I know, legacy code..), and the callback of this isn't called

@JoviDeCroock
Copy link
Member

JoviDeCroock commented Jun 26, 2019

@edimitchel it seems to work here: https://codesandbox.io/s/556-wlfp4

@edimitchel
Copy link

@JoviDeCroock Yes, I changed the Will by Did.

@JoviDeCroock
Copy link
Member

I execute another setState inside the callback (yes, I know, legacy code..), and the callback of this isn't called

I was talking about this part

@edimitchel
Copy link

edimitchel commented Jun 26, 2019

Sorry, I thought it was the same codesandbox url.
Can you see the issue here : https://codesandbox.io/s/556-glj62
The state doesn't change on the second callback.
This is indeed the origin of my issue !

The render have the right state but not inside the callback.. The first did though..

@JoviDeCroock
Copy link
Member

Oh, the state doesn't seem to be updated in the callback but it is in the render, will try to look into it soon.

@edimitchel
Copy link

I have to create an issue for that ? @JoviDeCroock

@JoviDeCroock
Copy link
Member

It would be nice to have these separate since this one revolves around componentWillMount and no callbacks at all.

@robertknight
Copy link
Member

Given that componentWillMount is deprecated in React, I wonder if it is still worth trying to fix this?

@planttheidea
Copy link

@robertknight componentWillMount has been deprecated, but only the unprefixed versions. UNSAFE_componentWillMount will continue to work going forward.

@ForsakenHarmony
Copy link
Member

ForsakenHarmony commented Dec 4, 2019

They're saying "Only the new “UNSAFE_” lifecycle names will work from this point forward.", not "The new “UNSAFE_” lifecycle names will work from this point forward."

Which doesn't say they're maintaining it long term, but yes there's no definite plan for fully removing them (there might be some very special use cases where you do want them)

Also "these lifecycles will be more likely to have bugs in future versions of React, especially once async rendering is enabled."

@planttheidea
Copy link

@ForsakenHarmony I suppose it depends on your definition of long-term. The non-prefixed versions will be deprecated after version 16, however "UNSAFE_" will still work and be maintained throughout version 17. No guarantees for version 18 and beyond, however that is a distant future. Should we dismiss the use-case because one day, years down the line, the API may be gone entirely?

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

Successfully merging a pull request may close this issue.

8 participants