@samitouri / QOS-React / commits / c7f1abade5

Stack and Fiber warn about missing getChildContext method

Previous (probably unintentional) behavior of Stack was to allow components to define childContextTypes without also supplying a getChildContext property. This PR updates Fiber to (temporarily) mimic that behavior. It also adds warning messages to both Fiber and Stack (along with a test). For the time being, Fiber components with a missing getChildContext method will return the parent context as-is, after warning.

Brian Vaughn committed Jan 10, 2017 at 13:44 UTC c7f1abade518c70a6386c15279054c2ed1abf2e8
5 files changed +95 -1
scripts/fiber/tests-passing.txt
+2
@@ -157,6 +157,8 @@ src/isomorphic/classic/__tests__/ReactContextValidator-test.js
157 * should pass next context to lifecycles
158 * should check context types
159 * should check child context types
160 +* should warn (but not error) if getChildContext method is missing
161 +* should pass parent context if getChildContext method is missing
162
163 src/isomorphic/classic/class/__tests__/ReactBind-test.js
164 * Holds reference to instance
src/isomorphic/classic/__tests__/ReactContextValidator-test.js
+67
@@ -289,4 +289,71 @@ describe('ReactContextValidator', () => {
289 expectDev(console.error.calls.count()).toBe(2);
290 });
291
292 + // TODO (bvaughn) Remove this test and the associated behavior in the future.
293 + // It has only been added in Fiber to match the (unintentional) behavior in Stack.
294 + it('should warn (but not error) if getChildContext method is missing', () => {
295 + spyOn(console, 'error');
296 +
297 + var MyComponent = React.createClass({
298 + childContextTypes: {
299 + foo: React.PropTypes.string.isRequired,
300 + },
301 +
302 + render: function() {
303 + return <div />;
304 + },
305 + });
306 +
307 + ReactTestUtils.renderIntoDocument(<MyComponent/>);
308 + expectDev(console.error.calls.count()).toBe(1);
309 + expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe(
310 + 'Warning: getChildContext() is not defined for MyComponent'
311 + );
312 + });
313 +
314 + // TODO (bvaughn) Remove this test and the associated behavior in the future.
315 + // It has only been added in Fiber to match the (unintentional) behavior in Stack.
316 + it('should pass parent context if getChildContext method is missing', () => {
317 + spyOn(console, 'error');
318 +
319 + var ParentContextProvider = React.createClass({
320 + childContextTypes: {
321 + foo: React.PropTypes.number,
322 + },
323 + getChildContext: function() {
324 + return {
325 + foo: 'FOO',
326 + };
327 + },
328 + render: function() {
329 + return <MiddleMissingContext />;
330 + },
331 + });
332 +
333 + var MiddleMissingContext = React.createClass({
334 + childContextTypes: {
335 + bar: React.PropTypes.string.isRequired,
336 + },
337 + render: function() {
338 + return <ChildContextConsumer />;
339 + },
340 + });
341 +
342 + var childContext;
343 + var ChildContextConsumer = React.createClass({
344 + contextTypes: {
345 + bar: React.PropTypes.string.isRequired,
346 + foo: React.PropTypes.string.isRequired,
347 + },
348 + render: function() {
349 + childContext = this.context;
350 + return <div />;
351 + },
352 + });
353 +
354 + ReactTestUtils.renderIntoDocument(<ParentContextProvider/>);
355 + expect(childContext.bar).toBeUndefined();
356 + expect(childContext.foo).toBe('FOO');
357 + });
358 +
359 });
src/renderers/shared/fiber/ReactFiberContext.js
+13
@@ -17,6 +17,7 @@ import type { StackCursor } from 'ReactFiberStack';
17
18 var emptyObject = require('emptyObject');
19 var invariant = require('invariant');
20 +var warning = require('warning');
21 var {
22 getComponentName,
23 isFiberMounted,
@@ -141,6 +142,18 @@ exports.pushTopLevelContextObject = function(fiber : Fiber, context : Object, di
142 function processChildContext(fiber : Fiber, parentContext : Object, isReconciling : boolean): Object {
143 const instance = fiber.stateNode;
144 const childContextTypes = fiber.type.childContextTypes;
145 +
146 + // TODO (bvaughn) Replace this behavior with an invariant() in the future.
147 + // It has only been added in Fiber to match the (unintentional) behavior in Stack.
148 + if (typeof instance.getChildContext !== 'function') {
149 + warning(
150 + false,
151 + 'getChildContext() is not defined for %s',
152 + getComponentName(fiber),
153 + );
154 + return parentContext;
155 + }
156 +
157 const childContext = instance.getChildContext();
158 for (let contextKey in childContext) {
159 invariant(
src/renderers/shared/shared/__tests__/ReactStatelessComponent-test.js
+5 -1
@@ -118,11 +118,15 @@ describe('ReactStatelessComponent', () => {
118
119 ReactDOM.render(<StatelessComponentWithChildContext name="A" />, container);
120
121 - expectDev(console.error.calls.count()).toBe(1);
121 + expectDev(console.error.calls.count()).toBe(2);
122 expectDev(console.error.calls.argsFor(0)[0]).toContain(
123 'StatelessComponentWithChildContext(...): childContextTypes cannot ' +
124 'be defined on a functional component.'
125 );
126 + expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(1)[0])).toBe(
127 + 'Warning: getChildContext() is not defined for ' +
128 + 'StatelessComponentWithChildContext'
129 + );
130 });
131
132 if (!ReactDOMFeatureFlags.useFiber) {
src/renderers/shared/stack/reconciler/ReactCompositeComponent.js
+8
@@ -698,6 +698,14 @@ var ReactCompositeComponent = {
698 );
699 }
700 return Object.assign({}, currentContext, childContext);
701 + } else {
702 + if (__DEV__) {
703 + warning(
704 + !Component.childContextTypes,
705 + 'getChildContext() is not defined for %s',
706 + this.getName(),
707 + );
708 + }
709 }
710 return currentContext;
711 },