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

Media query with calc in does not compile #5876

Closed
Kl4rry opened this issue Jan 12, 2021 · 8 comments · Fixed by #8430
Closed

Media query with calc in does not compile #5876

Kl4rry opened this issue Jan 12, 2021 · 8 comments · Fixed by #8430

Comments

@Kl4rry
Copy link

Kl4rry commented Jan 12, 2021

Description
When placing a calc inside of a media query you get a compiler error.

To Reproduce
This repl contains an example of the bug.
https://svelte.dev/repl/d3b8fb7d74d540b69b990f34105b9b22?version=3.31.2

This results in compiler error.
@media screen and (min-width: calc(100px - 10px)) { h1 { color: green; } }

Intended behavior
This works with normal css so it should produce valid css instead of throwing an error

System:
OS: Windows 10 10.0.18363
CPU: (16) x64 AMD Ryzen 7 2700X Eight-Core Processor
Memory: 7.15 GB / 15.94 GB
Binaries:
Node: 14.8.0 - C:\Program Files\nodejs\node.EXE
Yarn: 1.22.4 - ~\AppData\Roaming\npm\yarn.CMD
npm: 6.14.7 - C:\Program Files\nodejs\npm.CMD
Browsers:
Chrome: 87.0.4280.141
Edge: Spartan (44.18362.449.0)
Internet Explorer: 11.0.18362.1
npmPackages:
rollup: ^2.3.4 => 2.36.1
svelte: ^3.31.2 => 3.31.2

@Conduitry
Copy link
Member

I think this unfortunately will involve updating the version of css-tree that we're using. We're pinned to an old alpha from 2017 because later versions aren't very treeshakable. It's been a while since I've checked on that project to see whether they still have the treeshakability problems or even to see whether it's still maintained.

@dreitzner
Copy link
Contributor

dreitzner commented Jan 12, 2021

@Conduitry: it is still maintained (last npm update 2 months ago)

@ludofischer
Copy link

I think https://github.com/thysultan/stylis might be a better fit, emotion and styled components in React use it for CSS in JS. A sophisticated parser like CSSTree seems a bit too much for the use inside the Svelte compiler (the AST is even being traversed as if it was JavaScript).

@tanhauhau tanhauhau mentioned this issue Feb 3, 2021
1 task
@stale
Copy link

stale bot commented Jun 26, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@tanhauhau
Copy link
Member

using calc() within @media media query is not according to the css specs.

as we do parse into the @media, instead of skipping it as a Raw node, css-tree will complain as a parse error.

css-tree does provide an option to not parse AtRules, but it is a boolean that covers all at rules, including @keyframe, @supports, @font-face, ... will need to see whether we need to parse AtRules in the first place

upgrading css-tree to v2 alone will not fix this issue.

@nicoxxl
Copy link

nicoxxl commented Aug 21, 2022

A more standard way to get this bug would be using this media query:

 @media only screen and ((orientation: portrait) or (max-width: 600px))

@typhonrt
Copy link
Contributor

typhonrt commented Mar 3, 2023

I am seeking consensus from Svelte maintainers on the proposed solution below.

So, this issue is valid and you can use single value CSS functions in media queries. See calc() expressions in the browser compatibility sections showing wide support. This includes not only calc, but also clamp, max, min functions; IE functions that returns a single value.

The main problem is that css-tree is lagging behind in support of modern query syntax. I have submitted a PR that uses the fork capability of css-tree to extend it adding container query support. In the process of completing the container query PR I added support for single value CSS functions.

I propose that when the 8275 PR (link above this post) is accepted that I apply the same process for media queries extending css-tree media query nodes for range syntax and single value CSS function support. This is an interim solution that can be utilized until a release of css-tree is available that supports modern query syntax and can be easily reverted to the official css-tree release. The nice thing about the solution arrived upon is that the extension to css-tree can be committed to the Svelte repo for visibility. It will leverage the same solution and be located in the same place in the Svelte repo providing up to date MQ / CQ support and will close this issue.

So my proposal seeks to merge PR 8275 then immediately submit a follow on PR that fixes this issue for the same Svelte release.

@typhonrt
Copy link
Contributor

Great news folks... After the container query PR merge a couple of days ago I was able to refactor the css-tree extension code to add single value function (calc, etc.) support and range syntax support for media queries. This closes this issue with full support by local css-tree extension until css-tree ships modern query support.

You can find the PR here: #8430.

dummdidumm pushed a commit that referenced this issue Mar 30, 2023
… media query range syntax / MQ level 4 support. (#8430)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants