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

Stylis upgrade #2590

Merged
merged 5 commits into from Dec 12, 2021
Merged

Stylis upgrade #2590

merged 5 commits into from Dec 12, 2021

Conversation

Andarist
Copy link
Member

FYI @thysultan - this is what i've ended up with as to changes introduced to accommodate 4.0.12. I've decided to ignore 4.0.11 and removed the fix that I had for it ( 26b3e2d ) since that version was out only briefly and we didn't actually get any bug reports about this.

@changeset-bot
Copy link

changeset-bot bot commented Dec 10, 2021

🦋 Changeset detected

Latest commit: 7c3324d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 6 packages
Name Type
@emotion/babel-plugin Patch
@emotion/cache Patch
@emotion/css Patch
@emotion/css-prettifier Patch
@emotion/jest Patch
@emotion/react Patch

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

Comment on lines 104 to 111
if (
// @media
parent.type.charCodeAt(1) !== 109 &&
// @supports
parent.type.charCodeAt(1) !== 115
) {
return
}
Copy link
Member Author

Choose a reason for hiding this comment

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

This was a fix for stylis@4.0.11 that was out briefly but Sultan has patched the "regression" in a timely manner, and released stylis@4.0.12. So I've decided not to include this fix.

@codesandbox-ci
Copy link

codesandbox-ci bot commented Dec 10, 2021

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 7c3324d:

Sandbox Source
Emotion Configuration

@codecov
Copy link

codecov bot commented Dec 10, 2021

Codecov Report

Merging #2590 (7c3324d) into main (04681a5) will not change coverage.
The diff coverage is 100.00%.

Impacted Files Coverage Δ
packages/cache/src/stylis-plugins.js 100.00% <100.00%> (ø)

!element.length
// positive .length indicates that this rule contains pseudo
// negative .length indicates that this rule has been already prefixed
element.length < 1
Copy link
Member Author

Choose a reason for hiding this comment

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

this fixes prefixed rules (like .css-hash::-webkit-input-placeholder {}) - with the latest stylis@4.0.11 and stylis@4.0.12 those were replaced with their non-prefixed versions

Choose a reason for hiding this comment

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

Is stylis's prefixing of ::-webkit-input-placeholder incorrect as it stands?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, its prefixing it alright - we were screwing up the prefixed rules here though, before this change

@@ -19,7 +19,7 @@
"@emotion/sheet": "^1.1.0",
"@emotion/utils": "^1.0.0",
"@emotion/weak-memoize": "^0.2.5",
"stylis": "^4.0.10"
"stylis": "4.0.12"
Copy link
Member Author

Choose a reason for hiding this comment

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

I've decided to pin the version of this dependency as it's super crucial for Emotion and changes to it can affect our code in subtle ways. We want to keep this up-to-date with every release of Stylis - but we should always verify that our test suite passes before we can bump this dependency range

@Andarist Andarist mentioned this pull request Dec 10, 2021
@Andarist Andarist merged commit 1554a7e into main Dec 12, 2021
@Andarist Andarist deleted the stylis-upgrade branch December 12, 2021 21:58
@github-actions github-actions bot mentioned this pull request Dec 12, 2021
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

3 participants