-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
fix(jest-snapshot): always run prettier format a second time #11560
Conversation
Codecov Report
@@ Coverage Diff @@
## main #11560 +/- ##
==========================================
- Coverage 68.73% 68.73% -0.01%
==========================================
Files 322 322
Lines 16580 16577 -3
Branches 4784 4783 -1
==========================================
- Hits 11397 11394 -3
Misses 5150 5150
Partials 33 33
Continue to review full report at Codecov.
|
@SimenB I managed to get it reproduced :D I've restored the condition to confirm that the test fails on CI against the snapshot (which contains the correctly indented output) - will drop that commit after & restructure the code to call I originally tried to do this using an object, but for some reason the |
😞 tho I don't think thats the result of my change. |
@SimenB any chance of getting a quick review? 😅 |
Would love this merged so I can upgrade to Jest 27! Starting to use inline snapshots extensively and the lack of indentation is a deal breaker. |
@SimenB would be great if we could land this 🙂 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
And sorry about the delay
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Summary
When writing inline snapshots, the custom prettier parser that handles indenting the snapshots is only called if there's a formatting difference between the original code vs with the new snapshot, which will only happen if the file needs to be formatted with prettier anyway.
This means if you already have code that is correctly formatted in prettier's eyes, we get a regression of #9477:
This wasn't caught by the test suite because (among other things),
writeFiles
callsdedent
on all the file content its given which strips out all trailing and leading blank lines; this means that all files written by the suite using this method are considered not correctly formatted byprettier
because it requires files have a trailing blank newline.I've reproduced this by crafting a test whose file contents is considered correctly formatted, including with a trailing newline by using
\\n
to havededent
preserve that specific new line, and have fixed it by removing the formatting check - so nowprettier.format
is always called a second time to apply the snapshot parser/indenter.Resolves #11459
Test plan
I've added a new test.