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

Do not log failed rule insertions in the speedy mode for more vendor-prefixed pseudo-elements/classes #2393

Merged
merged 2 commits into from
May 31, 2021

Conversation

layershifter
Copy link
Contributor

@layershifter layershifter commented May 28, 2021

What:
This PR expands a fix that was made in #2149 to swallow errors from invalid insertions.

Why:
I added to the list to silence warnings:

  • -moz-focus-inner
  • -moz-focusring
  • -ms-clear

How:

Tested via yarn lint/yarn test & browser.

Checklist:

  • Code complete
  • Changeset

@changeset-bot
Copy link

changeset-bot bot commented May 28, 2021

🦋 Changeset detected

Latest commit: b64ae0a

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

This PR includes changesets to release 1 package
Name Type
@emotion/sheet 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

@layershifter
Copy link
Contributor Author

I tried to write a unit test for this case, but it seems that JSDOM accepts anything anything ¯_(ツ)_/¯

@codesandbox-ci
Copy link

codesandbox-ci bot commented May 28, 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 b64ae0a:

Sandbox Source
Emotion Configuration

@Andarist
Copy link
Member

Andarist commented May 28, 2021

@layershifter could you prepare a codesandbox that would showcase that each of those results in a warning right now?

@layershifter
Copy link
Contributor Author

layershifter commented May 31, 2021

@Andarist, sure, here it is https://codesandbox.io/s/pedantic-franklin-31zqw. I used low level APIs there as in our code. Please let me know if it okay or I should make another one.

Latest Chrome

There was a problem inserting the following rule: ".css-4z0ic9:-moz-focus-inner{border:4px solid green;}" Error: Failed to execute 'insertRule' on 'CSSStyleSheet': Failed to parse the rule '.css-4z0ic9:-moz-focus-inner{border:4px solid green;}'.
There was a problem inserting the following rule: ".css-4z0ic9:-moz-focusring{border:4px solid green;}" Error: Failed to execute 'insertRule' on 'CSSStyleSheet': Failed to parse the rule '.css-4z0ic9:-moz-focusring{border:4px solid green;}'.
There was a problem inserting the following rule: ".css-4z0ic9:-ms-clear{display:none;}" Error: Failed to execute 'insertRule' on 'CSSStyleSheet': Failed to parse the rule '.css-4z0ic9:-ms-clear{display:none;}'.

Latest Firefox

There was a problem inserting the following rule: ".css-4z0ic9:-moz-focus-inner{border:4px solid green;}" DOMException: An invalid or illegal string was specified
There was a problem inserting the following rule: ".css-4z0ic9:-ms-clear{display:none;}" DOMException: An invalid or illegal string was specified

@Andarist Andarist merged commit 405af5c into emotion-js:main May 31, 2021
@Andarist
Copy link
Member

@layershifter thank you!

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.

2 participants