-
Notifications
You must be signed in to change notification settings - Fork 24
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
.output() API Naming Bikeshed #66
Comments
So the current issue is that since we store both the "requested vnode" and the "rendered output" of the node, we have another "depth" that enzyme doesn't. We expose the ability to find the intermediates, which I think is cool, however it leads to this problem, using the new proposed const TestMe = () => <span>So Tested</span>
const context = shallow(<TestMe />);
// this is a little unexpected for some, one would think you'd get the "rendered" version
context.name() === 'TestMe';
// if we try to get at it using .output() we get stuck with a VNode, not a FindWrapper
// so .name() doesn't work anymore
context.output().nodeName === 'span';
// what we want is something like:
context.resolved().name() === 'span'; We just need a good name / concept to describe this phenomenon? |
I was going to suggest So to clarify - one method returns the component-resolved tree in a FindWrapper, the other returns the component-resolved tree as VNodes? |
Correct, one allowing you to hit it with further preact-render-spy API methods for comfort, and the other in case you want to compare / snapshot the VNodes |
another example of when this would be useful: const click = jest.fn();
const TestClick = () => <span onClick={click} />
const context = shallow(<TestClick />);
// doesn't do anything because the click is on the span, not the TestClick
context.simulate('click')
// so we need a way to get to the <span> output! |
I think this will work well with something else I've been thinking about this week. We should split up the FindWrapper methods into "core" and "extended" methods. (I'll be opening a separate issue for discussion on that.) This would let us implement the @gnarf Your example would work with either the current or new API. In either API you need a wrapper containing the |
Mentioned proposed change. #67 |
My earlier comment used the |
To respond a bit more to the naming question, I think the proposed names in the snippets |
We currently have
.output()
which returns a VNode, but the more I look at this, the more I think it might of been a mistake to not return aFindWrapper
instance here. We need a method that can return the FindWrapper version of component output, and we've been talking about it over in #57 PR comments.I wouldn't be entirely against going to a 2.0 and making
.output()
return a FindWrapper, and.nodes()
returning VNodes or something, but if we can come up with a good name foroutputWrapped()
we might not need to break back-compat.The text was updated successfully, but these errors were encountered: