Is your feature request related to a problem? Please describe.
I was frustrated when I had to test a component which contained another one with so-called children as a function (or render prop).
Let's recap for a minute how children-as-a-function works:
class Mouse extends React.Component {
constructor(props) {
super(props);
this.handleMouseMove = this.handleMouseMove.bind(this);
this.state = { x: 0, y: 0 };
}
handleMouseMove(event) {
this.setState({
x: event.clientX,
y: event.clientY
});
}
render() {
return this.props.children(this.state);
}
}
Let's check out this usage:
// App.jsx
const App = ({ someFlag }) => (
<div>
{someFlag && <SomeAlert />}
<Mouse>{ ({ x, y }) =>
<div>
{someFlag && x> 10 && y>10 && <SomeComponent />}
Current mouse position is {x}:{y}
</div>
}</Mouse>
</div>
)
The test for App.jsx can look like (best solution available on the web):
import { shallow } from 'enzyme';
describe('App', () => {
it('renders SomeComponent and SomeAlert when someFlag is true and mouse is at least 10px from the left and top screen borders', () => {
const topComp = shallow(<App someFlag/>);
// topComp has [function] as a child, enzyme don't go further in shallow rendering
// so we need to manually invoke that function
const insideTheMouseComp = shallow(topComp.find(Mouse).prop('children')({x: 11, y: 11}))
//Lets try to lookup for the SomeComponent
expect(insideTheMouseComp.find(SomeComponent).length).toEqual(1);
// Okay, not bad
// Let's try to lookup for the SomeAlert
expect(insideTheMouseComp.find(SomeAlert).length).toEqual(1);
// Oh no! Nothing there!
// That's because SomeAlert is outside of Mouse
// we still can expect(topComp.find(SomeAlert).length).toEqual(1);
// but it looks a bit ugly since we need to have knowledge
// about both Mouse component implementation
// and where it is located in App
//
});
});
Also what would happen if there are nested render-children-prop components,e.g.
<Mouse>{
({x,y}) => <KeyPressed>{
(isKeyPressed) => isKeyPressed ? <SomeComponent/> : <SomeAnotherComponent/>
}</KeyPressed>
}</Mouse>
If you are to refactor this file (e.g. swap Mouse and KeyPressed components positions), you will need to rewrite all tests regarding that .find(...).prop('children')(...)
to match the new structure.
Bad.
How about declaratively create a mocks for the children-as-a-function component?
describe('App', () => {
it('renders SomeComponent and SomeAlert when someFlag is true and mouse is at least 10px from the left and top screen borders', () => {
const MockedMouse = ({ children }) => children({ x: 11, y: 11 });
jest.mock('./Mouse', MockedMouse);
const justComp = shallow(<App someFlag />);
expect(justComp.find(SomeComponent).length).toEqual(1);
expect(justComp.find(SomeAlert).length).toEqual(1);
// Looks much cooler, we have single tree having all the info
// But no. We still have that [function] as a child and enzyme not going deeper
// These 2 expects would fail
// It seems we need a solution for this case
})
});
With this approach we still have to know how is Mouse working, but we are free now from the position it has in the tree.
Describe the solution you'd like
packages/enzyme-adapter-utils/src/Utils.js(Line 206):
export function elementToTree(el, recurse = elementToTree) {
...
else if (typeof children !== 'undefined') {
rendered = recurse(children);
}
}
Let's change that to
export function elementToTree(el, recurse = elementToTree) {
...
else if (typeof children !== 'undefined') {
if (typeof children === 'function' && nodeTypeFromType(type) === 'function') {
// Handling chilren-as-a-function component
rendered = recurse(type({ children }));
} else {
rendered = recurse(children);
}
}
Basically, we are saying that if children prop is function and currentElement is just function too (not ClassComponent), we are gonna pass those children to this component.
It's based upon two assumptions:
Describe alternatives you've considered
• Writing a lot of unmaintainable tests
• Not using children-as-a-function
Additional context
This probably should be revisited with upcoming React Hooks.
Also I can provide a PR with these changes, but I thought we should discuss it first.
PS. My first issue, be nice to me please :)
The next release of enzyme already will have a .renderProp() method.
A PR is premature; I'm pretty convinced that shallow rendering should absolutely not invoke a render prop.
.renderProp method doesn't solve the problem of having the inners or the outers of renderProp component in different places. You still have that cognitive overload regarding original structure of components.
You really DON'T want to keep in mind that first I should find a Mouse component then invoke his render function, then i should find another render-prop component and invoke this one. Oh, is it the other way around? *confused* And where should I be looking for this Alert component? *confusedEvenMore*
I think that in your example, if you’re shallow rendering Mouse, and Mouse invokes the render prop, then surely it should (and i believe already does) cause the render prop’s contents to show up in the tree. If the render prop is passed into a deeper component, then naturally it’d show up as a render prop there, and not actually be rendered in the shallow render.
A function child is no different than a non-function child as far as enzyme is concerned. It’s not a primitive concept of react; it’s just a function, in the child position. It’s up to a custom component’s implementation to invoke it and render the result. If a component being shallow rendered does that, great - it will be invoked - but if not (or if a component that is not being shallow rendered invokes it), then it won’t be.
Consider that render props often take arguments, and there’d be no way for enzyme to know what arguments it needs.
But again, the whole point of using render props is to give the App component control over Mouse's state. So, the functional child of Mouse becomes basically part of App component, not the Mouse component.
const App = () => <Mouse>{ (state) => <div>{state}</div>}</Mouse>
In this case i really want to be able to test that div in the App component.
What I'm now thinking is that my proposed way of solving this problem is not good, but maybe we could think of something that will solve this? Maybe there is some way of redesigning .renderProp, so it doesn't just return the inner part, but the whole component with rendered renderProp?
Like
// Finds the first Mouse component and replaces it with
// result of invoking his 'render' prop with arguments provided
// IMPORTANT: returns the same component (well in some sort)
component.renderProp(Mouse, 'render', {x:1, y: 1});
Added:
Or maybe as a option for shallow
shallow(
component,
{ renderProps: Map([
[ Mouse, { render: { x:1, y:1 } } ],
[ KeyPressed, { children: true } ]
])}
)
If you're trying to shallow render App, then you explicitly are not testing Mouse.
I'm saying that if you want to be able to test the render prop's contents, you have to shallow render Mouse - not App.
Magic transformations like this should not be a part of enzyme. Only you could know how to properly call ‘children’ without calling ‘type’, and only you can do it - mock mock, and call it a day.
https://gist.github.com/theKashey/e1a67f6bddb8e079c4b0f74a7d885583#file-mouse-comparison-js
https://hackernoon.com/why-i-always-use-shallow-rendering-a3a50da60942
@theKashey thanks for the tip on using remock - that solved my problem, yet it's a bit unpure solution.
Maybe there should be still a way to let specify some function to be invoked during each step of tree traversing with shallow render? Which will return the part of rendered tree? That will allow much easier mocks, also pure. What would you say to that, @ljharb?
I’m not sure what that would accomplish - in general, explicit mocks and stubs make tests more brittle. What would you hope to achieve with that feature? Can you give a more concrete example?
I'm trying to solve the same problem but with more generalised solution.
Example:
Shallow render component with mocking out Mouse render-prop component:
shallow(<App />, {
onTraverse: (ctx,...rest) =>
(ctx.currentNode.type === Mouse) ?
shallow(ctx.currentNode.children({x:1, y:1})) :
null // null for returning the original node
})
// Will return rendered App component
// with Mouse node replaced
// with result of render-prop call
I’m still not clear on what that is testing.
If you want to test the render prop itself, use the .renderProp API, which will be in the next enzyme release. If you want to test the component rendered by the render prop, do it in that component’s tests.
I have similar issue with enzyme.
For example, I have Link component that implements some magic generation of href prop. Most of time it should render just <a> tag (there is no problem with testability), but sometimes I need to override render logic for component like this:
<Link>
{({href, icon, content}) => (
<MobileLink iconLeft={icon} href={href}>
{content}
</MobileLink/>
)}
</Link>
I want to test this functionality like this:
const children = jest.fn(() => <div />)
const wrapper = shallow(<Link children={children} />)
expect(wrapper.type()).toBe('div')
expect(children).toHaveBeenCalled()
const renderOptions = children.mock.calls[0][0]
expect(renderOptions.href).toBe('https://github.com/airbnb/enzyme/issues/1902')
expect(renderOptions.icon).toBe('warning')
const childrenWrapper = shallow(renderOptions.content)
expect(childrenWrapper.type()).toBe(LinkContent)
But of course this will not work. So every time I run into this issue I need to write little helper:
function shallowOuter(element) {
const Outer = () => element
return shallow(<Outer />)
}
I think this functionality can be shipped with enzyme
enzyme now has a .renderProp API method on wrappers.
@darkartur in your case, if you're shallow-rendering Link, then you're going to make assertions on what Link renders - so you'd still want to render something real, and then assert that's what the wrapper is.
I believe this issue is answered, and that the .renderProp API should cover the majority of use cases here. Please file new issues with additional questions.
Most helpful comment
The next release of enzyme already will have a
.renderProp()method.A PR is premature; I'm pretty convinced that shallow rendering should absolutely not invoke a render prop.