fix(compiler): fix CSS animation rule scope (#56800)

It is valid CSS to list keyframe names in an animation declaration only
separating the names with a comma and no whitespace. This is typical of
production builds. Updated a couple of regexes and added a couple of
tests to account for this scenario.

Fixes #53038

PR Close #56800
This commit is contained in:
Daniel Puckowski 2024-07-01 19:22:42 -04:00 committed by Pawel Kozlowski
parent 02f5e73a8d
commit aef166f763
2 changed files with 71 additions and 4 deletions

View file

@ -334,8 +334,8 @@ export class ShadowCss {
* animation declaration (with possibly multiple animation definitions)
*
* The regular expression can be divided in three parts
* - (^|\s+)
* simply captures how many (if any) leading whitespaces are present
* - (^|\s+|,)
* captures how many (if any) leading whitespaces are present or a comma
* - (?:(?:(['"])((?:\\\\|\\\2|(?!\2).)+)\2)|(-?[A-Za-z][\w\-]*))
* captures two different possible keyframes, ones which are quoted or ones which are valid css
* idents (custom properties excluded)
@ -344,7 +344,7 @@ export class ShadowCss {
* semicolon or the end of the string
*/
private _animationDeclarationKeyframesRe =
/(^|\s+)(?:(?:(['"])((?:\\\\|\\\2|(?!\2).)+)\2)|(-?[A-Za-z][\w\-]*))(?=[,\s]|$)/g;
/(^|\s+|,)(?:(?:(['"])((?:\\\\|\\\2|(?!\2).)+)\2)|(-?[A-Za-z][\w\-]*))(?=[,\s]|$)/g;
/**
* Scope an animation rule so that the keyframes mentioned in such rule
@ -364,7 +364,7 @@ export class ShadowCss {
unscopedKeyframesSet: ReadonlySet<string>,
): CssRule {
let content = rule.content.replace(
/((?:^|\s+|;)(?:-webkit-)?animation(?:\s*):(?:\s*))([^;]+)/g,
/((?:^|\s+|;)(?:-webkit-)?animation\s*:\s*),*([^;]+)/g,
(_, start, animationDeclarations) =>
start +
animationDeclarations.replace(

View file

@ -122,6 +122,73 @@ describe('ShadowCss, keyframes and animations', () => {
});
});
it('should handle (scope or not) animation definition containing some names which do not have a preceding space', () => {
const COMPONENT_VARIABLE = '%COMP%';
const HOST_ATTR = `_nghost-${COMPONENT_VARIABLE}`;
const CONTENT_ATTR = `_ngcontent-${COMPONENT_VARIABLE}`;
const css = `.test {
animation:my-anim 1s,my-anim2 2s, my-anim3 3s,my-anim4 4s;
}
@keyframes my-anim {
0% {color: red}
100% {color: blue}
}
@keyframes my-anim2 {
0% {font-size: 1em}
100% {font-size: 1.2em}
}
`;
const result = shim(css, CONTENT_ATTR, HOST_ATTR);
const animationLineMatch = result.match(/animation:[^;]+;/);
let animationLine = '';
if (animationLineMatch) {
animationLine = animationLineMatch[0];
}
['my-anim', 'my-anim2'].forEach((scoped) =>
expect(animationLine).toContain(`_ngcontent-%COMP%_${scoped}`),
);
['my-anim3', 'my-anim4'].forEach((nonScoped) => {
expect(animationLine).toContain(nonScoped);
expect(animationLine).not.toContain(`_ngcontent-%COMP%_${nonScoped}`);
});
});
it('should handle (scope or not) animation definitions preceded by an erroneous comma', () => {
const COMPONENT_VARIABLE = '%COMP%';
const HOST_ATTR = `_nghost-${COMPONENT_VARIABLE}`;
const CONTENT_ATTR = `_ngcontent-${COMPONENT_VARIABLE}`;
const css = `.test {
animation:, my-anim 1s,my-anim2 2s, my-anim3 3s,my-anim4 4s;
}
@keyframes my-anim {
0% {color: red}
100% {color: blue}
}
@keyframes my-anim2 {
0% {font-size: 1em}
100% {font-size: 1.2em}
}
`;
const result = shim(css, CONTENT_ATTR, HOST_ATTR);
const animationLineMatch = result.match(/animation:[^;]+;/);
let animationLine = '';
if (animationLineMatch) {
animationLine = animationLineMatch[0];
}
expect(result).not.toContain('animation:,');
['my-anim', 'my-anim2'].forEach((scoped) =>
expect(animationLine).toContain(`_ngcontent-%COMP%_${scoped}`),
);
['my-anim3', 'my-anim4'].forEach((nonScoped) => {
expect(animationLine).toContain(nonScoped);
expect(animationLine).not.toContain(`_ngcontent-%COMP%_${nonScoped}`);
});
});
it('should handle (scope or not) multiple animation definitions in a single declaration', () => {
const css = `
div {