@samitouri / QOS-React-2 / commits / 02aafb5162

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.

Sebastian Markbage committed Jun 10, 2015 at 18:33 UTC 02aafb516297b19cd62cb2d25f483b6a25fed800
2 files changed +44 -1
src/renderers/shared/reconciler/ReactUpdateQueue.js
+1 -1
@@ -32,7 +32,7 @@ function enqueueUpdate(internalInstance) {
32 }
33
34 function getInternalInstanceReadyForUpdate(publicInstance, callerName) {
35 - invariant(
35 + warning(
36 ReactCurrentOwner.current == null,
37 '%s(...): Cannot update during an existing state transition ' +
38 '(such as within `render`). Render methods should be a pure function ' +
src/renderers/shared/reconciler/__tests__/ReactCompositeComponent-test.js
+43
@@ -346,6 +346,49 @@ describe('ReactCompositeComponent', function() {
346 expect(cbCalled).toBe(false);
347 });
348
349 +
350 + it('should warn about `setState` in render', function() {
351 + var container = document.createElement('div');
352 +
353 + var renderedState = -1;
354 +
355 + var Component = React.createClass({
356 + getInitialState: function() {
357 + return {value: 0};
358 + },
359 + render: function() {
360 + renderedState = this.state.value;
361 + if (this.state.value === 0) {
362 + this.setState({ value: 1 });
363 + }
364 + return <div />;
365 + },
366 + });
367 +
368 + expect(console.error.calls.length).toBe(0);
369 +
370 + var instance = React.render(<Component />, container);
371 +
372 + expect(console.error.calls.length).toBe(1);
373 + expect(console.error.argsForCall[0][0]).toBe(
374 + 'Warning: setState(...): Cannot update during an existing state ' +
375 + 'transition (such as within `render`). Render methods should be a pure ' +
376 + 'function of props and state.'
377 + );
378 +
379 + // The setState call is queued but the result isn't immediately available
380 + // during this pass. It is queued up but not flushed. The behavior is more
381 + // or less undefined.
382 + expect(renderedState).toBe(0);
383 + expect(instance.state.value).toBe(0);
384 +
385 + // Forcing a rerender anywhere will cause the update to happen.
386 + var instance2 = React.render(<Component prop={123} />, container);
387 + expect(instance).toBe(instance2);
388 + expect(renderedState).toBe(1);
389 + expect(instance2.state.value).toBe(1);
390 + });
391 +
392 it('should not allow `setProps` on unmounted components', function() {
393 var container = document.createElement('div');
394 document.body.appendChild(container);