Skip to content

Commit 9e87593

Browse files
clydindylhunn
clydin
authored and
dylhunn
committed
feat(compiler-cli): ensure template style elements are preprocessed as inline styles (#57429)
Previously, style elements within a template were used directly and not provided to the optional transformation step that may be present on the resource host interface. This causes such styles to not be processed by the Angular CLI's stylesheet pipeline and could cause the styles to not work properly on all browsers. The style elements are now processed in the same manner as inline styles within a component's metadata. Link elements within a stylesheet were already being processed as `styleUrls` equivalent and there is no behavior change in that regard. PR Close #57429
1 parent 270fb83 commit 9e87593

File tree

2 files changed

+141
-32
lines changed

2 files changed

+141
-32
lines changed

‎packages/compiler-cli/src/ngtsc/annotations/component/src/handler.ts‎

Lines changed: 50 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -347,44 +347,61 @@ export class ComponentDecoratorHandler
347347
this.defaultPreserveWhitespaces,
348348
this.extractTemplateOptions,
349349
this.compilationMode,
350-
).then((template: ParsedTemplateWithSource | null): Promise<void> | undefined => {
351-
if (template === null) {
352-
return undefined;
353-
}
350+
).then(
351+
(template): {templateUrl?: string; templateStyles: string[]; templateStyleUrls: string[]} => {
352+
if (template === null) {
353+
return {templateStyles: [], templateStyleUrls: []};
354+
}
354355

355-
return Promise.all(template.styleUrls.map((styleUrl) => resolveStyleUrl(styleUrl))).then(
356-
() => undefined,
357-
);
358-
});
356+
let templateUrl;
357+
if (template.sourceMapping.type === 'external') {
358+
templateUrl = template.sourceMapping.templateUrl;
359+
}
360+
361+
return {
362+
templateUrl,
363+
templateStyles: template.styles,
364+
templateStyleUrls: template.styleUrls,
365+
};
366+
},
367+
);
359368

360369
// Extract all the styleUrls in the decorator.
361370
const componentStyleUrls = extractComponentStyleUrls(this.evaluator, component);
362371

363-
// Extract inline styles, process, and cache for use in synchronous analyze phase
364-
let inlineStyles;
365-
if (component.has('styles')) {
366-
const litStyles = parseDirectiveStyles(component, this.evaluator, this.compilationMode);
367-
if (litStyles === null) {
368-
this.preanalyzeStylesCache.set(node, null);
369-
} else {
370-
inlineStyles = Promise.all(
371-
litStyles.map((style) =>
372+
return templateAndTemplateStyleResources.then(async (templateInfo) => {
373+
// Extract inline styles, process, and cache for use in synchronous analyze phase
374+
let styles: string[] | null = null;
375+
const rawStyles = parseDirectiveStyles(component, this.evaluator, this.compilationMode);
376+
if (rawStyles?.length) {
377+
styles = await Promise.all(
378+
rawStyles.map((style) =>
372379
this.resourceLoader.preprocessInline(style, {type: 'style', containingFile}),
373380
),
374-
).then((styles) => {
375-
this.preanalyzeStylesCache.set(node, styles);
376-
});
381+
);
382+
}
383+
if (templateInfo.templateStyles) {
384+
styles ??= [];
385+
styles.push(
386+
...(await Promise.all(
387+
templateInfo.templateStyles.map((style) =>
388+
this.resourceLoader.preprocessInline(style, {
389+
type: 'style',
390+
containingFile: templateInfo.templateUrl ?? containingFile,
391+
}),
392+
),
393+
)),
394+
);
377395
}
378-
} else {
379-
this.preanalyzeStylesCache.set(node, null);
380-
}
381396

382-
// Wait for both the template and all styleUrl resources to resolve.
383-
return Promise.all([
384-
templateAndTemplateStyleResources,
385-
inlineStyles,
386-
...componentStyleUrls.map((styleUrl) => resolveStyleUrl(styleUrl.url)),
387-
]).then(() => undefined);
397+
this.preanalyzeStylesCache.set(node, styles);
398+
399+
// Wait for both the template and all styleUrl resources to resolve.
400+
await Promise.all([
401+
...componentStyleUrls.map((styleUrl) => resolveStyleUrl(styleUrl.url)),
402+
...templateInfo.templateStyleUrls.map((url) => resolveStyleUrl(url)),
403+
]);
404+
});
388405
}
389406

390407
analyze(
@@ -743,9 +760,10 @@ export class ComponentDecoratorHandler
743760
styles.push(...litStyles);
744761
}
745762
}
746-
}
747-
if (template.styles.length > 0) {
748-
styles.push(...template.styles);
763+
764+
if (template.styles.length > 0) {
765+
styles.push(...template.styles);
766+
}
749767
}
750768

751769
// Collect all explicitly deferred symbols from the `@Component.deferredImports` field

‎packages/compiler-cli/src/ngtsc/annotations/component/test/component_spec.ts‎

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -398,6 +398,97 @@ runInEachFileSystem(() => {
398398
expect(analysis?.inlineStyles).toEqual(jasmine.arrayWithExactContents(['.xyz {}']));
399399
});
400400

401+
it('should replace template style element content for inline template with transformed content', async () => {
402+
const {program, options, host} = makeProgram([
403+
{
404+
name: _('/node_modules/@angular/core/index.d.ts'),
405+
contents: 'export const Component: any;',
406+
},
407+
{
408+
name: _('/entry.ts'),
409+
contents: `
410+
import {Component} from '@angular/core';
411+
412+
@Component({
413+
template: '<style>.abc {}</style>',
414+
}) class TestCmp {}
415+
`,
416+
},
417+
]);
418+
const {reflectionHost, handler, resourceLoader} = setup(program, options, host);
419+
resourceLoader.canPreload = true;
420+
resourceLoader.canPreprocess = true;
421+
resourceLoader.preprocessInline = async function (data, context) {
422+
expect(data).toBe('.abc {}');
423+
expect(context.containingFile).toBe(_('/entry.ts').toLowerCase());
424+
expect(context.type).toBe('style');
425+
426+
return '.xyz {}';
427+
};
428+
429+
const TestCmp = getDeclaration(program, _('/entry.ts'), 'TestCmp', isNamedClassDeclaration);
430+
const detected = handler.detect(TestCmp, reflectionHost.getDecoratorsOfDeclaration(TestCmp));
431+
if (detected === undefined) {
432+
return fail('Failed to recognize @Component');
433+
}
434+
435+
await handler.preanalyze(TestCmp, detected.metadata);
436+
437+
const {analysis} = handler.analyze(TestCmp, detected.metadata);
438+
expect(analysis?.inlineStyles).toEqual(jasmine.arrayWithExactContents(['.xyz {}']));
439+
});
440+
441+
it('should replace template style element content for external template with transformed content', async () => {
442+
const {program, options, host} = makeProgram([
443+
{
444+
name: _('/node_modules/@angular/core/index.d.ts'),
445+
contents: 'export const Component: any;',
446+
},
447+
{
448+
name: _('/component.ng.html'),
449+
contents: '<style>.abc {}</style>',
450+
},
451+
{
452+
name: _('/entry.ts'),
453+
contents: `
454+
import {Component} from '@angular/core';
455+
456+
@Component({
457+
templateUrl: '/component.ng.html',
458+
}) class TestCmp {}
459+
`,
460+
},
461+
]);
462+
const {reflectionHost, handler, resourceLoader} = setup(program, options, host);
463+
resourceLoader.canPreload = true;
464+
resourceLoader.canPreprocess = true;
465+
resourceLoader.resolve = function (v) {
466+
return _(v).toLowerCase();
467+
};
468+
resourceLoader.load = function (v) {
469+
return host.readFile(v) ?? '';
470+
};
471+
resourceLoader.preload = () => Promise.resolve();
472+
resourceLoader.preprocessInline = async function (data, context) {
473+
expect(data).toBe('.abc {}');
474+
expect(context.containingFile).toBe(_('/component.ng.html').toLowerCase());
475+
expect(context.type).toBe('style');
476+
477+
return '.xyz {}';
478+
};
479+
480+
const TestCmp = getDeclaration(program, _('/entry.ts'), 'TestCmp', isNamedClassDeclaration);
481+
const detected = handler.detect(TestCmp, reflectionHost.getDecoratorsOfDeclaration(TestCmp));
482+
if (detected === undefined) {
483+
return fail('Failed to recognize @Component');
484+
}
485+
486+
await handler.preanalyze(TestCmp, detected.metadata);
487+
488+
const {analysis} = handler.analyze(TestCmp, detected.metadata);
489+
expect(analysis?.inlineStyles).toEqual(jasmine.arrayWithExactContents(['.xyz {}']));
490+
});
491+
401492
it('should error if canPreprocess is true and async analyze is not used', async () => {
402493
const {program, options, host} = makeProgram([
403494
{

0 commit comments

Comments
 (0)