Jest serializer new api #1850
Jest serializer new api #1850
Conversation
🦋 Changeset is good to goLatest commit: 16fa4be 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. Latest deployment of this branch, based on commit 16fa4be:
|
Codecov Report
|
| @@ -0,0 +1,29 @@ | |||
| { | |||
| "name": "@emotion/css-prettifier", | |||
There was a problem hiding this comment.
I'm not super fond of this name - it seems super close to @emotion/css but yet it doesn't have to do anything directly with it.
There was a problem hiding this comment.
I think it's fine. For mostly internal packages, I don't think it matters much
| import memoize from '@emotion/memoize' | ||
| import { compile, serialize, combine, tokenize } from '@emotion/stylis' | ||
|
|
||
| // adjusted https://github.com/thysultan/stylis.js/blob/68b9043427c177b95a0fd6a2a13f5b636bf80236/src/Serializer.js#L26-L34 |
There was a problem hiding this comment.
I've started by forking the original stringify because I thought I would only have to change a thing or two... but I had to change a little bit more than that, somewhat to the point that it might not be worth it and maybe it would be better to just implement a specific to this use case serialize, rather than acting like its a Stylis plugin. OTOH - this works now and Im not sure if I want to refactor it.
| @@ -427,11 +427,11 @@ exports[`css nested at rule 1`] = ` | |||
| background-color: blue; | |||
| } | |||
|
|
|||
| @supports (width: 100vw) { | |||
| @supports (width: 100vw) { | |||
There was a problem hiding this comment.
this is an example of improved nested atrules printing
| prettyStyles = css.stringify(css.parse(styles)) | ||
| } catch (e) { | ||
| console.error(e) | ||
| throw new Error(`There was an error parsing the following css: "${styles}"`) |
There was a problem hiding this comment.
stylis doesnt throw on invalid input, it should just swallow it
Andarist
force-pushed
the
jest-serializer-new-api
branch
from
8cd2c67 to
92972d3
Compare
Andarist
force-pushed
the
jest-serializer-new-api
branch
from
92972d3 to
89925b8
Compare
# Conflicts: # packages/css/package.json # yarn.lock
.changeset/breezy-carpets-smile.md
Outdated
| '@emotion/jest': major | ||
| --- | ||
|
|
||
| `test` & `print` are no longer exported as named exports. If you want to access the default serializer just access the default export. |
There was a problem hiding this comment.
test & print were exported so you could add @emotion/jest to snapshotSerializers, I think I'd like to keep @emotion/jest working and throw an error when @emotion/jest/serializer is imported that tells people to use @emotion/jest
There was a problem hiding this comment.
Hm, isn't it packing too much into a single module? Having a separate entry point for this seems conceptually easier as with test & print being exported we kinda trick Jest to think that it's a serializer - structurally ok-ish but nominally not so much.
There was a problem hiding this comment.
Yeah, okay, sure. Let's use the seperate module. Could you make the changeset clearer that using the raw package name in the snapshotSerializers option will no longer work and they'll have to use @emotion/jest/serializer instead?
There was a problem hiding this comment.
added this info, could you re-check?
Co-authored-by: Mitchell Hamilton <mitchell@hamil.town>
|
I've merged this after applying your suggested changeset changes as this was the only comment. Feel free to comment further in #1817 if needed. |
It has been reported that snapshots show every style to be changed when snapshot & received don't match. After investigating this:
config.indentoption which somewhat comes helpful here because we were using the old serializers APISo I've decided to refactor those to use "new" serializers API (it got introduced in Jest 21) to eventually handle
config.indentregardless of what happens to the reported ticket in Jest repo. Also, it was impossible to handle it (at least not in a straightforward way) using the old APIIn the process, I've removed
cssdependency (a parser and stringifier) as we already depend on Stylis and we don't need two separate parsers. This has also actually "fixed"/improved printing of nested at rules.