Open Andarist opened 2 years ago
I'm not entirely sure I understand.
enzyme doesn't support or endorse snapshot-based testing; it's brittle and leads to folks rubberstamping changes they shouldn't.
The closest we offer is wrapper.debug()
. What does that output in this example, and why is that undesirable?
enzyme doesn't support or endorse snapshot-based testing; it's brittle and leads to folks rubberstamping changes they shouldn't.
I agree with this sentiment but some of our (Emotion) users depend on that.
The closest we offer is wrapper.debug(). What does that output in this example, and why is that undesirable?
wrapper.debug()
output:
<Fragment>
<Noop />
<Greeting className="css-6ph63u-cases">
Hello
</Greeting>
</Fragment>
It seems that it is basically stringifying the rendered nodes of the shallow wrapper. I'm after what is called "unrendered" in the codebase here.
I'm not entirely sure I understand.
The problem is that with the upcoming React 18 we must wrap our elements in a Fragment to keep the number of siblings and stuff matching between the server and the client. This avoids useId
hydration mismatches in scenarios involving Emotion.
However, this affects serialized snapshots because we add additional elements to the rendered React tree. Those elements are purely internal and ideally, they would be hidden from the serialized snapshots. In the past, this wasn't an issue (even though there was a Fragment there and stuff on the server) because the snapshots are usually captured in the JSDOM environment (so where we previously were avoiding rendering those "internal" elements).
The entire point of snapshot testing is to expose all those internals. Why would it be ideal to provide a fictional snapshot? Doesn't that defeat the entire alleged purpose of having snapshots in the first place?
(also, note that enzyme doesn't yet support react 17 at all yet, so "enzyme and react 18" problems are a very long way off, since all enzyme users are supposed to stay on react 16 until there's official support)
The entire point of snapshot testing is to expose all those internals. Why would it be ideal to provide a fictional snapshot? Doesn't that defeat the entire alleged purpose of having snapshots in the first place?
In this situation, it seems to me that people won't care about the library internals - they should be focused on their code and adding the fragment wrapper etc to the snapshot is just unnecessary noise.
(also, note that enzyme doesn't yet support react 17 at all yet, so "enzyme and react 18" problems are a very long way off, since all enzyme users are supposed to stay on react 16 until there's official support)
We are currently supporting >=16.8.0 || ^17 || ^18
. But we don't intend to special-case the runtime code for a specific React version. The stuff that has been added to improve the compatibility with React 18 works just fine with React 16 and React 17. So it's not as much about React 18 as it is about hiding those "internal" elements from the snapshot. Note that this is already possible for any element within the rendered tree - but the "unrendered" element is not a part of it, so the "root" cannot be handled in the same way. I could implement some heuristics to detect this situation but it would be brittle and possibly could result in false positives. That's why I would be interested in having a first-class API to check what has been used to render the given enzyme wrapper.
While my use case might not be that appealing to you, this could be used for different use cases as well. I'm not asking about something specific to the whole serialization stuff that I'm dealing with - it's just some background on why I'm requesting it.
I understand that you might be hesitant about adding this - just trying to make a case for myself here :wink:
Totally fine to ask :-) I'm still just trying to understand the value.
The root element is the thing you're testing. Since you're testing it, you are the author of it, you already know all the internal implementation details, and all of those details are relevant. It sounds like this is a scenario where someone's making a snapshot of a component they didn't author, which is confusing to me.
It sounds like this is a scenario where someone's making a snapshot of a component they didn't author, which is confusing to me.
This is the case here. With smth like this:
/** @jsx jsx */
<div css={{ color: 'red'}} />
the user is thinking about the div
and its styles - those are the details of the implementation that they care about. Because a css
prop is used here and thanks to a custom JSX pragma, in reality, a wrapper component is being rendered here though that renders that div
under the hood.
The content of the wrapper component is irrelevant to the user though and, IMHO, there would be little to no value in printing it in the snapshot.
A custom jsx pragma means you’re not using react rendering - which means you’d need a custom enzyme adapter. The react adapters only work with the react jsx transforms.
Emotion is using jsx pragma with React - we forward calls to React.createElement: https://github.com/emotion-js/emotion/blob/f046ae40bcae24400068311690a94ba2dbf20344/packages/react/src/jsx.js
So we dont quite need custom enzyme adapters at all.
right - but emotion's transform does different things than react's does, which is why you're seeing undesirable results in the enzyme debug tree. The only reason you wouldn't need a custom adapter is if the custom transform didn't do anything, which would defeat its purpose.
Hm, I'm not sure if I totally understand the point here. Maybe we could provide a custom adapter and thus gain control over this. It seems like a lot of work for a very little gain though - and also a problem for the users because we'd have to educate them to use that custom adapter over the "default" one. Emotion works OK with the regular adapter just fine - functional-wise. There is just this minor issue with the serialization output. We can actually handle this by using some heuristics - I would just prefer not to rely on guessing there and to be able to implement a straightforward logic.
Right, I do see the difficulty you're in.
I suppose we could come up with a way to configure the standard adapters with a custom jsx transform - but then your users would still need to configure their adapters with something like import { emotionEnzyme } from 'emotion'; enzyme.configure({ adapter, ...emotionEnzyme });
.
Certainly changing the default tree representation wouldn't be feasible. Exposing the internal UNRENDERED value is doable, but I'm very hesitant to do it prior to having working React 17 and 18 adapters, because i have no idea what kinds of internal changes will be needed for enzyme to support react 18.
Well, I've raised this mostly because I had a need for this and this would make my life slightly easier. It isn't something I'm desperate for (I've also implemented an ugly, ugly hack to make this work for me) so I'm OK with this just sitting around for now.
Certainly changing the default tree representation wouldn't be feasible. Exposing the internal UNRENDERED value is doable, but I'm very hesitant to do it prior to having working React 17 and 18 adapters, because i have no idea what kinds of internal changes will be needed for enzyme to support react 18.
Fair game. If you ever get to work on those you might have this ticket back in your head - if you forget about it, that is fine too 😉
Sounds good :-) Happy to leave this open, and we can revisit it once there's a React 17 and 18 adapter.
Is your feature request related to a problem? Please describe.
For serialization purposes (like when working with jest serializer) I'd like to omit some internal elements rendered by component:
Note that in the context of the serializer plugin I only have access to the
ShallowWrapper
's instance here. This wrapper renders smth like:And this is what I have direct access to on the
wrapper
. I would really like to serialize this as:To avoid handling this in a false-positive manner I really need to know what element has led to this render. This is available as "unrendered" internally but this is not exposed publicly.
Describe the solution you'd like
It would be sufficient if you'd be open to adding a method like
unrendered
(or similar) to the API.Describe alternatives you've considered
N/A
Additional context N/A