Conversation
Adds some temporary logic to strip `::ng-deep` from nested selectors which will help us land angular#69885.
47f6c77 to
ddf740f
Compare
|
|
||
| // TODO(crisbeto): this is temporary to help us land #69885. | ||
| // Strip ::ng-deep from nested content. | ||
| content = rule.content.replace(_shadowDeepSelectors, ' '); |
There was a problem hiding this comment.
I'm not convinced by this approach. Happy to chat tomorrow if that's easier, but I'll try to express some of my thoughts. I've tested a bunch of this, but please feel free to check my assertions.
Take for example the input :host { .foo {} }. Today we write [host] { .foo {} }, simply not encapsulating beyond the outer selector. It's my understanding that this change allows us to write CSS like :host { ::ng-deep .foo {} } to make the encapsulation opt-out explicit, so that once we support nesting we can land it without changing behavior.
Couldn't we just use :host ::ng-deep { .foo {} } today? I think on main today this already compiles to [host] { .foo {} }. It seems like this isn't actually stable on #69885, but arguably it should be, since :host ::ng-deep { .foo {} } is semantically equivalent to :host ::ng-deep .foo {}. I think we need to update #69885 to fix this case rather than stripping ::ng-deep in this PR. This should be safe for an LSC in g3 too.
As an aside:
This gets tricky with selector lists, though. I think we want to make sure not to make a decision yet in cases like ::ng-deep, .foo { .bar {} }. Following CSS nesting rules, this actually desugars to :is(::ng-deep, .foo) .bar {}, which I suspect we should just produce an error for someday. This would be easier if/when we move to a real parser. That leaves us in a weird place for #69885 whatever we decide to do. In this case both encapsulating .bar and not scoping .bar is wrong. I'm leaning towards saying we should not encapsulate .bar, since that matches today's behavior of not encapsulating nested selectors. It's a little funky because :is() technically takes a forgiving selector list, but nested selectors don't actually map cleanly onto :is() semantics in that way. For example, the following produces red text, indicating that the nested rule is not forgiving in the way that :is() is.
<p><i>this is red:is(::ng-deep, p) i { color: red }
::ng-deep, p { i { color: blue }}There was a problem hiding this comment.
Ah it makes sense that :host ::ng-deep { .foo {} } should be treated as :host ::ng-deep .foo {}. I'll update the other PR and see if that helps me deal with the test failures.
| } | ||
|
|
||
| .wrapper { | ||
| &::ng-deep .inner { |
There was a problem hiding this comment.
These cases have been tricky for ACX in the past. Sometimes you have a Sass mixin like
@mixin foo($sel) {
:host {
&$sel {
color: red;
}
}
}In this case if you call foo with a selector beginning with :ng-deep, you get different semantics since the approach demonstrated by this test would add a descendant ( ) combinator. I had to spend a good chunk of time uncovering intended behavior in g3 to get this right for ACX. I think we also want to make ::ng-deep touching a nesting selector a strict error in the future.
Adds some temporary logic to strip
::ng-deepfrom nested selectors which will help us land #69885.