Skip to content

Commit

Permalink
[#9627] Fix create-react-class isMounted ordering issue (#9638)
Browse files Browse the repository at this point in the history
* [#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
  • Loading branch information
mridgway authored and gaearon committed Jun 12, 2017
1 parent 088d593 commit 61e8ee7
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 4 deletions.
10 changes: 7 additions & 3 deletions addons/create-react-class/factory.js
Original file line number Diff line number Diff line change
Expand Up @@ -584,10 +584,13 @@ function factory(ReactComponent, isValidElement, ReactNoopUpdateQueue) {
}
}

var IsMountedMixin = {
var IsMountedPreMixin = {
componentDidMount: function () {
this.__isMounted = true;
},
}
};

var IsMountedPostMixin = {
componentWillUnmount: function () {
this.__isMounted = false;
}
Expand Down Expand Up @@ -679,8 +682,9 @@ function factory(ReactComponent, isValidElement, ReactNoopUpdateQueue) {

injectedMixins.forEach(mixSpecIntoComponent.bind(null, Constructor));

mixSpecIntoComponent(Constructor, IsMountedMixin);
mixSpecIntoComponent(Constructor, IsMountedPreMixin);
mixSpecIntoComponent(Constructor, spec);
mixSpecIntoComponent(Constructor, IsMountedPostMixin);

// Initialize the defaultProps property after all mixins have been merged.
if (Constructor.getDefaultProps) {
Expand Down
26 changes: 25 additions & 1 deletion addons/create-react-class/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,25 @@ describe('ReactClass-spec', () => {
var instance;
var Component = createReactClass({
displayName: 'MyComponent',
mixins: [
{
componentWillMount() {
this.log('mixin.componentWillMount');
},
componentDidMount() {
this.log('mixin.componentDidMount');
},
componentWillUpdate() {
this.log('mixin.componentWillUpdate');
},
componentDidUpdate() {
this.log('mixin.componentDidUpdate');
},
componentWillUnmount() {
this.log('mixin.componentWillUnmount');
},
},
],
log(name) {
ops.push(`${name}: ${this.isMounted()}`);
},
Expand Down Expand Up @@ -430,13 +449,18 @@ describe('ReactClass-spec', () => {
instance.log('after unmount');
expect(ops).toEqual([
'getInitialState: false',
'mixin.componentWillMount: false',
'componentWillMount: false',
'render: false',
'mixin.componentDidMount: true',
'componentDidMount: true',
'mixin.componentWillUpdate: true',
'componentWillUpdate: true',
'render: true',
'mixin.componentDidUpdate: true',
'componentDidUpdate: true',
'componentWillUnmount: false',
'mixin.componentWillUnmount: true',
'componentWillUnmount: true',
'after unmount: false',
]);

Expand Down

0 comments on commit 61e8ee7

Please sign in to comment.