feat(formatting): render children as function #338
Conversation
|
I will fix the flow errors. But failing test is not related to this PR and local |
|
Yep I don't get why it's failing on travis only |
|
cc @Spy-Seth maybe? |
|
Do you use the same node version as your travis build ? |
|
The ref/key order issue is fixed by #340. You should rebase your PR on top master 😃 |
There was a problem hiding this comment.
Thanks @adammockor for this contribution. This feature is really missing in react-element-to-jsx-string
For now, I have some concern about the current proposal to implement it. It's a good start to implement the feature 😺
| @@ -71,6 +72,14 @@ const parseReactElement = ( | |||
| .filter(onlyMeaningfulChildren) | |||
| .map(child => parseReactElement(child, options)); | |||
|
|
|||
| if (typeof element.props.children === 'function') { | |||
| const functionChildrens = parseReactElement( | |||
| element.props.children(), | |||
There was a problem hiding this comment.
I'm worried about the need to call the children function to be able to format it's result. This could have unwanted side effect in the user land.
For example:
reactElementToJSXString(
<div>
{() => {
console.log('Should not be logged');
return <div>Hello World</div>;
}}
</div>,
{
showFunctions: true,
}
)| ); | ||
| childrens.push(createReactFunctionTreeNode([functionChildrens])); | ||
| } | ||
|
|
There was a problem hiding this comment.
One gotcha with this detection method is is case of mixed children (or multiple function as a children):
// Multiple functions as child
reactElementToJSXString(
<div>
{() => <div>Hello Foo</div>}
{() => <div>Hello Bar</div>}
</div>,
{
showFunctions: true,
}
)
// Mixed content
reactElementToJSXString(
<div>
{() => <div>Hello Foo</div>}
<span>Some other children</span>
</div>,
{
showFunctions: true,
}
)I know this is should not be frequent or maybe event imaginable but the JSX allow it 🤷♂️
I do some try with you PR, it's a shame that React.Children filters out the function children. We do not want to re-implement it to be able to conserve function in our parsing.
This is copy paste of #317 since it doesn't get merged due commitlint errors. I fixed that. Thanks @DavidBabel for implementing this. I can provide support to this PR if needed.