Limit var definitions to :root

[necolas]

• Enforce global variables by ignoring anything not declared in :root.
• Collect all :root var definitions before replacement, to mimic all-at-once resolution of variable precedence.

[ianstormtaylor]

for reference, https://gist.github.com/chriseppstein/8013073

[ianstormtaylor]

oh and would also solve this, https://gist.github.com/chriseppstein/8012929

[chriseppstein]

IMO, if you just ignore scoped variable values, you're still hurting more than helping. It should be an error or it should go into "hands-off" mode for that variable and leave it to the browser to resolve.

Here's another brain teaser:

:root {
  var-grid-width: 40px;
}

@media screen and (max-device-width: 320px) {
  :root {
    var-grid-width: 10px;
  }
}

div.span-5 {
  width: calc(var(grid-width) * 5);
}

I'd expect the following output:

div.span-5 {
  width: 200px;
}
@media screen and (max-device-width: 320px) {
  div.span-5 {
    width: 50px;
  }
}

But instead, you get:

@media screen and (max-device-width: 320px) {

}

div.span-5 {
  width: 50px;
}

[necolas]

I'd expect the following output

Why would you expect a new rule to appear in the media query?

Not having the variable value leak out sounds right though.

[tabatkins]

Why would you expect a new rule to appear in the media query?

Because that faithfully reproduces the result of redefining the variable. You either have to do that (replacing each in-@media var-* with a duplicate of every style that uses the variable), or flag var-* in @media as an error because it can't be faithfully supported.

[necolas]

ah, i see. thanks

[necolas]

Issue for the media query problem: #17. Appreciate you guys pointing out that shortcoming.

For now, in the pull request for this issue I'm going to throw an error for any vars not in a simple rule (as defined by the css-parse AST). That way we can at least review the patch for the initial issues, protect against the MQ one, and then spend more time thinking about what to do for media queries going forward.

[xzyfer]

I tend to agree with @necolas here, I think the current behaviour shown by @chriseppstein is sane. I would expect that if I hadn't defined div.span-5 in my media query it wouldn't be outputted.

Also, it should be up to me to decide how I want to use the variable in that context, assuming calc(var(grid-width) * 5); is the width I want in all @media contexts is presumptuous. If wanted my grid columns to fallback to width: 100% in a smaller media query this assumption would now be incorrect.

Edit: this would be true if the math wasn't borked.

[kaelig]

I must say I'm with @chriseppstein on this one. I know you know it already, but re-working complex low-level language constructs on top of the same syntax is a tricky game to play and you'll have this type of problem again and again along the way. If Rework has clear advantages over classic CSS pre-processors, its shortcomings on this type of matter need to be very clearly addressed so they are not endangering the future-proofness of the produced CSS.

[necolas]

The overwriting bug in this plugin came from attempting to provide an implementation consistent with the prefix-free variable plugin. I've opened a companion issue against that project: LeaVerou/prefixfree#178

These issues should be resolved by 1ec1555 and the fix available in 2.0.0. Thanks for the help.