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): do not remove comments in component styles #50346

Closed
wants to merge 1 commit into from
Closed

fix(compiler): do not remove comments in component styles #50346

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
65 changes: 40 additions & 25 deletions packages/compiler/src/shadow_css.ts
View file
Open in desktop
Expand Up @@ -138,12 +138,30 @@ export class ShadowCss {
* The hostSelector is the attribute added to the host itself.
*/
shimCssText(cssText: string, selector: string, hostSelector: string = ''): string {
const commentsWithHash = extractCommentsWithHash(cssText);
cssText = stripComments(cssText);
cssText = this._insertDirectives(cssText);
// **NOTE**: Do not strip comments as this will cause component sourcemaps to break
// due to shift in lines.

// Collect comments and replace them with a placeholder, this is done to avoid complicating
// the rule parsing RegExp and keep it safer.
const comments: string[] = [];
cssText = cssText.replace(_commentRe, (m) => {
if (m.match(_commentWithHashRe)) {
comments.push(m);
} else {
// Replace non hash comments with empty lines.
// This is done so that we do not leak any senstive data in comments.
const newLinesMatches = m.match(_newLinesRe);
comments.push((newLinesMatches?.join('') ?? '') + '\n');
}

return COMMENT_PLACEHOLDER;
});

cssText = this._insertDirectives(cssText);
const scopedCssText = this._scopeCssText(cssText, selector, hostSelector);
return [scopedCssText, ...commentsWithHash].join('\n');
// Add back comments at the original position.
let commentIdx = 0;
return scopedCssText.replace(_commentWithHashPlaceHolderRe, () => comments[commentIdx++]);
}

private _insertDirectives(cssText: string): string {
Expand Down Expand Up @@ -434,7 +452,7 @@ export class ShadowCss {
return cssText.replace(_cssColonHostRe, (_, hostSelectors: string, otherSelectors: string) => {
if (hostSelectors) {
const convertedSelectors: string[] = [];
const hostSelectorArray = hostSelectors.split(',').map(p => p.trim());
const hostSelectorArray = hostSelectors.split(',').map((p) => p.trim());
for (const hostSelector of hostSelectorArray) {
if (!hostSelector) break;
const convertedSelector =
Expand Down Expand Up @@ -464,7 +482,7 @@ export class ShadowCss {
* .foo<scopeName> .bar { ... }
*/
private _convertColonHostContext(cssText: string): string {
return cssText.replace(_cssColonHostContextReGlobal, selectorText => {
return cssText.replace(_cssColonHostContextReGlobal, (selectorText) => {
// 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 All @@ -478,12 +496,12 @@ export class ShadowCss {
// Execute `_cssColonHostContextRe` over and over until we have extracted all the
// `:host-context` selectors from this selector.
let match: RegExpExecArray|null;
while (match = _cssColonHostContextRe.exec(selectorText)) {
while ((match = _cssColonHostContextRe.exec(selectorText))) {
// `match` = [':host-context(<selectors>)<rest>', <selectors>, <rest>]

// The `<selectors>` could actually be a comma separated list: `:host-context(.one, .two)`.
const newContextSelectors =
(match[1] ?? '').trim().split(',').map(m => m.trim()).filter(m => m !== '');
(match[1] ?? '').trim().split(',').map((m) => m.trim()).filter((m) => m !== '');

// We must duplicate the current selector group for each of these new selectors.
// For example if the current groups are:
Expand All @@ -507,8 +525,7 @@ export class ShadowCss {
repeatGroups(contextSelectorGroups, newContextSelectors.length);
for (let i = 0; i < newContextSelectors.length; i++) {
for (let j = 0; j < contextSelectorGroupsLength; j++) {
contextSelectorGroups[j + (i * contextSelectorGroupsLength)].push(
newContextSelectors[i]);
contextSelectorGroups[j + i * contextSelectorGroupsLength].push(newContextSelectors[i]);
}
}

Expand All @@ -520,7 +537,7 @@ export class ShadowCss {
// 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))
.join(', ');
});
}
Expand Down Expand Up @@ -574,7 +591,7 @@ export class ShadowCss {
* ```
*/
private _stripScopingSelectors(cssText: string): string {
return processRules(cssText, rule => {
return processRules(cssText, (rule) => {
const selector = rule.selector.replace(_shadowDeepSelectors, ' ')
.replace(_polyfillHostNoCombinatorRe, ' ');
return new CssRule(selector, rule.content);
Expand All @@ -583,7 +600,7 @@ export class ShadowCss {

private _scopeSelector(selector: string, scopeSelector: string, hostSelector: string): string {
return selector.split(',')
.map(part => part.trim().split(_shadowDeepSelectors))
.map((part) => part.trim().split(_shadowDeepSelectors))
.map((deepParts) => {
const [shallowPart, ...otherParts] = deepParts;
const applyScope = (shallowPart: string) => {
Expand Down Expand Up @@ -802,22 +819,18 @@ const _polyfillHostRe = /-shadowcsshost/gim;
const _colonHostRe = /:host/gim;
const _colonHostContextRe = /:host-context/gim;

const _newLinesRe = /\r?\n/g;
const _commentRe = /\/\*[\s\S]*?\*\//g;
const _commentWithHashRe = /\/\*\s*#\s*source(Mapping)?URL=/g;
const COMMENT_PLACEHOLDER = '%COMMENT%';
const _commentWithHashPlaceHolderRe = new RegExp(COMMENT_PLACEHOLDER, 'g');

const _placeholderRe = /__ph-(\d+)__/g;

function stripComments(input: string): string {
return input.replace(_commentRe, '');
}

const _commentWithHashRe = /\/\*\s*#\s*source(Mapping)?URL=[\s\S]+?\*\//g;

function extractCommentsWithHash(input: string): string[] {
return input.match(_commentWithHashRe) || [];
}

const BLOCK_PLACEHOLDER = '%BLOCK%';
const _ruleRe = /(\s*)([^;\{\}]+?)(\s*)((?:{%BLOCK%}?\s*;?)|(?:\s*;))/g;
const _ruleRe = new RegExp(
`(\\s*(?:${COMMENT_PLACEHOLDER}\\s*)*)([^;\\{\\}]+?)(\\s*)((?:{%BLOCK%}?\\s*;?)|(?:\\s*;))`,
'g');
const CONTENT_PAIRS = new Map([['{', '}']]);

const COMMA_IN_PLACEHOLDER = '%COMMA_IN_PLACEHOLDER%';
Expand Down Expand Up @@ -865,6 +878,7 @@ function escapeBlocks(
let blockStartIndex = -1;
let openChar: string|undefined;
let closeChar: string|undefined;

for (let i = 0; i < input.length; i++) {
const char = input[i];
if (char === '\\') {
Expand All @@ -888,12 +902,14 @@ function escapeBlocks(
resultParts.push(input.substring(nonBlockStartIndex, blockStartIndex));
}
}

if (blockStartIndex !== -1) {
escapedBlocks.push(input.substring(blockStartIndex));
resultParts.push(placeholder);
} else {
resultParts.push(input.substring(nonBlockStartIndex));
}

return new StringWithEscapedBlocks(resultParts.join(''), escapedBlocks);
}

Expand Down Expand Up @@ -1025,7 +1041,6 @@ function unescapeQuotes(str: string, isQuoted: boolean): string {
*
* And so on...
*
* @param hostMarker the string that selects the host element.
* @param contextSelectors an array of context selectors that will be combined.
* @param otherSelectors the rest of the selectors that are not context selectors.
*/
Expand Down
53 changes: 29 additions & 24 deletions packages/compiler/test/shadow_css/shadow_css_spec.ts
View file
Open in desktop
Expand Up @@ -96,30 +96,6 @@ describe('ShadowCss', () => {
expect(css).toEqualCss('div[contenta] {height:calc(100% - 55px);}');
});

it('should strip comments', () => {
expect(shim('/* x */b {c}', 'contenta')).toEqualCss('b[contenta] {c}');
});

it('should ignore special characters in comments', () => {
expect(shim('/* {;, */b {c}', 'contenta')).toEqualCss('b[contenta] {c}');
});

it('should support multiline comments', () => {
expect(shim('/* \n */b {c}', 'contenta')).toEqualCss('b[contenta] {c}');
});

it('should keep sourceMappingURL comments', () => {
expect(shim('b {c}/*# sourceMappingURL=data:x */', 'contenta'))
.toEqualCss('b[contenta] {c} /*# sourceMappingURL=data:x */');
expect(shim('b {c}/* #sourceMappingURL=data:x */', 'contenta'))
.toEqualCss('b[contenta] {c} /* #sourceMappingURL=data:x */');
});

it('should keep sourceURL comments', () => {
expect(shim('/*# sourceMappingURL=data:x */b {c}/*# sourceURL=xxx */', 'contenta'))
.toEqualCss('b[contenta] {c} /*# sourceMappingURL=data:x */ /*# sourceURL=xxx */');
});

it('should shim rules with quoted content', () => {
const styleStr = 'div {background-image: url("a.jpg"); color: red;}';
const css = shim(styleStr, 'contenta');
Expand All @@ -137,4 +113,33 @@ describe('ShadowCss', () => {
const css = shim(styleStr, 'contenta');
expect(css).toEqualCss('div[contenta]::after { content:"{}"}');
});

describe('comments', () => {
// Comments should be kept in the same position as otherwise inline sourcemaps break due to
// shift in lines.
it('should replace multiline comments with newline', () => {
expect(shim('/* b {c} */ b {c}', 'contenta')).toBe('\n b[contenta] {c}');
});

it('should replace multiline comments with newline in the original position', () => {
expect(shim('/* b {c}\n */ b {c}', 'contenta')).toBe('\n\n b[contenta] {c}');
});

it('should replace comments with newline in the original position', () => {
expect(shim('/* b {c} */ b {c} /* a {c} */ a {c}', 'contenta'))
.toBe('\n b[contenta] {c} \n a[contenta] {c}');
});

it('should keep sourceMappingURL comments', () => {
expect(shim('b {c} /*# sourceMappingURL=data:x */', 'contenta'))
.toBe('b[contenta] {c} /*# sourceMappingURL=data:x */');
expect(shim('b {c}/* #sourceMappingURL=data:x */', 'contenta'))
.toBe('b[contenta] {c}/* #sourceMappingURL=data:x */');
});

it('should handle adjacent comments', () => {
expect(shim('/* comment 1 */ /* comment 2 */ b {c}', 'contenta'))
.toBe('\n \n b[contenta] {c}');
});
});
});