Cloning the child of a context Consumer produces confusing warning and error

[cristianbogdan]

Do you want to request a feature or report a bug?
this is a bug, or at least a request for more precise warnings and error messages.

What is the current behavior?

I was cloning children to add some properties and I overlooked that the context Consumer subtree should not be cloned...

import React from 'react';
import {render} from 'react-dom';

const { Provider, Consumer} = React.createContext();

const Comp = ({children})=> <Provider>{cloneKids(children)}</Provider>;

const cloneKids=(children)=>React.Children.map(children, child =>
					       React.cloneElement(child, child.props,
								  child.props.children&&
								  cloneKids(child.props.children)));
render(
	<Comp><Consumer>{console.log}</Consumer></Comp>,
    document.getElementById('root')
);

The code produces the warning and error introduced with #12241

Warning: A context consumer was rendered with multiple children, or a child that isn't a function. A context consumer expects a single child that is a function. If you did pass a function, make sure there is no trailing or leading whitespace around it.

and (even more confusing)

TypeError: render is not a function

What is the expected behavior?

Maybe React.cloneElement should not attempt to clone functions? Whatever it does, the result is not a function.

The warning part "a child that isn't a function" should be separated from the other warnings. There can't be multiple children and one child that is not a function at the same time, so a more precise warning can be issued.

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?

Tested with react 16.3.0 in Stackblitz/Chrome 65 and react 16.3.2 in Chrome 65 and Firefox 59

[gaearon]

The issue isn't with cloneElement (it has no special logic for context), but that you're passing props.children (which is a function in this case) to Children.map() (which doesn't expect functions). Children.map only works with regular React nodes.

Perhaps Children.map could show a warning when it encounters something that isn't a React node.

In either case deeply cloning React trees like this sounds like you're trying to do something React wasn't designed for. Why do you need this?

[cristianbogdan]

Thanks!

I am delaying the evaluation of a few child properties until the parent is ready to provide context for their evaluation. For example:

<Parent ><Child prefix-prop="string expression"></Parent>
(the expression language is not javascript and it is evaluated remotely. I do not control Child, and there can be any number of children).

The child becomes internally something like:

<Child prop={evaluate("string expression")} />

To accomplish that, the parent visits the children and transforms the "prefix-prop" property containing the expression, into the "prop" property, which contains the expression value. Cloning is the only approach I found to alter component properties. Of course context Consumers should not be cloned, there are no properties to alter there.

I know that I should use function-as-child or render functions or similar (to delay evaluation) but I am designing a prototyping library and my users (who come from HTML and JSP, and are often novices beyond HTML and the expression language) will most probably not appreciate this notation:

<Parent>{ context=> <Child prop={context.evaluate(x)} />}</Parent>

(besides, if they forget a space just before the function, they will be confused by #12689)

and they will not quickly comprehend this form either:
<Parent display={ context=> <Child prop={context.evaluate(x)} />} />

Context consumers also use function-as-child so they have the same problem.

This is very early work so I am still considering options, but I somehow want to arrive at a clean parent-child notation like in the first quote. Probably I should write a babel plugin that transforms the "clean" notation to a functional one.

But maybe I'm missing something so I'd appreciate any input. I would really like my users to leave JSP and adopt React but if the notation is too cryptical, this will not happen...

[cristianbogdan]

Another idea I'm toying with is a functional parent, probably returning Component (some kind of HOC)

({context})=>
context.parent("context expression",
     context2=><Child prop={context2.evaluate("sub-expression")} context={context2} />)

where Child can contain again context.parent() calls, and so on.

Of course parent() could use a context Consumer in the render() of the component it returns, so the context prop can be dropped:

()=>
parent("context expression",
     context2=><Child prop={context2.evaluate("sub-expression")} />)

The problem here is that parent() being a Component can access the React context, but evaluate() cannot (to my knowledge) access the React context unless it returns a Component

which is not possible when you are actually trying to set a property.

Therefore I believe that accessing the React context from outside components would be ideal. Something like

React.getContext(Consumer, data=> ...)

If I remember right, when struggling with this, I tried to invoke Consumer.render() manually using the test framework, but I didn't get anything meaningful.

PS: The first notation above may seem cryptical but it's actually quite familiar with the current JSP form so I think that I can sell it to the users :)

<context:parent ctx="context-expression">
   HTML...<context:evaluate expr="sub-expression" />...HTML
</context:parent>

[gaearon]

Going to close as this doesn't seem like it comes up often.