Skip to content
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

Jump to bottom

fix(compiler): maintain multiline CSS selectors during CSS scoping #55509

Closed
wants to merge 1 commit into from
Closed

fix(compiler): maintain multiline CSS selectors during CSS scoping #55509

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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 13 additions & 8 deletions packages/compiler/src/shadow_css.ts
View file
Open in desktop
Expand Up @@ -658,8 +658,8 @@ export class ShadowCss {

private _scopeSelector(selector: string, scopeSelector: string, hostSelector: string): string {
return selector
.split(',')
.map((part) => part.trim().split(_shadowDeepSelectors))
.split(/ ?, ?/)
Copy link
Member

Choose a reason for hiding this comment

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

Should we use \s here rather than an actual space?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No otherwise the newlines will get lost which would cause the sourcemaps to break.

.map((part) => part.split(_shadowDeepSelectors))
.map((deepParts) => {
const [shallowPart, ...otherParts] = deepParts;
const applyScope = (shallowPart: string) => {
Expand Down Expand Up @@ -727,10 +727,10 @@ export class ShadowCss {
let scopedP = p.trim();

if (!scopedP) {
return '';
return p;
}

if (p.indexOf(_polyfillHostNoCombinator) > -1) {
if (p.includes(_polyfillHostNoCombinator)) {
scopedP = this._applySimpleSelectorScope(p, scopeSelector, hostSelector);
} else {
// remove :host since it should be unnecessary
Expand Down Expand Up @@ -765,13 +765,18 @@ export class ShadowCss {
// - `tag:host` -> `tag[h]` (this is to avoid breaking legacy apps, should not match anything)
// - `tag :host` -> `tag [h]` (`tag` is not scoped because it's considered part of a
// `:host-context(tag)`)
const hasHost = selector.indexOf(_polyfillHostNoCombinator) > -1;
const hasHost = selector.includes(_polyfillHostNoCombinator);
// Only scope parts after the first `-shadowcsshost-no-combinator` when it is present
let shouldScope = !hasHost;

while ((res = sep.exec(selector)) !== null) {
const separator = res[1];
const part = selector.slice(startIndex, res.index).trim();
// Do not trim the selector, as otherwise this will break sourcemaps
// when they are defined on multiple lines
// Example:
// div,
// p { color: red}
const part = selector.slice(startIndex, res.index);

// A space following an escaped hex value and followed by another hex character
// (ie: ".\fc ber" for ".über") is not a separator between 2 selectors
Expand All @@ -781,14 +786,14 @@ export class ShadowCss {
continue;
}

shouldScope = shouldScope || part.indexOf(_polyfillHostNoCombinator) > -1;
shouldScope = shouldScope || part.includes(_polyfillHostNoCombinator);
const scopedPart = shouldScope ? _scopeSelectorPart(part) : part;
scopedSelector += `${scopedPart} ${separator} `;
startIndex = sep.lastIndex;
}

const part = selector.substring(startIndex);
shouldScope = shouldScope || part.indexOf(_polyfillHostNoCombinator) > -1;
shouldScope = shouldScope || part.includes(_polyfillHostNoCombinator);
scopedSelector += shouldScope ? _scopeSelectorPart(part) : part;

// replace the placeholders with their original values
Expand Down
7 changes: 7 additions & 0 deletions packages/compiler/test/shadow_css/shadow_css_spec.ts
View file
Open in desktop
Expand Up @@ -117,6 +117,13 @@ describe('ShadowCss', () => {
expect(css).toEqualCss('div[contenta]::after { content:"{}"}');
});

it('should keep retain multiline selectors', () => {
// This is needed as shifting in line number will cause sourcemaps to break.
const styleStr = '.foo,\n.bar { color: red;}';
const css = shim(styleStr, 'contenta');
expect(css).toEqual('.foo[contenta], \n.bar[contenta] { color: red;}');
});

describe('comments', () => {
// Comments should be kept in the same position as otherwise inline sourcemaps break due to
// shift in lines.
Expand Down