[@emotion/sheet] Prevent any vendor-specific pseudo-selectors from throwing errors when speedy = true#3350
Open
tkajtoch wants to merge 2 commits intoemotion-js:mainfrom
Open
Conversation
🦋 Changeset detectedLatest commit: 6710b46 The changes in this PR will be included in the next version bump. 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. |
e045fa1 to
6710b46
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What:
This PR updates the error silencing logic to properly handle all vendor-specific pseudo-elements rather than just the hardcoded list.
Similarly to #3150, the updated logic uses a regex to detect any vendor prefixes including the more legacy ones for best compatibility (
-moz-,-webkit-,-o-,-ms-). The difference is, however, that it carefully checks whether the error is actuallySyntaxErrorand if it should be silenced following the CSSOMinsertRulespecification.Why:
The list of silenced SyntaxError errors caused by unrecognized pseudo-classes and pseudo-elements is currently not exhaustive and, realistically speaking, will never be enough, as browsers evolve and people test various things all the time.
There were multiple PRs open in the past updating the regex to include previously missing pseudo-elements, but it wasn't enough.
Here are some of the PRs adding new pseudo-elements and pseudo-classes merged over the years:
How:
Instead of maintaining a static list of pseudo-elements to ignore, this PR changes the approach to this problem. The regex is simplified to recognize all vendor-prefixes pseudo-elements and pseudo-classes (
/:-(webkit|moz|ms|o)-{/) and is combined with error type detection viainstanceof SyntaxErrorfollowing the specification, as well as additionally eliminates false-positives by checking whether the left curly bracket ({) exists in the rule string to ensure the thrownSyntaxErroractually originates from an unrecognized vendor-specific thing.This is needed due to the unstandardized error message when this kind of error happens:
Failed to execute 'insertRule' on 'CSSStyleSheet': Failed to parse the rule '%s'An invalid or illegal string was specified::-webkit-*pattern, but not:-webkit-*or any others to my knowledge 🤷🏻The string did not match the expected pattern.Unfortunately, JSDom doesn't throw any of these errors, and mocking this kind of behavior per-browser is quite tricky (and maybe pointless?) in the test environment. Let me know if you'd prefer to have some tests around this!
The change only affects development mode. The updated code takes ~67 bytes less after minification, but it changes basically nothing for a development-only code block.
Checklist: