[CSS] Prettier converts "invalid" no-space value to a "valid" space value (changes the meaning/behavior of CSS code)

[trusktr]

Prettier 2.0.5
Playground link

--parser css

Input:

.foo {
  --foo:; /* This should not change. It is valid "invalid" code, per CSS spec. */
  background-color: var(--foo, red);
}

Output:

.foo {
  --foo: ; /* This should not change. It is valid "invalid" code, per CSS spec. */
  background-color: var(--foo, red);
}

Expected behavior:

An valid "invalid" CSS property is converted into a "valid" CSS property containing a value of a single space.

Before:

https://codepen.io/trusktr/pen/2507ea1e5ecb8b8a41800335fc327349

After:

https://codepen.io/trusktr/pen/5518a13792dda7288323eee272b0b9a3

A value like --foo:; is "invalid" CSS, but converting it to a "valid" value like --foo: ; introduces a new value that changes code behavior and thus changes visual output.

It is important not to change the code behavior.

---

Sidenote: I stumbled on this info while playing with the Space Toggle technique

I found this while playing with the awesome "Space Toggle" technique.

The space toggle technique is used in @James0x57's css-media-vars lib, to provide variables that make it easier to write media queries within existing CSS code structure without new @media blocks, thus without having to break code structure apart and keeping it DRY.

For example (from the README), code like

  .breakpoints-demo > * {
    width: 100%;
    background: red;
  }
  @media (min-width: 37.5em) and (max-width: 56.249em) {
    .breakpoints-demo > * {
      width: 49%;
    }
  }
  @media (min-width: 56.25em) and (max-width: 74.99em) {
    .breakpoints-demo > * {
      width: 32%;
    }
  }
  @media (min-width: 56.25em) {
    .breakpoints-demo > * {
      background: green;
    }
  }
  @media (min-width: 75em) {
    .breakpoints-demo > * {
      width: 24%;
    }
  }

can be written as

  @import 'https://unpkg.com/css-media-vars@1.0.1/css-media-vars.css'; /* <-- Amazing, check this out. */

  .breakpoints-demo > * {
    --xs-width: var(--media-xs) 100%;
    --sm-width: var(--media-sm) 49%;
    --md-width: var(--media-md) 32%;
    --lg-width: var(--media-gte-lg) 24%;
    width: var(--xs-width, var(--sm-width, var(--md-width, var(--lg-width))));

    --sm-and-down-bg: var(--media-lte-sm) red;
    --md-and-up-bg: var(--media-gte-md) green;
    background: var(--sm-and-down-bg, var(--md-and-up-bg));
  }

This allows the media queries to be co-located in the CSS structure, similar to what we can do with Sass, Less, and Stylus, but this relies on standard CSS spec.

As another example, I had some desktop-only code,

#faces {
	position: absolute;
	top: 50%;
	left: 50%;
	transform: translate(-50%, -50%);
	touch-action: none;
	display: flex;
}

and I added mobile support like this:

#faces {
	position: absolute;
	top: 50%;
	left: 50%;
	transform: translate(-50%, -50%);
	touch-action: none;
	display: flex;
	--layout-mobile: var(--media-xs) column;
	--layout-desktop: var(--media-gte-sm) row;
	flex-direction: var(--layout-mobile, var(--layout-desktop));
}

The above css-media-var examples don't have to do with the issue directly, but I had noticed that an invalid value could be converted to a valid value that could override a previously valid value (like an initial value).

More info on Space Toggles:

• https://twitter.com/James0x57/status/1282303255826046977 - @James0x57's introductory tweet (actually quite complicated for casual CSS users)
• https://twitter.com/James0x57/status/1283596399196680192 - Tweet with a simpler example. Relating to the main issue, try changing --toggler: ; to --toggler:; in the CSS code and the visual outcome will change.
• https://propjockey.github.io/bcd7sdd/ - Uses space toggles to create a circuit board. The binary inputs (check boxes in this case, but vars could also be set from JS) lead to rendering a number like an old digital clock.
• https://twitter.com/James0x57/status/1283906181031960576 - Tweet introducing css-media-vars, with discussion on how the "Space Toggle" technique per spec, not relying on bugs or hacks.

Very neat outcomes, especially the css-media-vars!

Anywho, Space Toggle is only slightly related to the --foo:; issue, but it's how I stumbled on it.

[alexander-akait]

Ideally, the parser should throw the error in this case, it is not valid syntax

[trusktr]

It is valid "invalid" syntax (meaning the user can rely on the invalid syntax just like they can get away with malformed HTML). I don't think the parser should throw an error, just like browsers don't.

[thorn0]

It is valid "invalid" syntax

What's invalid "invalid" syntax then?

[trusktr]

Plus it would be annoying for someone to run prettier in a fully-working project for the first time and it fails.

[thorn0]

meaning the user can rely on the invalid syntax

To rely on it to achieve what exactly? The use case doesn't seem compelling unless I miss something.

[JaneOri]

If a user has this invalid value syntax in their CSS:
--css-var:;
the current behavior is to convert it to this:
--css-var: ;
which gives the variable a valid state and changes behavior

If it can't just leave the invalid syntax in place, what it should do instead is convert it to this:
--css-var: initial;
because initial is guaranteed invalid in the spec, which is the same end behavior as an error, but now the syntax is valid.

[thorn0]

So there is no specific use case in which this behavior is useful. It indeed should be a syntax error. Why would people have this in their CSS files?

[JaneOri]

I do not recommenced anyone relies on behavior of an invalid syntax such as --css-var:; but the "guaranteed invalid" state of a CSS var set to initial (or never set) at compute time, is likely the expectation they'd have if they do use it.

Invalid css-vars trigger fallback behavior when referenced. Any css var that has an initial valued var anywhere in the dependency tree (at compute time) will be treated as initial. It's very useful.

Recently it's been in the spotlight because of the CSS Space Toggle logic that becomes possible. Here's a summary of it: https://github.com/propjockey/css-sweeper#basics-of-space-toggle

[JaneOri]

Easier to see here:

	Syntax	Compute Time
--css-var:;	invalid	invalid	user input
--css-var: ;	valid	valid	current output
--css-var: initial;	valid	invalid	better output

Validity at compute time is the most important since var(--fallback, behavior) depends on it being invalid at compute time.

[alexander-akait]

@James0x57 Yes, only --css-var: ; is valid, why you file contains --css-var:;?

[JaneOri]

@evilebottnawi
--css-var: initial; is also valid (and useful) - it clears whatever the variable is set to for elements that match the selector.
(CSS var's "invalid at compute time" is a feature and should not be changed by any build process)

I believe @trusktr is using --css-var:; to declare the variable like you would in JS.

The current behavior of Prettier is to change --css-var:; to --css-var: ; which completely changes the behavior of all css that references it.

If Prettier changed --css-var:; into --css-var: initial; instead, the behavior in the CSS would not change from the author's intention.

I can demonstrate this in a jsbin if it would be helpful?

[JaneOri]

Actually, an even more accurate replacement could be wrapping it in a comment if you're not opposed to that because
--css-var:;
would not override if it's already set to a value,
I believe OP was just using the syntax to sort of declare them the first time they're potentially going to be used.
--css-var:; -> /* --css-var:; */

[alexander-akait]

If Prettier changed --css-var:; into --css-var: initial; instead, the behavior in the CSS would not change from the author's intention.

No, --css-var:; is not --css-var: initial;, we don't modify AST, you can use linter for this. And it is invalid, initial is normal value for any property.

I believe OP was just using the syntax to sort of declare them the first time they're potentially going to be used.
--css-var:; -> /* --css-var:; */

No, we don't modify AST.

Nobody argues that this changes logic.

Questions - Why it was in source code. In this case we should throw an error. It is invalid code and parser should not parse it

[trusktr]

Why would people have this in their CSS files?

I believe OP was just using the syntax to sort of declare them the first time they're potentially going to be used.
--css-var:; -> /* --css-var:; */

Yeah, basically to implement default value handling while showing a list of available variables (or to execute boolean logic), as described in the Space Toggle documentation that @James0x57 linked to above, for example.

I do agree, converting --foo:; to --foo: initial; is better if the behavior is identical. Then no matter the reason (and the build tool should not care about the reason) the user's code's behavior will not be modified.

Prettier should never modify code behavior.

No, we don't modify AST.

Prettier is modifying the AST that the browser parses.

Questions - Why it was in source code. In this case we should throw an error. It is invalid code and parser should not parse it

The explanation is above. I can write, I can ship it, and I can expect the app to behave exactly a certain way...

...then Prettier breaks the app.

The --foo: initial; workaround works fine though.