From 02aafb516297b19cd62cb2d25f483b6a25fed800 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 10 Jun 2015 18:33:59 -0700 Subject: [PATCH] Make setState in render a warning, not an invariant We keep track of the fact that something is rendering for a bunch of warnings. (ReactCurrentOwner.current !== null) Once we get rid of owner for string refs, I'll convert those to something like "isRendering" instead. The interesting part is that feature `__DEV__` only. It is only used for warnings. Except for this case. This means that we can get rid of the special case for the isRendering stack on in prod. --- .../shared/reconciler/ReactUpdateQueue.js | 2 +- .../__tests__/ReactCompositeComponent-test.js | 43 +++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/src/renderers/shared/reconciler/ReactUpdateQueue.js b/src/renderers/shared/reconciler/ReactUpdateQueue.js index d293bc9bc3..73850a7cd5 100644 --- a/src/renderers/shared/reconciler/ReactUpdateQueue.js +++ b/src/renderers/shared/reconciler/ReactUpdateQueue.js @@ -32,7 +32,7 @@ function enqueueUpdate(internalInstance) { } function getInternalInstanceReadyForUpdate(publicInstance, callerName) { - invariant( + warning( ReactCurrentOwner.current == null, '%s(...): Cannot update during an existing state transition ' + '(such as within `render`). Render methods should be a pure function ' + diff --git a/src/renderers/shared/reconciler/__tests__/ReactCompositeComponent-test.js b/src/renderers/shared/reconciler/__tests__/ReactCompositeComponent-test.js index c23ea8c98f..1565aba035 100644 --- a/src/renderers/shared/reconciler/__tests__/ReactCompositeComponent-test.js +++ b/src/renderers/shared/reconciler/__tests__/ReactCompositeComponent-test.js @@ -346,6 +346,49 @@ describe('ReactCompositeComponent', function() { expect(cbCalled).toBe(false); }); + + it('should warn about `setState` in render', function() { + var container = document.createElement('div'); + + var renderedState = -1; + + var Component = React.createClass({ + getInitialState: function() { + return {value: 0}; + }, + render: function() { + renderedState = this.state.value; + if (this.state.value === 0) { + this.setState({ value: 1 }); + } + return
; + }, + }); + + expect(console.error.calls.length).toBe(0); + + var instance = React.render(, container); + + expect(console.error.calls.length).toBe(1); + expect(console.error.argsForCall[0][0]).toBe( + 'Warning: setState(...): Cannot update during an existing state ' + + 'transition (such as within `render`). Render methods should be a pure ' + + 'function of props and state.' + ); + + // The setState call is queued but the result isn't immediately available + // during this pass. It is queued up but not flushed. The behavior is more + // or less undefined. + expect(renderedState).toBe(0); + expect(instance.state.value).toBe(0); + + // Forcing a rerender anywhere will cause the update to happen. + var instance2 = React.render(, container); + expect(instance).toBe(instance2); + expect(renderedState).toBe(1); + expect(instance2.state.value).toBe(1); + }); + it('should not allow `setProps` on unmounted components', function() { var container = document.createElement('div'); document.body.appendChild(container);