Skip to content

Commit 4716c3b

Browse files
clydinalxhub
authored andcommittedAug 26, 2024·
perf(compiler-cli): reduce duplicate component style resolution (#57502)
Previously, the component handler was processing and resolving stylesheets referenced via `styleUrl`/`styleUrls` multiple times when generating the compiler metadata for components. The style resource information collection for such styles has been further consolidated to avoid repeat resource loader resolve calls which potentially could be expensive. Further optimization is possible for the inline style case. However, inline styles here only require AST traversal and no potentially expensive external resolve calls. PR Close #57502
1 parent 34ce46f commit 4716c3b

File tree

2 files changed

+23
-55
lines changed

2 files changed

+23
-55
lines changed
 

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

+13-7
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ import {
160160
import {
161161
_extractTemplateStyleUrls,
162162
extractComponentStyleUrls,
163-
extractStyleResources,
163+
extractInlineStyleResources,
164164
extractTemplate,
165165
makeResourceNotFoundError,
166166
ParsedTemplateWithSource,
@@ -681,7 +681,7 @@ export class ComponentDecoratorHandler
681681
// component.
682682
let styles: string[] = [];
683683

684-
const styleResources = extractStyleResources(this.resourceLoader, component, containingFile);
684+
const styleResources = extractInlineStyleResources(component);
685685
const styleUrls: StyleUrlMeta[] = [
686686
...extractComponentStyleUrls(this.evaluator, component),
687687
..._extractTemplateStyleUrls(template),
@@ -690,6 +690,16 @@ export class ComponentDecoratorHandler
690690
for (const styleUrl of styleUrls) {
691691
try {
692692
const resourceUrl = this.resourceLoader.resolve(styleUrl.url, containingFile);
693+
if (
694+
styleUrl.source === ResourceTypeForDiagnostics.StylesheetFromDecorator &&
695+
ts.isStringLiteralLike(styleUrl.expression)
696+
) {
697+
// Only string literal values from the decorator are considered style resources
698+
styleResources.add({
699+
path: absoluteFrom(resourceUrl),
700+
expression: styleUrl.expression,
701+
});
702+
}
693703
const resourceStr = this.resourceLoader.load(resourceUrl);
694704
styles.push(resourceStr);
695705
if (this.depTracker !== null) {
@@ -711,11 +721,7 @@ export class ComponentDecoratorHandler
711721
? ResourceTypeForDiagnostics.StylesheetFromDecorator
712722
: ResourceTypeForDiagnostics.StylesheetFromTemplate;
713723
diagnostics.push(
714-
makeResourceNotFoundError(
715-
styleUrl.url,
716-
styleUrl.nodeForError,
717-
resourceType,
718-
).toDiagnostic(),
724+
makeResourceNotFoundError(styleUrl.url, styleUrl.expression, resourceType).toDiagnostic(),
719725
);
720726
}
721727
}

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

+10-48
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ import {
3838
*/
3939
export interface StyleUrlMeta {
4040
url: string;
41-
nodeForError: ts.Node;
41+
expression: ts.Expression;
4242
source:
4343
| ResourceTypeForDiagnostics.StylesheetFromTemplate
4444
| ResourceTypeForDiagnostics.StylesheetFromDecorator;
@@ -123,7 +123,9 @@ export interface ExternalTemplateDeclaration extends CommonTemplateDeclaration {
123123
export type TemplateDeclaration = InlineTemplateDeclaration | ExternalTemplateDeclaration;
124124

125125
/** Determines the node to use for debugging purposes for the given TemplateDeclaration. */
126-
export function getTemplateDeclarationNodeForError(declaration: TemplateDeclaration): ts.Node {
126+
export function getTemplateDeclarationNodeForError(
127+
declaration: TemplateDeclaration,
128+
): ts.Expression {
127129
return declaration.isInline ? declaration.expression : declaration.templateUrlExpression;
128130
}
129131

@@ -629,7 +631,7 @@ export function extractComponentStyleUrls(
629631
{
630632
url: styleUrl,
631633
source: ResourceTypeForDiagnostics.StylesheetFromDecorator,
632-
nodeForError: styleUrlExpr,
634+
expression: styleUrlExpr,
633635
},
634636
];
635637
}
@@ -657,7 +659,7 @@ function extractStyleUrlsFromExpression(
657659
styleUrls.push({
658660
url: styleUrl,
659661
source: ResourceTypeForDiagnostics.StylesheetFromDecorator,
660-
nodeForError: styleUrlExpr,
662+
expression: styleUrlExpr,
661663
});
662664
}
663665
}
@@ -675,44 +677,20 @@ function extractStyleUrlsFromExpression(
675677
styleUrls.push({
676678
url: styleUrl,
677679
source: ResourceTypeForDiagnostics.StylesheetFromDecorator,
678-
nodeForError: styleUrlsExpr,
680+
expression: styleUrlsExpr,
679681
});
680682
}
681683
}
682684

683685
return styleUrls;
684686
}
685687

686-
export function extractStyleResources(
687-
resourceLoader: ResourceLoader,
688-
component: Map<string, ts.Expression>,
689-
containingFile: string,
690-
): ReadonlySet<Resource> {
688+
export function extractInlineStyleResources(component: Map<string, ts.Expression>): Set<Resource> {
691689
const styles = new Set<Resource>();
692690
function stringLiteralElements(array: ts.ArrayLiteralExpression): ts.StringLiteralLike[] {
693691
return array.elements.filter((e): e is ts.StringLiteralLike => ts.isStringLiteralLike(e));
694692
}
695693

696-
// If styleUrls is a literal array, process each resource url individually and register ones that
697-
// are string literals. If `styleUrl` is specified, register a single stylesheet. Note that
698-
// `styleUrl` and `styleUrls` are mutually-exclusive. This is validated in
699-
// `extractComponentStyleUrls`.
700-
const styleUrlExpr = component.get('styleUrl');
701-
const styleUrlsExpr = component.get('styleUrls');
702-
if (styleUrlsExpr !== undefined && ts.isArrayLiteralExpression(styleUrlsExpr)) {
703-
for (const expression of stringLiteralElements(styleUrlsExpr)) {
704-
const resource = stringLiteralUrlToResource(resourceLoader, expression, containingFile);
705-
if (resource !== null) {
706-
styles.add(resource);
707-
}
708-
}
709-
} else if (styleUrlExpr !== undefined && ts.isStringLiteralLike(styleUrlExpr)) {
710-
const resource = stringLiteralUrlToResource(resourceLoader, styleUrlExpr, containingFile);
711-
if (resource !== null) {
712-
styles.add(resource);
713-
}
714-
}
715-
716694
const stylesExpr = component.get('styles');
717695
if (stylesExpr !== undefined) {
718696
if (ts.isArrayLiteralExpression(stylesExpr)) {
@@ -727,31 +705,15 @@ export function extractStyleResources(
727705
return styles;
728706
}
729707

730-
function stringLiteralUrlToResource(
731-
resourceLoader: ResourceLoader,
732-
expression: ts.StringLiteralLike,
733-
containingFile: string,
734-
): Resource | null {
735-
try {
736-
const resourceUrl = resourceLoader.resolve(expression.text, containingFile);
737-
return {path: absoluteFrom(resourceUrl), expression};
738-
} catch {
739-
// Errors in style resource extraction do not need to be handled here. We will produce
740-
// diagnostics for each one that fails in the analysis, after we evaluate the `styleUrls`
741-
// expression to determine _all_ style resources, not just the string literals.
742-
return null;
743-
}
744-
}
745-
746708
export function _extractTemplateStyleUrls(template: ParsedTemplateWithSource): StyleUrlMeta[] {
747709
if (template.styleUrls === null) {
748710
return [];
749711
}
750712

751-
const nodeForError = getTemplateDeclarationNodeForError(template.declaration);
713+
const expression = getTemplateDeclarationNodeForError(template.declaration);
752714
return template.styleUrls.map((url) => ({
753715
url,
754716
source: ResourceTypeForDiagnostics.StylesheetFromTemplate,
755-
nodeForError,
717+
expression,
756718
}));
757719
}

0 commit comments

Comments
 (0)
Please sign in to comment.