Skip to content
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

Strip static class names from jest snapshot results #320

Merged
merged 1 commit into from Jul 15, 2020

Conversation

blnoonan
Copy link
Contributor

@blnoonan blnoonan commented May 4, 2020

Due to changes in styled-components v5, certain hashed class names are no longer being included in masterSheet, and therefore are not being serialized in snapshots by jest-styled-components.

Specifically, this occurs in the case where a styled component (StyledDiv) refers to another styled component (StyledDiv = styled(BaseDiv)) and the base component (BaseDiv) is not referenced in the component tree.

This PR proposes stripping all static sc- prefixed classes from snapshot results as a brute force way to resolve this issue.

Relevant styled-components doc:
https://styled-components.com/docs/advanced#referring-to-other-components

Relevant issues:
#307
#276

Copy link

@igorrmotta igorrmotta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

On their docs it says:

It hasn't any style attached to it. Instead, it's used to quickly identify which styled component a DOM objects belongs to or to make minor changes in the DevTools.

https://styled-components.com/docs/faqs#why-do-my-dom-nodes-have-two-classes

@blnoonan
Copy link
Contributor Author

@igorrmotta any chance we can merge this in? I wrote up a blog post about my reasoning as well!

@igorrmotta
Copy link

@blnoonan Soz, I'm not a contributor. Can't merge your changes.

However, I have published your changes to my own npm repo. I've been using for the past month, it fixes a lot of my issues. There is still some, but they are not getting in the way right now.

You can find your changes published here.

@castamir
Copy link

@probablyup hey, could you please take a look on this PR?

@castamir
Copy link

@probablyup, @gsipos, @igorrmotta would it be possible to merge this PR? Ot what's blocking it?

@quantizor quantizor merged commit 5497740 into styled-components:master Jul 15, 2020
@laurenskling
Copy link

Can this be packaged in a new release? I'm pretty sure most of the jest-styled-components users need this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants