rtsao / csjs

:sparkles: Modular, scoped CSS with ES6
MIT License
576 stars 32 forks source link

Extends syntax conflicts with selector grouping #34

Closed sambs closed 7 years ago

sambs commented 8 years ago

I love how standard css seems to "just work" with csjs but I've noticed an ambiguity surrounding the extends syntax and css selector grouping, due to the use of commas to separate both selectors and extend targets.

.styleA,
.styleB {
  background: red;
}

.styleA extends .baseStyle,
.styleB {
  background: red;
}
rtsao commented 8 years ago

If I'm understanding you correctly, in the latter case you are hoping for something like below?:

.styleA extends .baseStyle,
.styleB {
  background: red;
}

yielding:

{"styleA": "styleA_scoped baseStyle_scoped", "styleB_scoped": "styleB_scoped"}
.styleA_scoped,
.styleB_scoped {
  background: red;
}

Where .styleA is a composition of .baseStyle and {background: red}, but .styleB is only {background: red}.

With regard to ambiguity, currently, only selectors with a single class selector can extend another class, and all selectors following an extends is greedily treated as part of the extends. This is certainly limiting, but I think technically unambiguous (albeit potentially confusing).

You could do something like this to achieve the aforementioned result:

.styleA,
.styleB {
  background: red;
}

.styleA extends .baseStyle {}

I'll admit this is a bit verbose.

I'm open to syntax alternatives that might be more powerful, but the current extension seems sufficient for most use cases and I'm hesitant to add complexity.

sambs commented 8 years ago

Sorry, I could've been clearer but you've got it. It's certainly something you can work around once you understand it but it seems like a tweak in syntax could remove the ambiguity. One option would be parentheses:

.styleA extends (.base1, .base) {
  background: red;
}

Or perhaps the comma between selectors isn't necessary at all?

.styleA extends .base1 .base {
  background: red;
}
sambs commented 8 years ago

Ambiguity isn't the right word, it's more about intuition - being able to guess the result of the syntax without looking it up in the docs or trying it out.

On futher thought, my second suggestion of dropping the commas makes .styleA look like it extends a descendent selector .base1 .base . Parentheses seem like a better choice and could be optional and therefore non-breaking.

rtsao commented 8 years ago

Cool, I had originally thought of using parens but had made the determination they weren't necessary -- but I hadn't considered the use case you brought up in this issue.

You make a good point about intuitiveness/clarity of developer intent vs ambiguity! I'll look into implementing parens syntax (probably do it this weekend). I suspect it should be pretty straightforward. But a PR with tests would also be happily accepted!

I think it'd be nice to make the syntax optional to avoid a breaking change, but at the same time maybe a breaking change is for the best for simplicity (of implementation and also consistency of syntax).

sambs commented 8 years ago

Great! I can take a look at the tests.

I've realised parens couldn't really be optional. They needn't be required when extending from a single rule but would be when extending from many else we're back to the current situation.

rtsao commented 7 years ago

Closing this for now. I think it's worth investigating syntax in a future version, where maybe we could make a breaking change.