Skip to content

Commit

Permalink
Fixes an issue where setState callbacks trigger even when component was
Browse files Browse the repository at this point in the history
unmounted
  • Loading branch information
Sampo Kivistö committed Mar 2, 2020
1 parent 68ab63f commit 0e6b243
Show file tree
Hide file tree
Showing 3 changed files with 156 additions and 11 deletions.
12 changes: 6 additions & 6 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -78,14 +78,14 @@
"test:package": "node fixtures/packaging/build-all.js"
},
"devDependencies": {
"@babel/core": "^7.8.4",
"@babel/core": "^7.8.6",
"@babel/plugin-proposal-class-properties": "7.8.3",
"@babel/preset-env": "7.8.4",
"@babel/preset-env": "7.8.6",
"@babel/preset-typescript": "^7.8.3",
"@rollup/plugin-alias": "^3.0.1",
"@types/history": "^4.7.5",
"@types/jest": "^25.1.3",
"@types/node": "^13.7.4",
"@types/node": "^13.7.7",
"babel-core": "^7.0.0-bridge.0",
"babel-jest": "^25.1.0",
"babel-plugin-inferno": "6.1.0",
Expand All @@ -102,7 +102,7 @@
"jest-silent-reporter": "^0.2.1",
"jsdom": "^16.2.0",
"lerna": "3.20.2",
"lint-staged": "^10.0.7",
"lint-staged": "^10.0.8",
"madge": "^3.7.0",
"merge-dirs": "^0.2.1",
"minimist": "^1.2.0",
Expand All @@ -112,7 +112,7 @@
"pre-commit": "^1.2.2",
"prettier": "^1.19.1",
"rimraf": "^3.0.2",
"rollup": "1.31.1",
"rollup": "1.32.0",
"rollup-plugin-babel": "^4.3.3",
"rollup-plugin-buble": "^0.19.8",
"rollup-plugin-commonjs": "^10.1.0",
Expand All @@ -123,6 +123,6 @@
"sinon": "^9.0.0",
"tslint": "^6.0.0",
"tslint-config-prettier": "^1.18.0",
"typescript": "3.8.2"
"typescript": "3.8.3"
}
}
147 changes: 145 additions & 2 deletions packages/inferno/__tests__/async-setstate.spec.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ describe('Async set state issue', () => {
expect(_failureCreatorCBRequested).toBe(4);
expect(_justBecauseCBRequested).toBe(4);

expect(container.innerHTML).toBe('<div><div>first 2 2 true true</div><div>second 2 2 true true</div></div>')
expect(container.innerHTML).toBe('<div><div>first 2 2 true true</div><div>second 2 2 true true</div></div>');

done();
}, 20);
Expand Down Expand Up @@ -284,7 +284,150 @@ describe('Async set state issue', () => {
expect(testBeforeAfterSpy.calledBefore(testAfterBeforeSpy)).toBe(true);
expect(testAfterBeforeSpy.calledBefore(testAfterAfterSpy)).toBe(true);

expect(container.innerHTML).toBe('<div><div>2</div><div>2</div></div>')
expect(container.innerHTML).toBe('<div><div>2</div><div>2</div></div>');

done();
}, 20);
});

it('Should not call applystate for components which were unmounted during the micro task startup', function (done) {
class HoC extends Component {
constructor(props) {
super(props);

this.update = this.update.bind(this);
}

update() {
this.setState({});
}

render(props) {
return (
<div>
<TestBefore update={this.update} run={props.run}/>
<TestAfter update={this.update} run={props.run}/>
</div>
);
}
}

let testBeforeBeforeSpy,
testBeforeAfterSpy,
testAfterBeforeSpy,
testAfterAfterSpy;

class TestBefore extends Component {
constructor(props) {
super(props);

this.state = {
async: 0
};

testBeforeBeforeSpy = spy(this, '_before');
testBeforeAfterSpy = spy(this, '_after');
}

_forceASYNC() {
// hack just for testing, this forces parent is updating so we can test async setState flow
if (this.state.counter === 1) {
this.props.update();
}
}

_before() {}

_after() {}

_fromCWRP() {
this._forceASYNC();

this.setState({
async: 1
}, this._before);

this.setState({
async: 2
}, this._after);
}

componentWillReceiveProps(nextProps, nextContext) {
this.setState({
counter: this.state.counter + 1
}, this._fromCWRP);
}

render() {
return (
<div>
{`${this.state.async}`}
</div>
);
}
}

class TestAfter extends Component {
constructor(props) {
super(props);

this.state = {
async: 0
};

testAfterBeforeSpy = spy(this, '_before');
testAfterAfterSpy = spy(this, '_after');
}

_forceASYNC() {
// hack just for testing, this forces parent is updating so we can test async setState flow
if (this.state.counter === 1) {
this.props.update();
}
}

_before() {}

_after() {}

_fromCWRP() {
this._forceASYNC();

this.setState({
async: 1
}, this._before);

this.setState({
async: 2
}, this._after);
}

componentWillReceiveProps(nextProps, nextContext) {
this.setState({
counter: this.state.counter + 1
}, this._fromCWRP);
}

render() {
return (
<div>
{`${this.state.async}`}
</div>
);
}
}

render(<HoC run={1} />, container);
render(<HoC run={2} />, container);
// Before micro task runs unmount them
render(null, container);

setTimeout(function () {
// Set state should be called as many times as it was requested
expect(testBeforeBeforeSpy.callCount).toBe(0);
expect(testBeforeAfterSpy.callCount).toBe(0);
expect(testAfterBeforeSpy.callCount).toBe(0);
expect(testAfterAfterSpy.callCount).toBe(0);

done();
}, 20);
Expand Down
8 changes: 5 additions & 3 deletions packages/inferno/src/core/component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,12 @@ export function rerender() {
microTaskPending = false;

while ((component = QUEUE.shift())) {
applyState(component, false);
if (!component.$UN) {
applyState(component, false);

if (component.$QU) {
callSetStateCallbacks(component);
if (component.$QU) {
callSetStateCallbacks(component);
}
}
}
}
Expand Down

0 comments on commit 0e6b243

Please sign in to comment.