feat(css/minifier) Remove useless zeroes

[ghostd]

Description:

Remove useless zeroes:
calc(3px + 10em -3px) => 10em

BREAKING CHANGE:

Related issue (if exists):

[alexander-akait]

Why we need extra loop? Can we solve it in nested_fold, because it is just sum with another sign, don't forget we should implement logic looking at all future math functions (not only calc)

[alexander-akait]

Please read my comment above, we should not add extra loops, these things can be solved in one loop and expression, any plus or minus with 0 just return value itself or value with anither sign

[alexander-akait]

We should refactor out internal logic, no need extra loops, please read a spec - https://www.w3.org/TR/css-values-4/#calc-simplification, finally we should refactor our logic to this

[yisibl]

I think calc(0 - 0px) is an invaild value.

[ghostd]

You're right, we should have something like calc(0q - (pi * 3px) - 0px - 4em)

[ghostd]

@alexander-akait do you mean we should refactor the code to strictly follow the spec (and create structures like "Invert" and "Negate" in an intermediate step as defined in the spec, by wrapping the matching AST node)? Or "just" refactor the code to follow the spec as much as possible with our current AST nodes without intermediate transformation ?

[alexander-akait]

Or "just" refactor the code to follow the spec as much as possible with our current AST nodes without intermediate transformation ?

I am fine with it, too

But will be great if you try to, but we can postpone it:

@alexander-akait do you mean we should refactor the code to strictly follow the spec (and create structures like "Invert" and "Negate" in an intermediate step as defined in the spec, by wrapping the matching AST node)?

Not at the parser level, just like PoC (i know it can take a time), because it may turn out that it is faster and more convenient.

[ghostd]

@alexander-akait btw, i just read this from the spec:

It’s important to maintain zero-valued terms, so the calc() doesn’t suddenly "change shape" in the middle of a transition when one of the values happens to have a zero value temporarily.

Do you think it's relevant in our case?

[alexander-akait]

@ghostd And yes and no, look at example:

https://tomhodgins.com/demo/cssom/

and put the code:

.class {
    width: calc(20px + 0px);
}

.class {
    width: calc(20px + 0%);
}

you will see how it works

[ghostd]

Not at the parser level, just like PoC (i know it can take a time), because it may turn out that it is faster and more convenient.

Ok, i'll give a try (in an other poc-branch) to refactor our current implementation like this:

• swc parse the CSS string (no changes)
• visit_mut_component_value calls our "simplify calc" function (no changes)
• transform our calc AST into an intermediate "CSS operator trees" as defined by https://www.w3.org/TR/css-values-4/#parse-a-calculation
• actually simplify the intermediate tree by following the spec
• serialize the intermediate tree into the our common AST nodes (i guess unwrapping the structures will be enough)

Is that ok for you?

[ghostd]

you will see how it works

Thanks for the pointer.

So i understood the spec correctly, and this PR is wrong since it transforms width: calc(20px + 0%); into width: calc(20px);... which is wrong. The simplification of calc(20px + 0px) into calc(20px) is already merged.

[alexander-akait]

@ghostd Yes, that is ideal solution ⭐

I think we need more 0 test here, with 0%, 0px, just 0, before/after parens and etc

[kdy1]

@ghostd Would you mind if I take on this?

[ghostd]

@kdy1 of course you can :)

I hope finish the other PR this weekend (i signed for a new position and was busy last weeks).

[kdy1]

Thank you!