React-apollo: getDataFromTree incorrectly handling React Context.Consumer

[jsantha]

Intended outcome:

Walking down the tree via getDataFromTree correctly handles Context.Consumer and continue walking down the whole component tree.

Actual outcome:

getDataFromTree incorrectly handles Context.Consumer component and consider it as a Context.Provider. That means that render prop children function of Context.Consumer is not called but passed down to walkTree which is unable to handle plain function and consider tree to be traversed. (https://github.com/jsantha/getDataFromTreeReactContextBug/blob/master/getDataFromTreeDebug.ts#L156)

React version provided in demo reports corresponding warning: Warning: Rendering <Context.Consumer.Consumer> is not supported and will be removed in a future major release. Did you mean to render <Context.Consumer> instead?

How to reproduce the issue:

Demo with debugged current version of getDataFromTree: https://github.com/jsantha/getDataFromTreeReactContextBug

Version

• [email protected]
• [email protected]

[disintegrator]

We are also seeing this issue after updating to React 16.6.0.

Is it possible for getDataFromTree to use react-is to avoid peeking into react internals? There is an example in that library for checking if an element is a context provider/consumer: https://www.npmjs.com/package/react-is#context

[3axap4eHko]

The issue is here https://github.com/apollographql/react-apollo/blob/master/src/getDataFromTree.ts#L70

if (isReactElement(element)) {

when walkTree is inside of Consumer, the child of consumer is a function, but walkTree expects a react element therefore it returns false and falling back to non react element branch visitor

else if (typeof element === 'string' || typeof element === 'number') {
            visitor(element, null, newContext, context);
        }

but type of the element is a function and this visitor also skipped
Maybe it should have one more condition like this

if (isReactElement(element)) {
  // react element logic
} else if (typeof element === 'function') { 
  // child as a function logic
else if (typeof element === 'string' || typeof element === 'number') {
  // non react logic
}

[JoviDeCroock]

I don't know if i truely understand the problem thoroughly since when i run it i can see the value etc.

[jsantha]

@JoviDeCroock you can see the value because it's correctly rendered by ReactDOM. The issue here is with getDataFromTree which does not traverse whole tree but finishes at Consumer rendering (see console).

[JoviDeCroock]

I think i solved it, mind giving my branch a try? :)

[jsantha]

I have tried to implement your branch to our project with more complicated component tree. Working as expected with react 16.6 without any warning or error. Context API is handled correctly now. As @disintegrator already mentioned, react-is seems like more convenient solution. Good job :)

[JoviDeCroock]

Well there's a more pragmatic solution located here: https://github.com/apollographql/react-apollo/pull/2533

[benjamn]

I can confirm that the reproduction @jsantha provided is fixed by [email protected] (if you just import { getDataFromTree } from "react-apollo"). Thanks all!

[dejay-at-work]

I'm curious, the changelog mentions that for react-apollo 2.1.6, "Adjust getDataFromTree to properly traverse React 16.3's context API provider/consumer approach. @marnusw in #1978". This is the only mention of React Consumer in the changelog.

Does it make sense to update the changelog to mention that React Consumer behavior is "really" fixed in 2.3.1?