Fix enzyme serializer for conditional styles #2269
Conversation
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit dfe1744:
|
🦋 Changeset detectedLatest commit: dfe1744 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
@mskelton I've pushed out some changes to the implementation which simplify how @ajs139 If you could take a look at what I'm doing here I would highly appreciate it. One thing that I know this won't handle are mixed caches in a single tree - but this seems so far-fetched that I don't really want to handle this right now. |
| @@ -0,0 +1,19 @@ | |||
| const tickledCssProps = new WeakMap() | |||
|
|
|||
| export const getTickledClassName = cssProp => tickledCssProps.get(cssProp) | |||
There was a problem hiding this comment.
| export const getTickledClassName = cssProp => tickledCssProps.get(cssProp) | |
| export const getTickledClassName = cssProp => { | |
| if(!tickledCssProps.has(cssProp)) { | |
| throw new Error('Could not find cssProp, ensure enzyme snapshots are configured correctly - https://emotion.sh/docs/testing.'); | |
| } | |
| return tickledCssProps.get(cssProp) | |
| }; |
There was a problem hiding this comment.
Just in case we detect a shallow component, but it wasn't "tickled".
There was a problem hiding this comment.
Actually - we'd have to complicate this check because the css prop can "resolve" to a null (or other falsy values and:
const wm = new WeakMap()
wm.set(null, {}) // throws, that's why we return for `!cssProp` in `tickle`
wm.get(null) // ok, just returns undefinedSo I would be in favor of keeping this as-is for simplicity. I don't see when the proposed error would actually throw and it might be friendlier to the user to actually print something, rather than preventing them to work for some kind of an edge case. This is not a crucial production path after all.
There was a problem hiding this comment.
Sounds good, thanks for looking into it.
|
@Andarist Since I opened the PR, I can't approve it, but your updates look awesome and fix all the remaining bugs! Can't wait for this to be fully completed so I can finish updating my company's apps to Emotion 11. Great work! |
What: The change in #2233 introduced a bug where conditional styles passed to the css prop would break the Enzyme snapshot serializer.
Why: To allow users who use Enzyme snapshots to use the Emotion snapshot serializer.
How: Return early if the style variable is falsy to prevent errors
Checklist: Yes
Screenshot