Remove @charset and @import rules when building CSS
#43
Conversation
This file contains 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
… appear mid-stream
pyronaur
requested changes
Dec 12, 2023
src/style-ast.ts
Outdated
| @@ -75,7 +75,7 @@ export class StyleAST { | |||
| const clone = new StyleAST( this.css, csstree.clone( this.ast ), this.errors ); | |||
|
|
|||
| clone.pruneMediaQueries(); | |||
| clone.pruneKeyframes(); | |||
| clone.pruneAtRules(); | |||
There was a problem hiding this comment.
going by naming only - wouldn't this remove @media too?
There was a problem hiding this comment.
No, it prunes unwanted At-rules, including @keyframes, @charset and @import. Would it help if it specifies unwanted in the fn name?
There was a problem hiding this comment.
I'd prefer pruneAtRules(['keyframe', 'charset', 'import']). Barring that, "unwanted" might be ok too.
There was a problem hiding this comment.
Yes, that's much cleaner. Applied in 289bcc6. Thanks :)
3 tasks
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.
@charsetrules can only appear at the very beginning of any CSS block and are technically invalid anywhere else. As Critical CSS joins multiple files together, it's cleaner to stripcharsetrules. Technically charset rules are only needed for non-utf8 stylesheets, which are vanishingly rare.On testing, it appears that non-utf8 stylesheets are currently not supported anyway, so we don't lose anything by stripping the tags. If we come across sites with non-utf8 stylesheets, we can consider adding explicit support down the road.
This update also strips out
@importdirectives - both because they are also fussy about where they appear in a CSS file, and because by the time you@importsomething you are waiting on a resource to load - so it has no business in a Critical CSS block.*** To test ***
I recommend using @haqadn's yalc setup to test the change in Boost - pc9hqz-1NI-p2 - and verify that no
@charsetrules appear in the resultant Critical CSS block.