Fix snapshot styles order #1880
Conversation
Due to jest writing styles in the order it encounters rendered styled components across tests, an issue can occur where skipping/removing/reordering your tests will invalidate test snapshots. This fix sorts the style elements when serializing emotion styles.
🦋 Changeset is good to goLatest commit: 24d4fc4 We got this. 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 |
|
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. |
|
Sorry for the late reply, I was really busy in the past days. I'm afraid that the proposed solution is too simplistic - for example, it prints rules for pseudo-classes and pseudo-elements before the actual classes. It also can print I think a better solution would be to figure out how to sort them based on the order of the Please let me know if you would like to work on this one, if not I might take it over but it also might take me some time before I do that. Any help is highly appreciated 🙏 |
|
Ok, thanks for looking into this. I've also been super busy lately so I can totally sympathize, no worries. I'd like to take a look into it but I don't know how long it will take me to get back to. Feel free to take it on at any point, but I will make an effort to find time for it. |
# Conflicts: # packages/css/test/__snapshots__/css.test.js.snap # packages/css/test/instance/__snapshots__/css.test.js.snap # packages/css/test/no-babel/__snapshots__/index.test.js.snap # packages/styled/__tests__/__snapshots__/styled.js.snap # packages/styled/test/__snapshots__/composition.test.js.snap # yarn.lock
…ules insertion order
|
@Jimmydalecleveland I've finally found some time to fix this properly - u can take a look at how I have done it if you are interested. |
…cted rules (emotion-js#1880) * Fix inconsistent style ordering for snapshots Due to jest writing styles in the order it encounters rendered styled components across tests, an issue can occur where skipping/removing/reordering your tests will invalidate test snapshots. This fix sorts the style elements when serializing emotion styles. * Add changeset for @jest/emotion style sorting * update snapshots for style reordering fix * Rewrite style extraction in the serializer to be independent of the rules insertion order Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com>
What: Fix testing snapshot style reordering when the order that tests are run changes. I have an issue open discussing the details of the bug here: #1878
Due to jest writing styles in the order it encounters rendered styled components across tests, an issue can occur where skipping/removing/reordering your tests will invalidate test snapshots.
Why: Also described in the issue linked above, but the gist is that reordering/skipping/removing tests causes other test snapshots to fail in certain situations where that components styles were already being rendered by emotion. This causes the style order to be changed, but the same styles to be present, causing a lot of noise in test and spooking my coworkers on our code reliability. It's also really hard to track down that this is actually what is going on (many hours for me to figure that out)
How: This fix sorts the style elements when serializing emotion styles.
Checklist:
I made this a patch with changeset, because it doesn't require users to change any code to use it. But I was pretty iffy about that, because it does break many snapshots by reordering their css, and they would have to run update for all their snapshots when installing this patch. I'm open to critique there and changing the semver for that if need be.
I realize this is a pretty drastic change outcome, even if the code change is pretty small. In the reproduction repo (https://github.com/Jimmydalecleveland/emotion-jest-reordering-bug) I linked in the previous issue I have examples of how to produce this bug, and I used this repo to test that my proposed change works to fix that issue.
I don't really know if this is the right way to go about it, and many people probably don't even know they have an issue waiting to happen when they move their tests around or rerun them in different orders. We thought this was general snapshot noise in our project, which is quite large, for many months. Again, I'm open to a complete rejection of this PR or any feedback on a better way to handle it.