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

Fix race condition in render/unmount #124

Closed
wants to merge 1 commit into from

Conversation

NumminorihSF
Copy link

@NumminorihSF NumminorihSF commented Jan 17, 2017

There is a race condition in rendering/unmounting portal.
If you open the portal and unmount parent component, state.active will be false. But portal will be mounted and active. This is because this.setState() may works async. The solution is use the 2nd setState() argument, that is callback.

@Gyran
Copy link

Gyran commented Feb 6, 2017

I ran into a case where this PR would have saved me.

@dwick
Copy link

dwick commented Apr 20, 2017

@tajo this also fixes an issue for me. Any chance this might land soon?

@dwick
Copy link

dwick commented Apr 20, 2017

I've forked this and published to https://www.npmjs.com/package/react-portal-fork for those waiting on this to be fixed.

@davidsturgeon
Copy link

Ran into this issue as well. Any idea on when a merge into master will happen?

@tajo tajo added this to the v4 milestone Jul 29, 2017
@tajo tajo added the bug label Jul 29, 2017
@bradleyayers
Copy link

I just ran into it, I think the real fix here should be to use componentDidUpdate rather than componentWillReceiveProps. componentDidUpdate takes into consideration state changes as well as prop changes. I've adapted the code and tested it in my scenario and it seems much more stable.

@tajo
Copy link
Owner

tajo commented Oct 1, 2017

Hey, please check the new major version (complete rewrite) of react-portal: #157

It's React v16 only since its uses the new official Portal API. There is the first beta released and I would like to get your feedback. I don't have bandwidth to maintain v3 which is very different and full of hacks.

If you feel your PR still applies, please rebase it against the master and re-open it. Sorry for the lack of response in past! Thanks!

@tajo tajo closed this Oct 1, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants