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

Expressions are not being parsed #388

Open
raxbg opened this issue Sep 19, 2022 · 9 comments · May be fixed by #389
Open

Expressions are not being parsed #388

raxbg opened this issue Sep 19, 2022 · 9 comments · May be fixed by #389

Comments

@raxbg
Copy link
Contributor

raxbg commented Sep 19, 2022

Simple expressions do not seem to be parsed. Example:

div {
    height: (vh - 10);
}

This ends up being treated as:

div {}
@raxbg raxbg linked a pull request Sep 19, 2022 that will close this issue
@sabberworm
Copy link
Contributor

I’m sorry, is this a new CSS feature that I don’t know about yet?

@raxbg
Copy link
Contributor Author

raxbg commented Sep 20, 2022

Not really. I believe I oversimplified the example 😃 Consider this:

div {
  background: green;
  width: calc((100vw - 100px) / 2);
  height: 100px;
}

Without this fix the value in calc will not be parsed correctly.

@rafaucau
Copy link

I encountered a similar issue in the AMP plugin, which uses this library. ampproject/amp-wp#7291

Example:

:where(.container) {
   width: min(var(--container-max-width), 100% - calc(var(--container-padding) * 2));
   margin-inline: auto;
}

@JakeQZ
Copy link
Contributor

JakeQZ commented Feb 15, 2024

On the basis of the examples here, the issue seems to be with the nested brackets. Either with var(), or nested calc(). Both of these IIRC are CSS4, so relatively new.

The OP example is definitely new to me. I don't know if it's allowed for the calc keyword to be implicit. Or for there to be a default implicit length unit. @raxbg, would you be able to cite the relevant standards track for this?

@richmilns
Copy link

For me, it seems to be that the use of arithmetic causes issues:

.heading {
    font-size: clamp(1.4rem, 3vw + 1rem, 2rem);
}

Whereas this example gets parsed without a problem:

.heading {
    font-size: clamp(1.4rem, 3vw, 2rem);
}

This approach for using clamp() in responsive font sizes was taken from Smashing Magazine, and whilst it's a newish idea and clamp() is actually still working draft at the time of writing, it is well supported in browsers.

@jhard
Copy link

jhard commented Dec 31, 2024

Parentheses in math-functions are an implicit calc as far as I understand, e.g. https://drafts.csswg.org/css-values/#funcdef-calc

Example 32
Standard mathematical precedence rules for the operators apply: calc(2 + 3 * 4) is equal to 14, not 20.
Parentheses can be used to manipulate precedence: calc((2 + 3) * 4) is instead equal to 20.
Parentheses and nesting additional calc() functions are equivalent; the preceding expression could equivalently have been written as calc(calc(2 + 3) * 4).

And this is also valid for regular math function and their parameters (and you don't need extra parentheses, either), e.g. https://drafts.csswg.org/css-values/#example-67532822

Note: Full math expressions are allowed in each of the arguments; there’s no need to nest a calc() inside! You can also provide more than two arguments, if you have multiple constraints to apply.

You can write something like sin(10% + 1em) which would be equivalent to sin(calc(10% + 1em)).

@JakeQZ
Copy link
Contributor

JakeQZ commented Jan 1, 2025

Parentheses in math-functions are an implicit calc as far as I understand

Yes, that seems to be the case. calc is implicit for any parenthesized expression within a function (such as min, clamp, or sin).

However, calc is required if no other function is involved. From Example 32:

.aspect-ratio-box {
  --ar: calc(16 / 9);
  --w: calc(100% / 3);
  --h: calc(var(--w) / var(--ar));
  width: var(--w);
  height: var(--h);
}

Although --ar could have been written as simply --ar: (16 / 9);, --w is used both on its own (in width) and as a calc() component (in --h), so it has to be written as a full calc() function itself.

I think this is saying only the calc from --ar could be dropped, and that the others are required. Expanded, it evaluates to

width: calc(100% / 3);
height: calc(calc(100% / 3) / calc(16 / 9));

Or, in other words,

width: (100% / 3);

would be invalid. It needs an outer calc if no other function is involved.

So the example in the OP is invalid, and should be dropped (even if it is more correctly height: (100vh - 10px);). Browsers will discard it, and so should we. However, the subsequent example with width: calc((100vw - 100px) / 2); is valid and should be parsed correctly.

I've made a new year resolution to move forward to resolve this and related issues...

HNY.

@jhard
Copy link

jhard commented Jan 1, 2025

Yeah, I agree that the original example is invalid and should be discarded.
A consumer can currently not learn about these cases if I understand correctly since the exceptions will be caught by the parser. Might be a good idea to either rethrow if some flags are given, or emit events for invalid (and also overwritten in the same ruleset?) rules.

With the --w example it's interesting. I've only tested in my browser to get a feeling for how it behaves, so take it with a grain of salt. var() doesn't trigger calc() as sin() would (which makes sense, you don't always want it to be calculated).

--w: calc(200px / 3); width: var(--w); and --w: (200px / 3); width: calc(var(--w)); and --w: 200px / 3; width: calc(var(--w)); are identical, so assignments to variables are not interpreted at all unless you use a function in them directly (again, makes sense, making no assumptions about what you're going to do with those variables).

--w: (200px / 3); width: var(--w); isn't flagged as invalid in dev tools (probably because the syntax is valid and it's not checking the internal state), but doesn't affect the computed styles. I believe it's out of scope for this library to detect that, it's CSS Parser after all, not CSS Interpreter.

width: calc(100px * asin(100 / 3)) is invalid syntax (because asin returns an angle and you can't multiply an angle by a width to receive a width, I assume).
--w: calc(100px * asin(100 / 3)) is valid, and width: var(--w) is valid syntax as well, but again fails to affect the styles.
width: calc(100px * asin(100 / var(--myvar))) is also valid syntax but produces an invalid state, so width: 100px; width: calc(100px * asin(100 / var(--myvar))) will leave you with no width definition since the latter overwrites the former but itself doesn't get applied.

--a: (100px / 3); --b: + 10px; --c: var(--a) var(--b); width: calc(var(--c)); is valid syntax and also works just like you think it would (in firefox, not tested widely), and so does --a: (100px / 3); --b: *; --c: 5; --d: var(--a) var(--b) var(--c); width: calc(var(--d));.

Lots of great ways to produce bugs that are hard to track down that I'm totally not going to add our candidate tests.

@JakeQZ
Copy link
Contributor

JakeQZ commented Jan 2, 2025

A consumer can currently not learn about these cases if I understand correctly since the exceptions will be caught by the parser. Might be a good idea to either rethrow if some flags are given, or emit events for invalid (and also overwritten in the same ruleset?) rules.

The current implementation has a lenient/strict mode parsing option. The latter (re-)throws exceptions upon invalid constructs, whereas the former silently drops them. Moving forward, the plan is to replicate browser behaviour as much as possible - i.e. operate in lenient mode; remove the strict mode option, and instead accept a Logger interface implementation through which invalid constructs can be reported. See #461 and #462.

var() doesn't trigger calc() as sin() would (which makes sense, you don't always want it to be calculated).

Indeed. var() is just a substitution, and not a CSS function per se.

width: calc(100px * asin(100 / 3)) is invalid syntax (because asin returns an angle and you can't multiply an angle by a width to receive a width, I assume)

asin(100/3) is a complex number - the argument must be between -1 and +1 for the result to be real. That said, it doesn't seem to work even if provided an argument in that range. In mathematics, the result is just a number. I don't see why CSS should be so picky in treating it as an angle - if that is even the reason for the error.

I believe it's out of scope for this library to detect that, it's CSS Parser after all, not CSS Interpreter.

Absolutely. We don't care for the meaning, just the syntax.

@jhard, we would welcome any PRs that would go some way to resolving some of these issues. There are a few open and outstanding that may be used for reference, inc;uding this. The code base has moved on, so it may be difficult to rebase the existing PRs.

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.

6 participants