@samitouri / QOS-React / commits / 8dfdac6780

ComponentWillUnmount should only ever be invoked once (#6613)

Jim committed Apr 25, 2016 at 18:06 UTC 8dfdac67803d8dd50fb614d6a115e68c9dc8efe6
2 files changed +46 -1
src/renderers/shared/reconciler/ReactCompositeComponent.js
+5 -1
@@ -132,6 +132,9 @@ var ReactCompositeComponentMixin = {
132
133 // See ReactUpdates and ReactUpdateQueue.
134 this._pendingCallbacks = null;
135 +
136 + // ComponentWillUnmount shall only be called once
137 + this._calledComponentWillUnmount = false;
138 },
139
140 /**
@@ -416,7 +419,8 @@ var ReactCompositeComponentMixin = {
419 }
420 var inst = this._instance;
421
419 - if (inst.componentWillUnmount) {
422 + if (inst.componentWillUnmount && !inst._calledComponentWillUnmount) {
423 + inst._calledComponentWillUnmount = true;
424 if (safely) {
425 var name = this.getName() + '.componentWillUnmount()';
426 ReactErrorUtils.invokeGuardedCallback(name, inst.componentWillUnmount.bind(inst));
src/renderers/shared/reconciler/__tests__/ReactCompositeComponent-test.js
+41
@@ -1303,4 +1303,45 @@ describe('ReactCompositeComponent', function() {
1303
1304 });
1305
1306 + it('should only call componentWillUnmount once', function() {
1307 + var app;
1308 + var count = 0;
1309 +
1310 + class App extends React.Component {
1311 + render() {
1312 + if (this.props.stage === 1) {
1313 + return <UnunmountableComponent />;
1314 + } else {
1315 + return null;
1316 + }
1317 + }
1318 + }
1319 +
1320 + class UnunmountableComponent extends React.Component {
1321 + componentWillUnmount() {
1322 + app.setState({});
1323 + count++;
1324 + throw Error('always fails');
1325 + }
1326 +
1327 + render() {
1328 + return <div>Hello {this.props.name}</div>;
1329 + }
1330 + }
1331 +
1332 + var container = document.createElement('div');
1333 +
1334 + var setRef = (ref) => {
1335 + if (ref) {
1336 + app = ref;
1337 + }
1338 + };
1339 +
1340 + expect(function() {
1341 + ReactDOM.render(<App ref={setRef} stage={1} />, container);
1342 + ReactDOM.render(<App ref={setRef} stage={2} />, container);
1343 + }).toThrow();
1344 + expect(count).toBe(1);
1345 + });
1346 +
1347 });