Skip to content

fix(compiler): strip ::ng-deep from nested selectors #69959

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
crisbeto wants to merge 1 commit into from
Closed

fix(compiler): strip ::ng-deep from nested selectors #69959

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/compiler/src/shadow_css.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -602,6 +602,10 @@ export class ShadowCss {
hostSelector,
isParentSelector: true,
});

// TODO(crisbeto): this is temporary to help us land #69885.
// Strip ::ng-deep from nested content.
content = rule.content.replace(_shadowDeepSelectors, ' ');

@mattrbeck mattrbeck Jul 28, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 }}

@crisbeto crisbeto Jul 28, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

} else if (scopedAtRuleIdentifiers.some((atRule) => rule.selector.startsWith(atRule))) {
content = this._scopeSelectors(rule.content, scopeSelector, hostSelector);
} else if (rule.selector.startsWith('@font-face') || rule.selector.startsWith('@page')) {
Expand Down
66 changes: 66 additions & 0 deletions packages/compiler/test/shadow_css/ng_deep_spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,70 @@ describe('ShadowCss, ng-deep', () => {
css = ':host > ::ng-deep > .x {}';
expect(shim(css, 'contenta', 'h')).toEqualCss('[h] > > .x {}');
});

// TODO(crisbeto): this is temporary until we land #69885.
it('should strip ::ng-deep from nested selectors', () => {
const css = `
.parent {
::ng-deep .child {
color: red;
}

.wrapper {
&::ng-deep .inner {

@mattrbeck mattrbeck Jul 28, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

color: blue;
}

&:hover ::ng-deep .inner-hover {
color: green;
}

.deep {
::ng-deep .deepest {
color: yellow;
}
}
}
}

@media screen and (max-width: 600px) {
.media-parent {
::ng-deep .media-child {
color: purple;
}
}
}
`;
expect(shim(css, 'contenta')).toEqualCss(`
.parent[contenta] {
.child {
color: red;
}

.wrapper {
& .inner {
color: blue;
}

&:hover .inner-hover {
color: green;
}

.deep {
.deepest {
color: yellow;
}
}
}
}

@media screen and (max-width: 600px) {
.media-parent[contenta] {
.media-child {
color: purple;
}
}
}
`);
});
});
Loading