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

componentDidMount not called in a specific case #172

Closed
kruczy opened this issue Jun 1, 2016 · 5 comments
Closed

componentDidMount not called in a specific case #172

kruczy opened this issue Jun 1, 2016 · 5 comments
Milestone

Comments

@kruczy
Copy link
Contributor

kruczy commented Jun 1, 2016

The lifecycle hook componentDidMount does not get called for components that return false form shouldComponentUpdate and are wrapped in a normal component

see repro:
http://jsfiddle.net/2sh26um7/4/ (4.8.0)
http://jsfiddle.net/48e5jdsk/2/ (5.0.0-beta5)

the beta fixes the immediate issue from 4.8.0 but if you wrap the component twice the same problem happens

click the toggle button 4 times in console you will see mount unmount unmount
the expected log would be mount unmount mount unmount

if you remove the wrapper the log is correct

If there is anything I can help with please let me know

@ouzhenkun
Copy link
Contributor

ouzhenkun commented Jun 1, 2016

kruczy I have the similar issue before and [email protected] can fix it. please take a try.
http://jsfiddle.net/ryan_ou/8n9338h8/
ref #143

@developit
Copy link
Member

Yes, 5.0.0-beta6 fixes the mount event. It looks like it might duplicate the unmount event though, looking into that now.

@kruczy
Copy link
Contributor Author

kruczy commented Jun 1, 2016

Whoops, didn't see beta6
this is a version with beta6: ~~http://jsfiddle.net/48e5jdsk/3/~~http://jsfiddle.net/48e5jdsk/4/

it calls both mount and unmount now but unmount gets called 3 times in a row, this is much better now as protecting against multiple calls to unmount is quite easy, though probably should be fixed

@developit
Copy link
Member

Agreed. I'm thinking it might finally be time to store a "mounted" boolean, but I'd also just like to address why Preact is even trying to unmount them in the first place. I think it's the High Order Component recursion seeing a new "owner" component for the same element 3 times (because the nesting level is 3).

Thanks so much for this repro! You guys have been awesome.

@developit developit added this to the 5.0 milestone Jun 3, 2016
@developit
Copy link
Member

@kruczy + @zhenkunou - Forgot to tag the commit with this issue, but I've published what I hope is a proper fix for this. It fixes the repro: http://jsfiddle.net/developit/48e5jdsk/7/

Closing as hopeful for now but let me know if you have other cases/repros that are still affected!

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