From 46a6324c82a41b69c16a4c8c9f3fc52d1ecf6917 Mon Sep 17 00:00:00 2001 From: Georgy Serga Date: Thu, 26 Sep 2024 21:25:44 +0000 Subject: [PATCH] fix(compiler): scope :host-context inside pseudo selectors, do not decrease specificity (#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 #57796 --- packages/compiler/src/shadow_css.ts | 45 ++++++++++--------- .../shadow_css/host_and_host_context_spec.ts | 36 +++++++++++++++ .../test/shadow_css/shadow_css_spec.ts | 40 ++++++++++++----- 3 files changed, 87 insertions(+), 34 deletions(-) diff --git a/packages/compiler/src/shadow_css.ts b/packages/compiler/src/shadow_css.ts index 76007cf03d99aa..1d7988568fbf49 100644 --- a/packages/compiler/src/shadow_css.ts +++ b/packages/compiler/src/shadow_css.ts @@ -541,7 +541,7 @@ export class ShadowCss { * .foo .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. @@ -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(', '); }); } @@ -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)) { @@ -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); @@ -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; @@ -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 = [ @@ -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); @@ -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(','); } diff --git a/packages/compiler/test/shadow_css/host_and_host_context_spec.ts b/packages/compiler/test/shadow_css/host_and_host_context_spec.ts index c4dc29c372b4e8..5b5689feb2be1e 100644 --- a/packages/compiler/test/shadow_css/host_and_host_context_spec.ts +++ b/packages/compiler/test/shadow_css/host_and_host_context_spec.ts @@ -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] {}', diff --git a/packages/compiler/test/shadow_css/shadow_css_spec.ts b/packages/compiler/test/shadow_css/shadow_css_spec.ts index 452e0393547f0c..77a0a361a31992 100644 --- a/packages/compiler/test/shadow_css/shadow_css_spec.ts +++ b/packages/compiler/test/shadow_css/shadow_css_spec.ts @@ -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)) {}', ); }); @@ -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', () => { @@ -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]) {}', @@ -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] {}', @@ -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) {}', );