Allow value definitions with commas in them.

[tec27]

Feel free to tell me that this is dumb and I shouldn't be using values this way, but one thing I've wanted to use values for was to reuse some box-shadow definitions that contain multiple shadows. Declaring a box-shadow looks something like this:

.foo {
  box-shadow: 0 11px 15px -7px rgba(0,0,0,.2),
      0 24px 38px 3px rgba(0,0,0,.14);
}

I'd like to save the whole definition in a value, so that other modules that import that value don't have to e.g. know how many shadows it contains or the specifics about their structure:

@value myShadow: 0 11px 15px -7px rgba(0,0,0,.2), 0 24px 38px 3px rgba(0,0,0,.14);
.foo {
  box-shadow: myShadow;
}

Currently with this module, however, that's not possible, as value defintions cannot contain commas (except when they are inside parentheses). This can also cause problems with trying to define other multiple-value things, such as media query conditions (see #11 for other examples as well). I realize that I could also re-use box-shadow by composing a class that has that value set, but class composition doesn't always work perfectly for all purposes (i.e. when I need to apply a particular property value when an element is :active).

This PR makes the syntax for value definitions more permissive, essentially allowing anything to be contained in the actual value. To do so, it makes value declarations slightly more strict by removing the ability to declare multiple values within one @value statement.

i.e. before this PR:

@value foo: 7, bar: 8;

would give two values, foo and bar. After this PR, it would give only a single value, foo. (Imports still work as before, and multiple values can be imported with a single @value statement). This feature doesn't seem to be documented anywhere, so I'm not sure that its widely used anyway.

The benefit here is that values can now contain multiple comma-seperated items, which is very useful for reusing things like:

• Multiple box-shadow shadows
• Multiple transition specifications
• Multiple media query conditions (OR'd)

It does allow for a little bit of weirder usage in this, e.g. for specifying a subset of items in a comma-separated rgba value:

@value redAndGreen: 150, 150;
.foo { background-color: rgba(redAndGreen, 0, 1); }

I don't think that's a super-desirable usage, but I also don't think it really harms anything by being allowed, so I haven't attempted to mitigate it.

I think that allowing multiple value declarations in a single @value statement is possible to preserve while still allowing for comma-separate values, but would probably involve moving to an actual parser (doubtful that it can really be done in a regular expression). It might also involve mandating ':' usage, which seems to be undesirable for SASS compatibility

[tec27]

This would also fix #21.

[geelen]

Ok, so I definitely want to support your use case of @value myShadow: 0 11px 15px -7px rgba(0,0,0,.2), 0 24px 38px 3px rgba(0,0,0,.14); but I also really want @value a, b, c from "d.css"; i.e. multiple declarations on one line. This is maybe a problem that has come from combining value-as-import and value-as-declaration.

I'm thinking maybe quotes are a better alternative:

@value myShadow: "0 11px 15px -7px rgba(0,0,0,.2), 0 24px 38px 3px rgba(0,0,0,.14)", redAndGreen: "150,150";

At the moment quotes are not treat specially at all, but as #20 points out that is causing some weirdness. So I think we can come up with some special handling of quotes and special injection of quotes when passing them to future @value x from y and composes declarations.

Let me see if that works...

[tec27]

That solution sounds fine to me! That was actually something I had tried before writing this PR (but of course, as the code stands, it leaves the quotes in place when it replaces the value reference).

[geelen]

Ugh actually this is... annoying. We need a full parser. Otherwise we'll still hit edge cases. Background gradients, for example. Breaking on commas is just a bad idea.

I'm thinking now that there are two valid syntaxes:

@value valueName[:] literally anything . allowed , here;
@value valueName[, valueName2]+ from "./path.css";

So you never have to quote anything if it's a value definition, and you can import multiples for a value importation. Basically your PR is most of the way there. I might just add a test case for the multiple imports case.

[tivac]

@geelen It's pretty simple to support the simple local form (@value foo: ...;) using postcss-value-parser.

https://github.com/tivac/modular-css/blob/master/src/plugins/values-local.js#L17

I ended up writing a parser based on the tokens from postcss-value-parser for the composition form (@value a, b, c from "../fooga.css";). It's the only way I could rationalize about it sanely.

https://github.com/tivac/modular-css/blob/master/src/_composition.js

Not my favorite bit of code, but gets the job done.

[dfreeman]

@tec27 is there anything I can do to help move this forward? I like @geelen's proposed approach, and I'd love to have more freedom in @value values.

[max]

Any update on this? Can we help to move this forward?

[joshgillies]

Can we consider this PR ready to merge or is there still missing features/edge cases not catered for here @geelen ?

[joefiorini]

Ran across another use case for this feature: I want to compose media queries like:

@value --large-viewport: (min-width: 769px);
@value --medium-viewport: (min-width: 667px) and (max-width: 768px);
@value --medium-up: --medium-viewport, --large-viewport

The --medium-up value should transform into:

(min-width: 667px) and (max-width: 772px), (min-width: 769px)

but instead I just get (min-width: 667px) and (max-width: 768px).

[alvinvogelzang]

Would be nice if this PR will be merged soon. This is blocking us on ember-css-modules / salsify/ember-css-modules#22

[geelen]

Apologies for the delay! Pushed a fix in v1.2.0