Skip to content

fix(@ngtools/webpack): emit diagnostic for unsupported conditional templateUrl expressions #32888

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

Closed
maruthang wants to merge 3 commits into from
+155 −1
Closed

fix(@ngtools/webpack): emit diagnostic for unsupported conditional templateUrl expressions #32888

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0abae8b
fix(@ngtools/webpack): emit diagnostic for unsupported conditional te…
maruthang Mar 27, 2026
3aa479a
fix(@ngtools/webpack): extend non-string literal validation to styleU…
maruthang Apr 2, 2026
285f1b8
refactor(@ngtools/webpack): use template literals for error messages
maruthang Apr 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 33 additions & 1 deletion packages/ngtools/webpack/src/transformers/replace_resources.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ export function replaceResources(
resourceImportDeclarations,
moduleKind,
inlineStyleFileExtension,
node,
),
),
...(ts.getModifiers(node) ?? []),
Expand Down Expand Up @@ -87,6 +88,7 @@ function visitDecorator(
resourceImportDeclarations: ts.ImportDeclaration[],
moduleKind?: ts.ModuleKind,
inlineStyleFileExtension?: string,
classDeclaration?: ts.ClassDeclaration,
): ts.Decorator {
if (!isComponentDecorator(node, typeChecker)) {
return node;
Expand All @@ -106,6 +108,8 @@ function visitDecorator(
const objectExpression = args[0];
const styleReplacements: ts.Expression[] = [];

const className = classDeclaration?.name?.text ?? 'Unknown';

// visit all properties
let properties = ts.visitNodes(objectExpression.properties, (node) =>
ts.isObjectLiteralElementLike(node)
Expand All @@ -116,6 +120,7 @@ function visitDecorator(
resourceImportDeclarations,
moduleKind,
inlineStyleFileExtension,
className,
)
: node,
) as ts.NodeArray<ts.ObjectLiteralElementLike>;
Expand Down Expand Up @@ -148,6 +153,7 @@ function visitComponentMetadata(
resourceImportDeclarations: ts.ImportDeclaration[],
moduleKind: ts.ModuleKind = ts.ModuleKind.ES2015,
inlineStyleFileExtension?: string,
className?: string,
): ts.ObjectLiteralElementLike | undefined {
if (!ts.isPropertyAssignment(node) || ts.isComputedPropertyName(node.name)) {
return node;
Expand All @@ -161,7 +167,12 @@ function visitComponentMetadata(
case 'templateUrl': {
const url = getResourceUrl(node.initializer);
if (!url) {
return node;
const sourceFile = node.getSourceFile();
const { line } = sourceFile.getLineAndCharacterOfPosition(node.initializer.getStart());

throw new Error(
`Component '${className}' in '${sourceFile.fileName}' contains a non-string literal 'templateUrl' value at line ${line + 1}. The 'templateUrl' property must be a string literal. Expressions, variables, or other dynamic values are not supported.`,
);
Comment on lines +173 to +175
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot Mar 27, 2026

Choose a reason for hiding this comment

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

medium

For improved readability and maintainability, consider using a template literal for this error message instead of string concatenation.

throw new Error(`Component '${className}' in '${sourceFile.fileName}' contains a non-string literal 'templateUrl' value at line ${line + 1}. The 'templateUrl' property must be a string literal. Expressions, variables, or other dynamic values are not supported.`);

}
Comment on lines 169 to 176
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot Mar 27, 2026

Choose a reason for hiding this comment

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

high

This change correctly adds a build-time error for dynamic templateUrl values. However, the PR description states that this should also apply to styleUrls, but that case is not handled. Non-string literal values in styleUrls are still silently ignored, which can lead to confusing runtime issues.

To ensure consistency and prevent these issues, a similar error should be thrown for non-string literal expressions in styleUrl and styleUrls.

This would likely involve:

  • In visitComponentMetadata, for the styleUrl/styleUrls case, check if node.initializer is not a string or array literal and throw an error if so.
  • Passing the className to transformInlineStyleLiteral.
  • In transformInlineStyleLiteral, when !ts.isStringLiteralLike(node) and !isInlineStyle, throw an error similar to the one for templateUrl.

Additionally, please add tests for these styleUrls cases to replace_resources_spec.ts to cover these scenarios.


const importName = createResourceImport(
Expand Down Expand Up @@ -198,6 +209,19 @@ function visitComponentMetadata(
) as ts.StringLiteralLike,
];
} else if (ts.isArrayLiteralExpression(node.initializer)) {
if (!isInlineStyle) {
// Validate each element is a string literal for styleUrls
for (const element of node.initializer.elements) {
if (!ts.isStringLiteralLike(element)) {
const sourceFile = node.getSourceFile();
const { line } = sourceFile.getLineAndCharacterOfPosition(element.getStart());

throw new Error(
`Component '${className}' in '${sourceFile.fileName}' contains a non-string literal '${name}' value at line ${line + 1}. The '${name}' property must contain string literals. Expressions, variables, or other dynamic values are not supported.`,
);
}
}
}
styles = ts.visitNodes(node.initializer.elements, (node) =>
transformInlineStyleLiteral(
node,
Expand All @@ -208,6 +232,14 @@ function visitComponentMetadata(
moduleKind,
),
) as ts.NodeArray<ts.Expression>;
} else if (!isInlineStyle) {
// styleUrl or styleUrls with a non-string, non-array initializer
const sourceFile = node.getSourceFile();
const { line } = sourceFile.getLineAndCharacterOfPosition(node.initializer.getStart());

throw new Error(
`Component '${className}' in '${sourceFile.fileName}' contains a non-string literal '${name}' value at line ${line + 1}. The '${name}' property must be a string literal. Expressions, variables, or other dynamic values are not supported.`,
);
} else {
return node;
}
Expand Down
122 changes: 122 additions & 0 deletions packages/ngtools/webpack/src/transformers/replace_resources_spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -469,4 +469,126 @@ describe('find_resources', () => {
const result = transform(input, false);
expect(tags.oneLine`${result}`).toEqual(tags.oneLine`${output}`);
});

it('should throw an error when templateUrl is a conditional expression', () => {
const input = tags.stripIndent`
import { Component } from '@angular/core';

@Component({
selector: 'app-root',
templateUrl: true === true
? './app.component.html'
: './app.component.copy.html',
styleUrls: ['./app.component.css', './app.component.2.css']
})
export class AppComponent {
title = 'app';
}
`;

expect(() => transform(input)).toThrowError(
/Component 'AppComponent'.*contains a non-string literal 'templateUrl' value/,
);
});

it('should throw an error when templateUrl is a variable reference', () => {
const input = tags.stripIndent`
import { Component } from '@angular/core';

const myTemplate = './app.component.html';

@Component({
selector: 'app-root',
templateUrl: myTemplate,
styleUrls: ['./app.component.css']
})
export class AppComponent {
title = 'app';
}
`;

expect(() => transform(input)).toThrowError(
/Component 'AppComponent'.*contains a non-string literal 'templateUrl' value/,
);
});

it('should throw an error when styleUrls contains a conditional expression', () => {
const input = tags.stripIndent`
import { Component } from '@angular/core';

@Component({
selector: 'app-root',
templateUrl: './app.component.html',
styleUrls: [true === true ? './app.component.css' : './app.component.dark.css']
})
export class AppComponent {
title = 'app';
}
`;

expect(() => transform(input)).toThrowError(
/Component 'AppComponent'.*contains a non-string literal 'styleUrls' value/,
);
});

it('should throw an error when styleUrls contains a variable reference', () => {
const input = tags.stripIndent`
import { Component } from '@angular/core';

const myStyle = './app.component.css';

@Component({
selector: 'app-root',
templateUrl: './app.component.html',
styleUrls: [myStyle]
})
export class AppComponent {
title = 'app';
}
`;

expect(() => transform(input)).toThrowError(
/Component 'AppComponent'.*contains a non-string literal 'styleUrls' value/,
);
});

it('should throw an error when styleUrl is a conditional expression', () => {
const input = tags.stripIndent`
import { Component } from '@angular/core';

@Component({
selector: 'app-root',
templateUrl: './app.component.html',
styleUrl: true === true ? './app.component.css' : './app.component.dark.css'
})
export class AppComponent {
title = 'app';
}
`;

expect(() => transform(input)).toThrowError(
/Component 'AppComponent'.*contains a non-string literal 'styleUrl' value/,
);
});

it('should throw an error when styleUrl is a variable reference', () => {
const input = tags.stripIndent`
import { Component } from '@angular/core';

const myStyle = './app.component.css';

@Component({
selector: 'app-root',
templateUrl: './app.component.html',
styleUrl: myStyle
})
export class AppComponent {
title = 'app';
}
`;

expect(() => transform(input)).toThrowError(
/Component 'AppComponent'.*contains a non-string literal 'styleUrl' value/,
);
});
});
Loading