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

[#9627] Fix create-react-class isMounted ordering issue #9638

Merged
merged 2 commits into from Jun 12, 2017

Conversation

Projects
None yet
3 participants
@mridgway
Contributor

mridgway commented May 9, 2017

Split the IsMountedMixin in two so that the __isMounted flag is set to false after componentWillUnmount is executed in mixins and the component.

Fixes #9627

@acdlite I wasn't sure how to build the create-react-class.js and create-react-class.min.js. yarn build did not seem to touch those files.

[#9627] Fix create-react-class isMounted ordering issue
Split the IsMountedMixin in two so that the __isMounted flag is set to false after componentWillUnmount is executed in mixins and the component.

@aweary aweary requested a review from acdlite May 9, 2017

@gaearon

This comment has been minimized.

Show comment
Hide comment
@gaearon

gaearon May 10, 2017

Member

I wasn't sure how to build the create-react-class.js and create-react-class.min.js. yarn build did not seem to touch those files.

The sad part is there's no automated way to do it. 😞
I built it with several by hand manipulations last time, and it was pretty error-prone.

I can do it again, but I'd like to make sure we did all changes we want to first.

Member

gaearon commented May 10, 2017

I wasn't sure how to build the create-react-class.js and create-react-class.min.js. yarn build did not seem to touch those files.

The sad part is there's no automated way to do it. 😞
I built it with several by hand manipulations last time, and it was pretty error-prone.

I can do it again, but I'd like to make sure we did all changes we want to first.

@flarnie flarnie self-requested a review May 24, 2017

@flarnie flarnie referenced this pull request May 24, 2017

Closed

React 15.6 Umbrella #9398

41 of 49 tasks complete
@gaearon

This comment has been minimized.

Show comment
Hide comment
@gaearon

gaearon Jun 12, 2017

Member

Looks great. Flow failure seems unrelated—if it continues after merge I'll investigate. Thanks!

Member

gaearon commented Jun 12, 2017

Looks great. Flow failure seems unrelated—if it continues after merge I'll investigate. Thanks!

@gaearon gaearon merged commit 61e8ee7 into facebook:15.6-dev Jun 12, 2017

1 check failed

ci/circleci Your tests failed on CircleCI
Details
@gaearon

This comment has been minimized.

Show comment
Hide comment
@gaearon

gaearon Jun 12, 2017

Member

(I confirmed the test replicates 15.4.x behavior)

Member

gaearon commented Jun 12, 2017

(I confirmed the test replicates 15.4.x behavior)

@mridgway mridgway deleted the mridgway:fixIsMounted branch Jun 12, 2017

nhunzaker added a commit to nhunzaker/react that referenced this pull request Jun 13, 2017

[#9627] Fix create-react-class isMounted ordering issue (#9638)
* [#9627] Fix create-react-class isMounted ordering issue

Split the IsMountedMixin in two so that the __isMounted flag is set to false after componentWillUnmount is executed in mixins and the component.

* Revert changes to integration test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment