Fix Parse error with @-webkit-keyframes at rule#6
Fix Parse error with @-webkit-keyframes at rule#6dylanirlbeck wants to merge 2 commits intoastrada:masterfrom
Conversation
lib/lexer.ml
Outdated
| let at_rule_without_body = [%sedlex.regexp? "@", ("charset" | "import" | "namespace")] | ||
|
|
||
| let nested_at_rule = [%sedlex.regexp? "@", ("document" | "keyframes" | "media" | "supports" | "scope")] | ||
| let nested_at_rule = [%sedlex.regexp? "@", ("document" | "keyframes" | "media" | "supports" | "scope" | "-webkit-keyframes" )] |
There was a problem hiding this comment.
I was wondering, should we add all the possible rules here, or only the most common ones?
There was a problem hiding this comment.
That's a great point! I'll cc @astrada so he sees this comment.
There was a problem hiding this comment.
Thanks! Feel free to add all the rules you need. When you are done, I will publish a new version on opam.
There was a problem hiding this comment.
@gaku-sei How many possible rules are out there? Adding the most common ones (at least to start) might be our best bet. If you can point me to a list of the ones you think I should add that'd be great!
There was a problem hiding this comment.
Sure, I could find a list on mdn: https://developer.mozilla.org/en-US/docs/Web/CSS/At-rule but it doesn't contain the vendor prefixes 😕
I tried them all on https://autoprefixer.github.io/:
/* input */
@charset {}
@import {}
@namespace {}
@media {}
@supports {}
@document {}
@page {}
@font-face {}
@keyframes {}
@viewport {}
@counter-style {}
@font-feature-values {}
/* output */
@charset {}
@import {}
@namespace {}
@media {}
@supports {}
@document {}
@page {}
@font-face {}
@-webkit-keyframes {}
@keyframes {}
@-ms-viewport {}
@-o-viewport {}
@viewport {}
@counter-style {}
@font-feature-values {}Notice that some are still experimental, like document, and viewport.
|
@astrada I'm having trouble building the project locally, but I just added the full list of at rules that I'll need for my project (you may want to try and pull down my branch and test on your machine). Let me know if you need anything else from me! On a side note, I think it'd be a good idea to try and add a CI integration that runs on pull requests, if you have time, but no worries. EDIT: I can confirm the projects builds! I just had to remove the css-parser.opam file (for reasons I do not know) |
|
I've rebased your patch on master reformatted with ocamlformat (I thought that github would consider this PR as merged, but it isn't so). I also cleaned the @scope div {
span {
color: blue;
}
}As you can see, inside @scope body, you can specify selectors. Otherwise, rules like @page {
margin: 1cm;
}don't need a special treatment because their body can contain just a set of declaration, and so they are treated as standard rules. If you check the tests, you should see that all the at-rules supported have their own test. The only missing ones are the vendor prefixed rules. |
|
I'm publishing the patched version to opam. Let me know if it fixes your issues. |
|
@astrada I guess I'm a bit confused, are the vendor prefixed rules not supposed to be recognized by the parser? I'm worried one of my project's users will run into future issues because not all the vendor prefixes are included in the parser (and thus will throw an error). Can you elaborate a bit more on the differences between the at rules and how they relate to the parser? I might just be missing something... thanks! |
|
No, generally, the parser doesn't care if the rules are prefixed or not. But there are some rules that are implemented with special casing and these rules are the at-rules listed in: |
|
So to sum it up, according to https://autoprefixer.github.io/, the only rule that needs special casing is |
|
@astrada Gotcha, thanks! |
@astrada A user for one of my projects reported the parser throwing an error for the following at rule:
It seems like the change to support this rule would be trivial --- just update the at rule regexp, so I went ahead and made the change.
I edited this file from GH, so let me know if you want me to pull this down and format it. You're also more than welcome to close this PR and implement the changes yourself (and perhaps include a test?). Thanks in advance!