Skip to content

Commit

Permalink
[facebook#9627] Fix create-react-class isMounted ordering issue
Browse files Browse the repository at this point in the history
  • Loading branch information
mridgway committed May 9, 2017
1 parent 4f7136e commit 783edb1
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 5 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 @@ -585,10 +585,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 @@ -680,8 +683,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
26 changes: 25 additions & 1 deletion src/isomorphic/classic/class/__tests__/ReactCreateClass-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,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 @@ -419,13 +438,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 783edb1

Please sign in to comment.