Skip to content

Commit

Permalink
fix(compiler): scope :host-context inside pseudo selectors, do not de…
Browse files Browse the repository at this point in the history 
…crease specificity (angular#57796)

parse constructions like `:where(:host-context(.foo))` correctly
revert logic which lead to decreased specificity if `:where` was applied
to another selector, for example `div` is transformed to `div[contenta]`
with specificity of (0,1,1) so `div:where(.foo)` should not decrease it
leading to `div[contenta]:where(.foo)` with the same specificity (0,1,1)
instead of `div:where(.foo[contenta])` with specificity equal to (0,0,1)

PR Close angular#57796
  • Loading branch information
@GeorgySerga @devversion
GeorgySerga authored and devversion committed Oct 10, 2024
1 parent 69529d8 commit 46a6324
Show file tree
Hide file tree
Showing 3 changed files with 87 additions and 34 deletions.
45 changes: 23 additions & 22 deletions packages/compiler/src/shadow_css.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -541,7 +541,7 @@ export class ShadowCss {
* .foo<scopeName> .bar { ... }
*/
private _convertColonHostContext(cssText: string): string {
return cssText.replace(_cssColonHostContextReGlobal, (selectorText) => {
return cssText.replace(_cssColonHostContextReGlobal, (selectorText, pseudoPrefix) => {
// We have captured a selector that contains a `:host-context` rule.

// For backward compatibility `:host-context` may contain a comma separated list of selectors.
Expand Down Expand Up @@ -596,10 +596,12 @@ export class ShadowCss {
}

// The context selectors now must be combined with each other to capture all the possible
// selectors that `:host-context` can match. See `combineHostContextSelectors()` for more
// selectors that `:host-context` can match. See `_combineHostContextSelectors()` for more
// info about how this is done.
return contextSelectorGroups
.map((contextSelectors) => combineHostContextSelectors(contextSelectors, selectorText))
.map((contextSelectors) =>
_combineHostContextSelectors(contextSelectors, selectorText, pseudoPrefix),
)
.join(', ');
});
}
Expand Down Expand Up @@ -802,18 +804,12 @@ export class ShadowCss {
_cssPrefixWithPseudoSelectorFunction,
);
if (cssPrefixWithPseudoSelectorFunctionMatch) {
const [cssPseudoSelectorFunction, mainSelector, pseudoSelector] =
cssPrefixWithPseudoSelectorFunctionMatch;
const hasOuterHostNoCombinator = mainSelector.includes(_polyfillHostNoCombinator);
const scopedMainSelector = mainSelector.replace(
_polyfillExactHostNoCombinatorReGlobal,
`[${hostSelector}]`,
);

// Unwrap the pseudo selector, to scope its contents.
const [cssPseudoSelectorFunction] = cssPrefixWithPseudoSelectorFunctionMatch;

// Unwrap the pseudo selector to scope its contents.
// For example,
// - `:where(selectorToScope)` -> `selectorToScope`;
// - `div:is(.foo, .bar)` -> `.foo, .bar`.
// - `:is(.foo, .bar)` -> `.foo, .bar`.
const selectorToScope = selectorPart.slice(cssPseudoSelectorFunction.length, -1);

if (selectorToScope.includes(_polyfillHostNoCombinator)) {
Expand All @@ -827,9 +823,7 @@ export class ShadowCss {
});

// Put the result back into the pseudo selector function.
scopedPart = `${scopedMainSelector}:${pseudoSelector}(${scopedInnerPart})`;

this._shouldScopeIndicator = this._shouldScopeIndicator || hasOuterHostNoCombinator;
scopedPart = `${cssPseudoSelectorFunction}${scopedInnerPart})`;
} else {
this._shouldScopeIndicator =
this._shouldScopeIndicator || selectorPart.includes(_polyfillHostNoCombinator);
Expand Down Expand Up @@ -967,7 +961,8 @@ class SafeSelector {
}
}

const _cssPrefixWithPseudoSelectorFunction = /^([^:]*):(where|is)\(/i;
const _cssScopedPseudoFunctionPrefix = '(:(where|is)\\()?';
const _cssPrefixWithPseudoSelectorFunction = /^:(where|is)\(/i;
const _cssContentNextSelectorRe =
/polyfill-next-selector[^}]*content:[\s]*?(['"])(.*?)\1[;\s]*}([^{]*?){/gim;
const _cssContentRuleRe = /(polyfill-rule)[^}]*(content:[\s]*(['"])(.*?)\3)[;\s]*[^}]*}/gim;
Expand All @@ -978,13 +973,15 @@ const _polyfillHost = '-shadowcsshost';
const _polyfillHostContext = '-shadowcsscontext';
const _parenSuffix = '(?:\\((' + '(?:\\([^)(]*\\)|[^)(]*)+?' + ')\\))?([^,{]*)';
const _cssColonHostRe = new RegExp(_polyfillHost + _parenSuffix, 'gim');
const _cssColonHostContextReGlobal = new RegExp(_polyfillHostContext + _parenSuffix, 'gim');
const _cssColonHostContextReGlobal = new RegExp(
_cssScopedPseudoFunctionPrefix + '(' + _polyfillHostContext + _parenSuffix + ')',
'gim',
);
const _cssColonHostContextRe = new RegExp(_polyfillHostContext + _parenSuffix, 'im');
const _polyfillHostNoCombinator = _polyfillHost + '-no-combinator';
const _polyfillHostNoCombinatorWithinPseudoFunction = new RegExp(
`:.*\\(.*${_polyfillHostNoCombinator}.*\\)`,
);
const _polyfillExactHostNoCombinatorReGlobal = /-shadowcsshost-no-combinator/g;
const _polyfillHostNoCombinatorRe = /-shadowcsshost-no-combinator([^\s]*)/;
const _polyfillHostNoCombinatorReGlobal = new RegExp(_polyfillHostNoCombinatorRe, 'g');
const _shadowDOMSelectorsRe = [
Expand Down Expand Up @@ -1237,7 +1234,11 @@ function unescapeQuotes(str: string, isQuoted: boolean): string {
* @param contextSelectors an array of context selectors that will be combined.
* @param otherSelectors the rest of the selectors that are not context selectors.
*/
function combineHostContextSelectors(contextSelectors: string[], otherSelectors: string): string {
function _combineHostContextSelectors(
contextSelectors: string[],
otherSelectors: string,
pseudoPrefix = '',
): string {
const hostMarker = _polyfillHostNoCombinator;
_polyfillHostRe.lastIndex = 0; // reset the regex to ensure we get an accurate test
const otherSelectorsHasHost = _polyfillHostRe.test(otherSelectors);
Expand Down Expand Up @@ -1266,8 +1267,8 @@ function combineHostContextSelectors(contextSelectors: string[], otherSelectors:
return combined
.map((s) =>
otherSelectorsHasHost
? `${s}${otherSelectors}`
: `${s}${hostMarker}${otherSelectors}, ${s} ${hostMarker}${otherSelectors}`,
? `${pseudoPrefix}${s}${otherSelectors}`
: `${pseudoPrefix}${s}${hostMarker}${otherSelectors}, ${pseudoPrefix}${s} ${hostMarker}${otherSelectors}`,
)
.join(',');
}
Expand Down
36 changes: 36 additions & 0 deletions packages/compiler/test/shadow_css/host_and_host_context_spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,42 @@ describe('ShadowCss, :host and :host-context', () => {
});

describe(':host-context', () => {
it('should transform :host-context with pseudo selectors', () => {
expect(
shim(':host-context(backdrop:not(.borderless)) .backdrop {}', 'contenta', 'hosta'),
).toEqualCss(
'backdrop:not(.borderless)[hosta] .backdrop[contenta], backdrop:not(.borderless) [hosta] .backdrop[contenta] {}',
);
expect(shim(':where(:host-context(backdrop)) {}', 'contenta', 'hosta')).toEqualCss(
':where(backdrop[hosta]), :where(backdrop [hosta]) {}',
);
expect(shim(':where(:host-context(outer1)) :host(bar) {}', 'contenta', 'hosta')).toEqualCss(
':where(outer1) bar[hosta] {}',
);
expect(
shim(':where(:host-context(.one)) :where(:host-context(.two)) {}', 'contenta', 'a-host'),
).toEqualCss(
':where(.one.two[a-host]), ' + // `one` and `two` both on the host
':where(.one.two [a-host]), ' + // `one` and `two` are both on the same ancestor
':where(.one .two[a-host]), ' + // `one` is an ancestor and `two` is on the host
':where(.one .two [a-host]), ' + // `one` and `two` are both ancestors (in that order)
':where(.two .one[a-host]), ' + // `two` is an ancestor and `one` is on the host
':where(.two .one [a-host])' + // `two` and `one` are both ancestors (in that order)
' {}',
);
expect(
shim(':where(:host-context(backdrop)) .foo ~ .bar {}', 'contenta', 'hosta'),
).toEqualCss(
':where(backdrop[hosta]) .foo[contenta] ~ .bar[contenta], :where(backdrop [hosta]) .foo[contenta] ~ .bar[contenta] {}',
);
expect(shim(':where(:host-context(backdrop)) :host {}', 'contenta', 'hosta')).toEqualCss(
':where(backdrop) [hosta] {}',
);
expect(shim('div:where(:host-context(backdrop)) :host {}', 'contenta', 'hosta')).toEqualCss(
'div:where(backdrop) [hosta] {}',
);
});

it('should handle tag selector', () => {
expect(shim(':host-context(div) {}', 'contenta', 'a-host')).toEqualCss(
'div[a-host], div [a-host] {}',
Expand Down
40 changes: 28 additions & 12 deletions packages/compiler/test/shadow_css/shadow_css_spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,19 +69,14 @@ describe('ShadowCss', () => {
expect(shim('[attr] {}', 'contenta')).toEqualCss('[attr][contenta] {}');
});

it('should transform :host and :host-context with attributes and pseudo selectors', () => {
it('should transform :host with attributes', () => {
expect(shim(':host [attr] {}', 'contenta', 'hosta')).toEqualCss('[hosta] [attr][contenta] {}');
expect(shim(':host(create-first-project) {}', 'contenta', 'hosta')).toEqualCss(
'create-first-project[hosta] {}',
);
expect(shim(':host[attr] {}', 'contenta', 'hosta')).toEqualCss('[attr][hosta] {}');
expect(shim(':host[attr]:where(:not(.cm-button)) {}', 'contenta', 'hosta')).toEqualCss(
'[hosta][attr]:where(:not(.cm-button)) {}',
);
expect(
shim(':host-context(backdrop:not(.borderless)) .backdrop {}', 'contenta', 'hosta'),
).toEqualCss(
'backdrop:not(.borderless)[hosta] .backdrop[contenta], backdrop:not(.borderless) [hosta] .backdrop[contenta] {}',
'[attr][hosta]:where(:not(.cm-button)) {}',
);
});

Expand All @@ -94,6 +89,27 @@ describe('ShadowCss', () => {
expect(shim('.one\\:two .three\\:four {}', 'contenta')).toEqualCss(
'.one\\:two[contenta] .three\\:four[contenta] {}',
);
expect(shim('div:where(.one) {}', 'contenta', 'hosta')).toEqualCss(
'div[contenta]:where(.one) {}',
);
expect(shim('div:where() {}', 'contenta', 'hosta')).toEqualCss('div[contenta]:where() {}');
// See `xit('should parse concatenated pseudo selectors'`
expect(shim(':where(a):where(b) {}', 'contenta', 'hosta')).toEqualCss(
':where(a)[contenta]:where(b) {}',
);
expect(shim('*:where(.one) {}', 'contenta', 'hosta')).toEqualCss('*[contenta]:where(.one) {}');
expect(shim('*:where(.one) ::ng-deep .foo {}', 'contenta', 'hosta')).toEqualCss(
'*[contenta]:where(.one) .foo {}',
);
});

xit('should parse concatenated pseudo selectors', () => {
// Current logic leads to a result with an outer scope
// It could be changed, to not increase specificity
// Requires a more complex parsing
expect(shim(':where(a):where(b) {}', 'contenta', 'hosta')).toEqualCss(
':where(a[contenta]):where(b[contenta]) {}',
);
});

it('should handle pseudo functions correctly', () => {
Expand Down Expand Up @@ -136,7 +152,7 @@ describe('ShadowCss', () => {
expect(shim(':where([foo]) {}', 'contenta', 'hosta')).toEqualCss(':where([foo][contenta]) {}');

// :is()
expect(shim('div:is(.foo) {}', 'contenta', 'a-host')).toEqualCss('div:is(.foo[contenta]) {}');
expect(shim('div:is(.foo) {}', 'contenta', 'a-host')).toEqualCss('div[contenta]:is(.foo) {}');
expect(shim(':is(.dark :host) {}', 'contenta', 'a-host')).toEqualCss(':is(.dark [a-host]) {}');
expect(shim(':is(.dark) :is(:host) {}', 'contenta', 'a-host')).toEqualCss(
':is(.dark) :is([a-host]) {}',
Expand Down Expand Up @@ -182,12 +198,12 @@ describe('ShadowCss', () => {
'hosta',
),
).toEqualCss(
':where(:where(a[contenta]:has(.foo), b[contenta]) :is(.one[contenta], .two:where(.foo[contenta] > .bar[contenta]))) {}',
':where(:where(a[contenta]:has(.foo), b[contenta]) :is(.one[contenta], .two[contenta]:where(.foo > .bar))) {}',
);

// complex selectors
expect(shim(':host:is([foo],[foo-2])>div.example-2 {}', 'contenta', 'a-host')).toEqualCss(
'[a-host]:is([foo], [foo-2]) > div.example-2[contenta] {}',
'[a-host]:is([foo],[foo-2]) > div.example-2[contenta] {}',
);
expect(shim(':host:is([foo], [foo-2]) > div.example-2 {}', 'contenta', 'a-host')).toEqualCss(
'[a-host]:is([foo], [foo-2]) > div.example-2[contenta] {}',
Expand Down Expand Up @@ -220,11 +236,11 @@ describe('ShadowCss', () => {
'.header[contenta]:not(.admin) {}',
);
expect(shim('.header:is(:host > .toolbar, :host ~ .panel) {}', 'contenta', 'hosta')).toEqualCss(
'.header:is([hosta] > .toolbar[contenta], [hosta] ~ .panel[contenta]) {}',
'.header[contenta]:is([hosta] > .toolbar, [hosta] ~ .panel) {}',
);
expect(
shim('.header:where(:host > .toolbar, :host ~ .panel) {}', 'contenta', 'hosta'),
).toEqualCss('.header:where([hosta] > .toolbar[contenta], [hosta] ~ .panel[contenta]) {}');
).toEqualCss('.header[contenta]:where([hosta] > .toolbar, [hosta] ~ .panel) {}');
expect(shim('.header:not(.admin, :host.super .header) {}', 'contenta', 'hosta')).toEqualCss(
'.header[contenta]:not(.admin, .super[hosta] .header) {}',
);
Expand Down

0 comments on commit 46a6324

Please sign in to comment.