-
-
Notifications
You must be signed in to change notification settings - Fork 145
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
Diff showing on unchanged snapshot lines #355
Comments
Helps the library comply with Jest's serializer's indent configuration, which in turn fixes styled-components#355
Hey there... this fix would be really helpful. I look forward to having this fix included in the next release :-) |
Workaround for styled-components#355 inspired by @fritz-c.
Hi @fritz-c, is there any update on this? The fix for this issue would be really useful. This is quite an important issue, which needs to be fixed because the latest version of the library is not usable due to this bug. I think this issue was introduced in v7.0.7 because if I use v7.0.6, then it works fine. |
@Bazif-Khan I haven't had the time to work on a fix for it, though the path to resolution seemed pretty clear at the time I left it. The code is up on GitHub for anyone who has some working hours to take over implementing the fix. |
Helps the library comply with Jest's serializer's indent configuration, which in turn fixes styled-components#355
Helps the library comply with Jest's serializer's indent configuration, which in turn fixes styled-components#355
Helps the library comply with Jest's serializer's indent configuration, which in turn fixes styled-components#355
* create failing tests for indent config compliance * use new serializer plugin API * pass indentation config to css.stringify Helps the library comply with Jest's serializer's indent configuration, which in turn fixes #355 * add comments to serialize util in tests * Manual updates post-rebase - Update test for new class format - Upgrade pretty-format to reduce indirect deps Co-authored-by: fritz-c <4413963+fritz-c@users.noreply.github.com> Co-authored-by: Ell Bradshaw <joelbr@tailormed.co>
There is an issue with diff output from jest when changes are found in snapshots. Specifically, it will show extraneous diffs in lines that have no changes:
The detection of changes itself is not flawed. This diff issue only happens when legitimate changes are detected elsewhere in the snapshot. The issue resides in the logic printing the changes.
Another CSS-in-JS library, emotion, and its jest library, @emotion/jest, had a very similar issue with diff output. It was identified in emotion-js/emotion#1847 and resolved with a serializer overhaul in emotion-js/emotion#1850 .
The root cause of the issue starts with some behavior introduced with Jest v25. Prior to Jest v25, snapshot diffs printed to stdout were sensitive to indentation, but a pull request made it insensitive to indentation like so:
However, this new diff logic requires that snapshot serializers respect the indentation config in order for it to work properly. Older serializers that don't take the indentation config into account can get false positives on changed lines (like the diff I shared above). There is actually a fallback in the jest-snapshot logic to account for this, but in the case of jest-styled-components and presumably @emotion/jest, there is a mixture of compliant serializers and non-compliant serializers called to produce the output, which results in the fallback failing to work correctly.
The fix for this issue must be made in the logic provided by jest-styled-components.
First, the serializer must be updated from the old plugin style (
{ test(){}, print(){} }
) to the new plugin style ({ test(){}, serialize(){} }
), in order to receive indentation configuration in the arguments. This may lose support for very old versions of Jest (I have confirmed at leastjest >= 22.0.0
would be fine), so it might need to be considered a breaking change if the jest version is not enforced somewhere (I didn't see it in peerDependencies). Following that change, we have two options to fix the diff issue:Respect the indentation configuration provided by jest. This is made difficult by the dependency on the
css
library for the serialization of style rules. That library has yet-unfixed bug in that indentation set as an empty string is ignored.Modify the indentation config being passed on by the serializer in jest-styled-components such that all serializers it calls are equally ignorant to indentation configuration, helping the fallback in jest-snapshot be triggered. This would allow us to fix the issue just with changes in this library, but it has the potential to change users' existing snapshots if they were using custom indentation before, and therefore might be considered a breaking change.
The text was updated successfully, but these errors were encountered: