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

[css extraction] Handle subsidiary at-rules #1377

Merged
merged 1 commit into from
Sep 25, 2023
Merged

Conversation

tidoust
Copy link
Member

@tidoust tidoust commented Sep 24, 2023

The @font-feature-values at-rule can contain subsidiary at-rules, defined in the spec as at-rules scoped to @font-feature-values: https://drafts.csswg.org/css-fonts/#font-feature-values-syntax

Reffy was not expecting at-rules to be scoped and extracted them as root at-rules, with a for attribute that did not pass schema validation.

With this update, Reffy no longer lists subsidiary at-rules at main at-rules, and rather lists them as descriptors under the main at-rule they are scoped to.

The `@font-feature-values` at-rule can contain subsidiary at-rules, defined in
the spec as at-rules scoped to `@font-feature-values`:
https://drafts.csswg.org/css-fonts/#font-feature-values-syntax

Reffy was not expecting at-rules to be scoped and extracted them as root
at-rules, with a `for` attribute that did not pass schema validation.

With this update, Reffy no longer lists subsidiary at-rules at main at-rules,
and rather lists them as descriptors under the main at-rule they are scoped to.
@tidoust tidoust merged commit 4fa4b0e into main Sep 25, 2023
1 check passed
@tidoust tidoust deleted the atrule-subsidiary branch September 25, 2023 08:53
tidoust added a commit that referenced this pull request Sep 29, 2023
New feature:
- [css extraction] Handle subsidiary at-rules (#1377). A `"for"` property may
now appear under at-rules values in CSS extracts, with values that are at-rules
themselves. This only affects the definition of `@font-feature-values` in the
CSS Fonts extract for now.

Dependencies bumped:
- Bump rollup from 3.28.1 to 3.29.4 (#1372, #1383)
- Bump chai from 4.3.8 to 4.3.10 (#1384)
- Bump undici from 5.23.0 to 5.25.2 (#1368, #1375, #1378)
- Bump puppeteer from 21.1.0 to 21.3.5 (#1373, #1382)
- Bump respec from 34.1.6 to 34.1.8 (#1370)
- Bump web-specs from 2.66.0 to 2.68.0 (#1374)
@cdoublev
Copy link
Contributor

cdoublev commented Oct 2, 2023

Do you plan to update your schema and consider them as namespaced rules rather than descriptors in a future version?

@tidoust
Copy link
Member Author

tidoust commented Oct 2, 2023

Do you plan to update your schema and consider them as namespaced rules rather than descriptors in a future version?

The plan was more to force push them into the current schema not to have to introduce a breaking change in the schema ;)

Nested rules have a "type": "at-rule" property in the extract to distinguish them from more usual descriptors if needed.

If they are a pain to deal with in the current form, we could perhaps handle them differently and either move them back to the atrules level with a for property, or list them under an atrules property within the parent at-rule object.

@cdoublev
Copy link
Contributor

cdoublev commented Oct 2, 2023

It is not that painful. I consume the rules extracted by Reffy only to watch for new/modified rules. I pointed out that it might be easier to only define rules with prose.

That said, CSS Syntax does not consider descriptors and namespaced at-rules the same (emphasize added):

Descriptors are similar to properties (and are declared with the same syntax) but are associated with a particular type of at-rule rather than with elements and boxes in the tree.

https://drafts.csswg.org/css-syntax-3/#css-descriptor

Also note that descriptors for @media-query are also not really descriptors:

While not technically descriptors, media features can be defined with this block as well, if an "mq" class is put on the block. It then only requires Name, For, and Value.

https://speced.github.io/bikeshed/#descdef-block

Fortunately, I do not think something like @media (min-width: 1px) { min-width: 1px } will be valid in the future. Otherwise there would be no way to know what is valid in () but not in {}, or vice-versa.

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.

3 participants