Add support for nested selectors

[ocoste]

When I parse a CSS with nested selector, the rendered CSS bugs

Example :

$css = "
.presentations_frameEditorItem {
    position: relative;
    a {
        border: 1px solid var(--theme-primary-border);
    }

    &.presentations_isDragging {
        opacity: 0
    }
}
";

$parser = (new Parser($css))->parse();

$css = $parser->render();

---

.presentations_frameEditorItem {
position: relative;
border: 1px solid var(--theme-primary-border);
}

How can we prevent this?

Thanks

[oliverklee]

Thanks for reporting this!

This seems to be a missing feature. I've tagged and renamed this issue accordingly.

[JakeQZ]

This shouldn't be too tricky to implement. These are my initial thoughts:

• RuleSet::$rules elements would need to be allowed to be either a Rule or a DeclarationBlock:
  • A (possibly-empty) interface RuleOrDeclarationBlock (for want of a better name) can be introduced for type-strictness;
  • This may allow creation of invalid constructs, such as a declaration block inside an at-rule where it is not permitted, but it would be caveat emptor - invalid constructs would be rejected at the parsing phase, and only possible via manipulation;
• RuleSet::parseRuleSet() would need an extra Boolean parameter to permit declaration blocks as well as property-value rules:
  • Only DeclarationBlock::parse() would pass this as true;
  • Determining whether the next item is a rule or a nested declaration block is slightly tricky:
    • The next characters of interest that would determine which it is are { (for a declaration block) and ; or } for a rule;
    • But these could be contained within a string in either situation (e.g. content property for a rule, or attribute selector), so proper parsing is required;
    • We could attempt to initially parse as a rule, but a:hover would pass unless we then checked for a { coming later;
    • Or we could attempt to initially parse as a declaration block, which should always fail if there's no {;
    • I think the latter seems more reliable;
    • Either way, I think it would involve catching an exception during normal parsing operation, which I'm not keen on, because exceptions should only be used for exceptional situations, but I can't think of a way of easily avoiding that.
• Rendering should take care of itself and need no changes.

@oliverklee, @sabberworm, WDYT?

[JakeQZ]

Though RuleSet shouldn't need to know about DeclarationBlock, as the latter is a subclass (and if it does, there's a circular dependecy). Instead it should have methods that DeclarationBlock can override.

[JakeQZ]

• A (possibly-empty) interface RuleOrDeclarationBlock (for want of a better name) can be introduced for type-strictness

RuleSetItem?

[JakeQZ]

Ref: https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_nesting

[oliverklee]

RuleSetItem?

I like that. (And it should definitely be an interface, not a class.)

RuleSet::parseRuleSet() would need an extra Boolean parameter to permit declaration blocks as well as property-value rules:

I consider boolean parameters that change the general way a method works an anti-pattern. Instead, I propose we add a second, well-named method.

[JakeQZ]

RuleSet::parseRuleSet() would need an extra Boolean parameter to permit declaration blocks as well as property-value rules:

I consider boolean parameters that change the general way a method works an anti-pattern. Instead, I propose we add a second, well-named method.

I think RuleSet::parseRuleSet() would need to call an overrideable method parseRuleSetItem, which DeclarationBlock can override to parse a nested declaration block if that's what's next, whilst the base RuleSet only parses Rules. There's a potential issue with these methods being static, and I'd prefer to avoid late-static binding if possible. Will see what the situation is when I look into implementation. But this approach should avoid the anti-pattern you mention, as well as a the circular dependency I mentioned.

[oliverklee]

This is the code smell/antipattern I was referring to: https://luzkan.github.io/smells/flag-argument

[JakeQZ]

• A (possibly-empty) interface RuleOrDeclarationBlock (for want of a better name) can be introduced for type-strictness

RuleSetItem?

Actually, this isn't going to work like that (at all easily). RuleSet uses the Rules' property name as a key, and various methods use this to identify them. DeclarationBlocks don't have a property name.

Instead I think DeclarationBlock will need a separate list of DeclarationBlock children. When it comes to rendering, it won't matter if, say, all the Rules are output before all the DeclarationBlock children, because (I think) the children can't match any of the same elements (but will need to think a bit more to be sure).

[JakeQZ]

When it comes to rendering, it won't matter if, say, all the Rules are output before all the DeclarationBlock children, because (I think) the children can't match any of the same elements (but will need to think a bit more to be sure).

Actually the order can matter, in this contrived example using the universal selector:

body,
body * {
  * {
    color: red;
  }
  color: green;
}

Child elements of body will be coloured green. But if the nested declaration block comes after the rule, they will be red, because the universal selector does not affect specificity. Note that if * is changed to, say, p, the nested rule will always take precedence (if matched), since the outer specificity is taken from the most-specific selector, whether or not it is actually matched.

This is further complicated by the fact that the nested items don't have to be simple declaration blocks. Some at-rules are allowed.

One approach might be to change DeclarationBlock so that it extends CSSBlockList rather than RuleSet, has a RuleSet as a property instead, and for backward-compatibility provides the RuleSet methods chaining on to the inner RuleSet property. That would deal with the latter problem (perhaps along with discarding at-rules which are not allowed to be nested), and also retain satisfaction for the CSSBlockList::getAllDeclarationBlocks method description: "Gets all DeclarationBlock objects recursively, no matter how deeply nested the selectors are." There's still the ordering problem to deal with. But if everything has a line number, the whole jumbled mess can be put through a sort function at render() time. I'm thinking this is the best way forward, but will sleep on it...

[oliverklee]

Maybe we might put this on the back-burner and tackle this after a rewrite.