Change DEV-only invariants to be warnings (#11630)
* Change DEV-only invariant about instance.state type to a warning * Change DEV-only invariant childContextTypes check to a warning
Dan Abramov committed
Nov 22, 2017 at 18:58 UTC
913a125ad51777ffec91eedefa03c534a7590395
6 files changed
+69
-71
packages/react-dom/src/__tests__/ReactDOMServerIntegration-test.js
+20
-19
@@ -2671,26 +2671,27 @@ describe('ReactDOMServerIntegration', () => {
2671
},
2672
);
2673
2674
- // TODO: this being DEV-only is likely a bug.
2675
- // https://github.com/facebook/react/issues/11618
2676
- if (__DEV__) {
2677
- itThrowsWhenRendering(
2678
- 'if getChildContext exists without childContextTypes',
2679
- render => {
2680
- class MyComponent extends React.Component {
2681
- render() {
2682
- return <div />;
2683
- }
2684
- getChildContext() {
2685
- return {foo: 'bar'};
2686
- }
2674
+ itRenders(
2675
+ 'if getChildContext exists but childContextTypes is missing with a warning',
2676
+ async render => {
2677
+ function HopefulChild(props, context) {
2678
+ return context.foo || 'nope';
2679
+ }
2680
+ HopefulChild.contextTypes = {
2681
+ foo: PropTypes.string,
2682
+ };
2683
+ class ForgetfulParent extends React.Component {
2684
+ render() {
2685
+ return <HopefulChild />;
2686
}
2688
- return render(<MyComponent />);
2689
- },
2690
- 'MyComponent.getChildContext(): childContextTypes must be defined ' +
2691
- 'in order to use getChildContext().',
2692
- );
2693
- }
2687
+ getChildContext() {
2688
+ return {foo: 'bar'};
2689
+ }
2690
+ }
2691
+ const e = await render(<ForgetfulParent />, 1);
2692
+ expect(e.textContent).toBe('nope');
2693
+ },
2694
+ );
2695
2696
itThrowsWhenRendering(
2697
'if getChildContext returns a value not in childContextTypes',
packages/react-dom/src/server/ReactPartialRenderer.js
+15
-12
@@ -456,19 +456,22 @@ function resolve(
456
var childContext;
457
if (typeof inst.getChildContext === 'function') {
458
var childContextTypes = Component.childContextTypes;
459
- invariant(
460
- typeof childContextTypes === 'object',
461
- '%s.getChildContext(): childContextTypes must be defined in order to ' +
462
- 'use getChildContext().',
463
- getComponentName(Component) || 'Unknown',
464
- );
465
- childContext = inst.getChildContext();
466
- for (let contextKey in childContext) {
467
- invariant(
468
- contextKey in childContextTypes,
469
- '%s.getChildContext(): key "%s" is not defined in childContextTypes.',
459
+ if (typeof childContextTypes === 'object') {
460
+ childContext = inst.getChildContext();
461
+ for (let contextKey in childContext) {
462
+ invariant(
463
+ contextKey in childContextTypes,
464
+ '%s.getChildContext(): key "%s" is not defined in childContextTypes.',
465
+ getComponentName(Component) || 'Unknown',
466
+ contextKey,
467
+ );
468
+ }
469
+ } else {
470
+ warning(
471
+ false,
472
+ '%s.getChildContext(): childContextTypes must be defined in order to ' +
473
+ 'use getChildContext().',
474
getComponentName(Component) || 'Unknown',
471
- contextKey,
475
);
476
}
477
}
packages/react-reconciler/src/ReactFiberClassComponent.js
+2
-2
@@ -328,14 +328,14 @@ export default function(
328
329
const state = instance.state;
330
if (state && (typeof state !== 'object' || isArray(state))) {
331
- invariant(
331
+ warning(
332
false,
333
'%s.state: must be set to an object or null',
334
getComponentName(workInProgress),
335
);
336
}
337
if (typeof instance.getChildContext === 'function') {
338
- invariant(
338
+ warning(
339
typeof workInProgress.type.childContextTypes === 'object',
340
'%s.getChildContext(): childContextTypes must be defined in order to ' +
341
'use getChildContext().',
packages/react/src/__tests__/ReactCoffeeScriptClass-test.coffee
+8
-12
@@ -149,7 +149,9 @@ describe 'ReactCoffeeScriptClass', ->
149
expect(renderCount).toBe 1
150
undefined
151
152
- it 'should throw with non-object in the initial state property', ->
152
+ it 'should warn with non-object in the initial state property', ->
153
+ spyOnDev console, 'error'
154
+
155
[['an array'], 'a string', 1234].forEach (state) ->
156
class Foo extends React.Component
157
constructor: ->
@@ -158,20 +160,14 @@ describe 'ReactCoffeeScriptClass', ->
160
render: ->
161
span()
162
163
+ test React.createElement(Foo), 'SPAN', ''
164
if __DEV__
162
- expect(->
163
- test React.createElement(Foo), 'SPAN', ''
164
- ).toThrowError(
165
+ expect(console.error.calls.count()).toBe 1
166
+ expect(console.error.calls.argsFor(0)[0]).toContain(
167
'Foo.state: must be set to an object or null'
168
)
167
- else
168
- # This is a difference between development and production.
169
- # I'm not sure if this is intentional, as generally we avoid this.
170
- # TODO: investigate if this was intentional or an oversight.
171
- # https://github.com/facebook/react/issues/11618
172
- expect(->
173
- test React.createElement(Foo), 'SPAN', ''
174
- ).not.toThrowError()
169
+ console.error.calls.reset()
170
+
171
undefined
172
173
it 'should render with null in the initial state property', ->
packages/react/src/__tests__/ReactES6Class-test.js
+6
-8
@@ -163,7 +163,8 @@ describe('ReactES6Class', () => {
163
expect(renderCount).toBe(1);
164
});
165
166
- it('should throw with non-object in the initial state property', () => {
166
+ it('should warn with non-object in the initial state property', () => {
167
+ spyOnDev(console, 'error');
168
[['an array'], 'a string', 1234].forEach(function(state) {
169
class Foo extends React.Component {
170
constructor() {
@@ -174,16 +175,13 @@ describe('ReactES6Class', () => {
175
return <span />;
176
}
177
}
178
+ test(<Foo />, 'SPAN', '');
179
if (__DEV__) {
178
- expect(() => test(<Foo />, 'SPAN', '')).toThrowError(
180
+ expect(console.error.calls.count()).toBe(1);
181
+ expect(console.error.calls.argsFor(0)[0]).toContain(
182
'Foo.state: must be set to an object or null',
183
);
181
- } else {
182
- // This is a difference between development and production.
183
- // I'm not sure if this is intentional, as generally we avoid this.
184
- // TODO: investigate if this was intentional or an oversight.
185
- // https://github.com/facebook/react/issues/11618
186
- expect(() => test(<Foo />, 'SPAN', '')).not.toThrowError();
184
+ console.error.calls.reset();
185
}
186
});
187
});
packages/react/src/__tests__/ReactTypeScriptClass-test.ts
+18
-18
@@ -361,31 +361,31 @@ describe('ReactTypeScriptClass', function() {
361
expect(renderCount).toBe(1);
362
});
363
364
- it('should throw with non-object in the initial state property', function() {
364
+ it('should warn with non-object in the initial state property', function() {
365
+ spyOnDev(console, 'error');
366
+ test(React.createElement(ArrayState), 'SPAN', '');
367
if (__DEV__) {
366
- expect(() => test(React.createElement(ArrayState), 'SPAN', ''))
367
- .toThrowError(
368
+ expect((<any>console.error).calls.count()).toBe(1);
369
+ expect((<any>console.error).calls.argsFor(0)[0]).toContain(
370
'ArrayState.state: must be set to an object or null'
371
);
370
- expect(() => test(React.createElement(StringState), 'SPAN', ''))
371
- .toThrowError(
372
+ (<any>console.error).calls.reset()
373
+ }
374
+ test(React.createElement(StringState), 'SPAN', '');
375
+ if (__DEV__) {
376
+ expect((<any>console.error).calls.count()).toBe(1);
377
+ expect((<any>console.error).calls.argsFor(0)[0]).toContain(
378
'StringState.state: must be set to an object or null'
379
);
374
- expect(() => test(React.createElement(NumberState), 'SPAN', ''))
375
- .toThrowError(
380
+ (<any>console.error).calls.reset()
381
+ }
382
+ test(React.createElement(NumberState), 'SPAN', '');
383
+ if (__DEV__) {
384
+ expect((<any>console.error).calls.count()).toBe(1);
385
+ expect((<any>console.error).calls.argsFor(0)[0]).toContain(
386
'NumberState.state: must be set to an object or null'
387
);
378
- } else {
379
- // This is a difference between development and production.
380
- // I'm not sure if this is intentional, as generally we avoid this.
381
- // TODO: investigate if this was intentional or an oversight.
382
- // https://github.com/facebook/react/issues/11618
383
- expect(() => test(React.createElement(ArrayState), 'SPAN', ''))
384
- .not.toThrowError();
385
- expect(() => test(React.createElement(StringState), 'SPAN', ''))
386
- .not.toThrowError();
387
- expect(() => test(React.createElement(NumberState), 'SPAN', ''))
388
- .not.toThrowError();
388
+ (<any>console.error).calls.reset()
389
}
390
});
391