fix(issue:4211) parse entities in comma list

[puckowski]

correctly parse all entities in a comma separated list such that all URLs are rewritten correctly

Resolves: #4211

Given:

root
├─index.html
├─asset
|   ├─rabbit
|   |    ├─hello.jpg
├─style
|   ├─rabbit
|   |    ├─index.less
|   |    ├─styles.less     

where index.less imports styles.less and styles.less has the following:

#root {
  --bg-compose: url('../../asset/rabbit/hello.jpg') no-repeat top center 100% 68px,
    url('../../asset/rabbit/hello.jpg') no-repeat 0px 67px 100% calc(100% - 67px - 67px),
    url('../../asset/rabbit/hello.jpg') no-repeat bottom center 100% 68px;
}

and index.html includes index.less and is served using ws, the output is:

#root {
  --bg-compose: url('http://192.168.0.17:8000/asset/rabbit/hello.jpg') no-repeat top center 100% 68px, url('http://192.168.0.17:8000/asset/rabbit/hello.jpg') no-repeat 0px 67px 100% calc(100% - 67px - 67px), url('http://192.168.0.17:8000/asset/rabbit/hello.jpg') no-repeat bottom center 100% 68px;
}

I ran all tests locally and they pass.

What:

Not all entities are parsed correctly in a comma separated list and may result in incorrect CSS output.

Why:

Users expect correct CSS output when using comma separated lists with valid CSS syntax.

Checklist:

• Documentation
• Added/updated unit tests
• Code complete

[matthew-dean]

@puckowski
I think this problem is more complex and probably needs re-thinking.

When I did the original custom property implementation to unblock people using custom properties, it was a somewhat naive implementation.

Here is the original problem:

1. Custom properties allow almost anything, but
2. In Less, property value nodes (and sub-nodes) have automatic resolution behavior.

My naive implementation worked like this:

1. Try to parse as much as possible as distinct nodes.
2. When #1 fails, just parse the rest as an "escaped" quote, following the basic rules of custom property value syntax (which is quite permissive).
3. Allow some simple substitutions of variables

I would now classify this as a bad solution, and:

1. we should immediately deprecate it,
2. we should break this behavior in the future

Why the current implementation is bad

Even though a custom property can superficially look like a CSS property, it isn't, and its parsing rules are not similar. CSS has no unified syntax; it is a collection of "micro-syntaxes" and the syntax rules are flipped on/off depending on what preceded it.

Take this example:

.box {
  @width: 30px;
  --my-width: @width;
}

What should happen? Currently, we can superficially imagine that the author wants to treat this like a regular property and sub in the variable value. We get this output:

.box {
  --my-width: 30px;
}

However, this too is a valid custom property:

.box {
  --my-custom-syntax: #define @query => (val + 30);
}

It's unclear when an author is writing a custom property value if they intend to use JavaScript or a CSS worklet to interpret the values differently, and, in general, those values should be preserved. (Incidentally, this is technically true of unknown at-rules, which have their own micro-syntaxes.) But, today, Less will throw an error that @query is not defined.

Instead, my feeling is that the custom property value node should be, entirely, an anonymous value with interpolated variable substitutions only. Meaning the author would be required to write:

.box {
  @width: 30px;
  --my-width: @{width};
}

@{[var]} is a much more specific syntax that indicates author intent (and aligns with Sass requirements for custom properties).

Related to this issue

So, as to the URL issue itself as it relates to lists, my feeling is that the fact that Less is re-writing URLs at all in a custom property is an unintended side-effect behavior, and the "fix" is not to rewrite more URLs, but to rewrite no URLs, because we do not know author intent. However, this would be a breaking change and should come with:
a) a deprecation period to tell people to write @{width} instead of @width (and supporting both).
b) a major version bump where only @{[var]}-type references are replaced in custom property values.