From 0e6b2438bac4539a97c4debe97cbda8c07703b28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sampo=20Kivist=C3=B6?= Date: Mon, 2 Mar 2020 08:07:35 +0200 Subject: [PATCH] Fixes an issue where setState callbacks trigger even when component was unmounted --- package.json | 12 +- .../inferno/__tests__/async-setstate.spec.jsx | 147 +++++++++++++++++- packages/inferno/src/core/component.ts | 8 +- 3 files changed, 156 insertions(+), 11 deletions(-) diff --git a/package.json b/package.json index d5e3c4f17..c5416ab64 100644 --- a/package.json +++ b/package.json @@ -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", @@ -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", @@ -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", @@ -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" } } diff --git a/packages/inferno/__tests__/async-setstate.spec.jsx b/packages/inferno/__tests__/async-setstate.spec.jsx index 78235c992..847e7e2a9 100644 --- a/packages/inferno/__tests__/async-setstate.spec.jsx +++ b/packages/inferno/__tests__/async-setstate.spec.jsx @@ -137,7 +137,7 @@ describe('Async set state issue', () => { expect(_failureCreatorCBRequested).toBe(4); expect(_justBecauseCBRequested).toBe(4); - expect(container.innerHTML).toBe('
first 2 2 true true
second 2 2 true true
') + expect(container.innerHTML).toBe('
first 2 2 true true
second 2 2 true true
'); done(); }, 20); @@ -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('
2
2
') + expect(container.innerHTML).toBe('
2
2
'); + + 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 ( +
+ + +
+ ); + } + } + + 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 ( +
+ {`${this.state.async}`} +
+ ); + } + } + + 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 ( +
+ {`${this.state.async}`} +
+ ); + } + } + + render(, container); + render(, 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); diff --git a/packages/inferno/src/core/component.ts b/packages/inferno/src/core/component.ts index f22d53496..8dfbf1997 100644 --- a/packages/inferno/src/core/component.ts +++ b/packages/inferno/src/core/component.ts @@ -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); + } } } }