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

feat: media query range syntax & single value function support via css-tree extension #8430

Merged
merged 1 commit into from
Mar 30, 2023
Merged

Conversation

typhonrt
Copy link
Contributor

@typhonrt typhonrt commented Mar 29, 2023

Closes #5876. As discussed there, css-tree is not updated for the latest CSS syntax including the ability to use single value functions (calc, etc.) in media queries. css-tree also does not yet support media query range syntax that is part of the media query level 4 specification.

This PR broadly helps the entire Svelte ecosystem including SvelteKit.

Given the acceptance of the container query PR (#8275) to extend css-tree to support container queries I was able to refactor this extension support repurposing the range syntax support to apply to both container & media queries. At the same time the single value function support implemented for container queries also applies to media queries.

Changes
src/compiler/parse/read/css-tree-cq/css_tree_parse.ts: cleanup new AST node import / assignment.

src/compiler/parse/read/css-tree-cq/node/index.ts: added for convenience to export all nodes cleanly.

src/compiler/parse/read/css-tree-cq/node/lookahead_is_range.ts: separated utility function out to be used by CQ / MQ.

src/compiler/parse/read/css-tree-cq/node/media_query.ts: provides new media query node implementation.

src/compiler/parse/read/css-tree-cq/node/query_feature.ts: renamed from container_feature.ts

src/compiler/parse/read/css-tree-cq/node/query_range_feature.ts: renamed from container_range_feature.ts

test/css/samples/media-query: Updated parsing test case with range syntax and single value function use cases.

Impact
No downsides that I'm aware of regarding these changes. Simply that Svelte can now ship with updated media query syntax support. I have thoroughly tested the code and updated the media query parsing test accordingly. Because the fork capability of css-tree is used to extend it the css-tree version can keep being bumped if new versions come out without updated MQ support. However, when the unified queries support is added to css-tree the local extensions can be removed and an easy swap back to the official css-tree parse capability is trivial.

Testing
The media query parsing test has been updated for range syntax and single value function use cases.

css-tree Extension
The bulk of this PR is a refactor of the container query implementation to share the range syntax and single value function support added in the CQ PR between both media and container queries via local extension of css-tree using the fork capability of css-tree to just modify how the MediaQuery node is parsed. The benefit of taking this approach is that the temporary modifications necessary to extend css-tree are committed to the Svelte repo and a separate hard fork of css-tree isn't necessary. The css-tree version can continue to be bumped as necessary and when css-tree supports modern query syntax the local extension can be removed.

It is unknown when css-tree will be updated to support modern query support. However, when css-tree does offer support a one line change and removal of the extensions provided in this PR is trivial.

… media query range syntax / MQ level 4 support.
@vercel
Copy link

vercel bot commented Mar 29, 2023

@typhonrt is attempting to deploy a commit to the Svelte Team on Vercel.

A member of the Team first needs to authorize it.

@typhonrt
Copy link
Contributor Author

@dummdidumm. This is the follow up I mentioned regarding refactoring the css-tree extension. It is well tested and expands on the recent CQ PR bringing media query support up to date to the latest standards. It's great as there are minimal code changes.

@dummdidumm dummdidumm merged commit d42ca04 into sveltejs:master Mar 30, 2023
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.

Media query with calc in does not compile
3 participants