postcss / postcss-custom-properties

Use Custom Properties in CSS
https://postcss.github.io/postcss-custom-properties
MIT License
596 stars 77 forks source link

Line breaks inside nested fallbacks unduly cause a syntax error #70

Closed chris-morgan closed 6 years ago

chris-morgan commented 7 years ago

Minimum viable example:

:root {
    foo: var(--a, var(--b,
0));
}

CssSyntaxError: postcss-custom-properties: <css input>:2:5: missing closing ')' in the value 'var(--b,' 1 | :root { > 2 | foo: var(--a, var(--b, | ^ 3 | 0)); 4 | }

Remove the line break and it works. Remove the outer level of var() wrapping and it works.


Furthermore, that this compiles:

:root {
    foo: var(--a, 0)))))))));
}

to this:

:root {
    foo: 0))))))));
}

also worries me. Both, I presume, are symptoms of the data not being handled as proper parsed function call syntax, but rather as strings being pulled apart in hazardous ways.

chris-morgan commented 7 years ago

It doesn’t need to be nested fallbacks, just nested parentheses:

Input:

a {
b: var(--c,
d(e
f));
}

Expected output:

a {
b: d(e
f);
}

Actual output:

a {
b: d(e;
}

This is forcing me to depart from my CSS style guide and end up with messy things lines like this:

foo:hover {
    background-image: var(--foo-hover-background-image,
        linear-gradient(var(--foo-hover-color-1, color(var(--foo-color-1) l(+5%))), var(--foo-hover-color-2, color(var(--foo-color-2) l(+5%)))));
}

instead of slightly more elegant things like

foo:hover {
    background-image: var(--foo-hover-background-image,
        linear-gradient(
            var(--foo-hover-color-1, color(var(--foo-color-1) l(+5%))),
            var(--foo-hover-color-2, color(var(--foo-color-2) l(+5%)))
        )
    );
}
jonathantneal commented 6 years ago

This issue hasn’t been addressed in a long time. If someone wants to create a CodePen related to this issue showing how var() usage works where this plugin does not or vice versa, we can continue the conversation. Otherwise, I will close this out next week.

chris-morgan commented 6 years ago

I gave a clear and precise example of it doing what is very obviously the wrong thing. CodePen is altogether unnecessary.

jonathantneal commented 6 years ago

In your first post, your example compiles as expected. The function can be parsed but the characters after it aren’t.

jonathantneal commented 6 years ago

I’ve been looking into this further, and I’ve noticed this plugin does handle whitespace, so I’ve added support for spec-valid whitespace per https://www.w3.org/TR/css-syntax-3/#whitespace-diagram

jonathantneal commented 6 years ago

@chris-morgan, are you still having this issue in the latest 6.3.x?

chris-morgan commented 6 years ago

@jonathantneal I tried 7.0.0 and it seems to be fixed. Thanks!

The spare closing parentheses are still permitted and behave weirdly, where I’d have thought they should be a syntax error instead, but I’ll let that pass.