Support untagged template literals in expressions #59230

crisbeto · 2024-12-18T08:33:38Z

Updates the compiler to allow users to use untagged template literals inside of our expression syntax (e.g. hello ${world}) which makes string concatenation a bit simpler and more convenient. Includes the following changes:

refactor(compiler): allow lexer to produce multiple tokens

Reworks the lexer's scanner to produce more than one token at a time. This can be useful for the cases where one token means the end of another one.

Also cleans up the scanner by making all non-essential methods private and using strict equality everywhere.

refactor(compiler): tokenize template literals

Reworks the lexer to produce tokens for template literal expressions.

refactor(compiler): produce AST for template literals

Updates the compiler to parse the template literal tokens into the new TemplateLiteral and TemplateLiteralElement AST nodes.

refactor(compiler): clean up tagged templates in output AST

Makes the following cleanups in the output AST:

The TemplateLiteral and TemplateLiteralElement nodes have been renamed to TemplateLiteralExpr and TemplateLiteralElementExpr respectively for consistency and to avoid overlaps with the expression AST nodes.
The TemplateLiteralExpr and TemplateLiteralElementExpr have been refactored to be Expressions for correctness. This involves updating some existing code.
The TaggedTemplateExpr has been renamed to TaggedTemplateLiteralExpr for consistency.

refactor(compiler-cli): handle template literals in ngtsc

Updates the translators that convert expression ASTs to account for template literals.

feat(compiler): support untagged template literals in expressions

Updates the compiler to support untagged template literals inside of the expression syntax (e.g. hello ${world}).

crisbeto · 2024-12-18T09:04:59Z

packages/compiler-cli/src/ngtsc/translator/src/translator.ts

I didn't bother implementing the downleveling code here, because all the browsers we support also support template literals natively.

Makes sense. There wasn't downleveling here before anyway right? (even with tagged template literals)

I think for the tagged ones we only downleveled "known" ones like $localize and not in a standards-based way.

jelbourn · 2024-12-20T20:38:03Z

As a follow up, we should make sure to update the docs: https://angular.dev/guide/templates/expression-syntax#unsupported-literals

sheikalthaf · 2024-12-26T04:16:42Z

@crisbeto whether it will support the class with template literals?

<div class=`h-12 mt-4 ${cond() ? 'flex flex-col' : 'flex flex-row'} ${cond1() ? 'p-4' : 'p-2'}`></div>

JeanMeche · 2024-12-26T04:26:32Z

@sheikalthaf This PR adds the support for template literals within angular expressions. Your example isn't an angular expression.

sheikalthaf · 2024-12-26T04:29:01Z

@JeanMeche then this will work right?

<div class="{{`h-12 mt-4 ${cond() ? 'flex flex-col' : 'flex flex-row'} ${cond1() ? 'p-4' : 'p-2'}`}}"></div>

I'm thinking of using this pattern alternative to ngClass. What's your thoughts on this?

devversion

LGTM overall. Nice work!

I don't see any reasons why we wouldn't want to support this, as it's aligning us even further with TS/JS.

A few minor nits on the lexer portion, but the rest looks great

packages/compiler/src/expression_parser/lexer.ts

devversion · 2025-01-20T13:13:05Z

packages/compiler/src/expression_parser/lexer.ts

Why is there an expected error?

It's because TS doesn't know that advance mutated this.peek so it narrows the value to chars.$$ and doesn't allow the comparison to chars.$LBRACE.

Might be worth a TODO

devversion · 2025-01-20T13:16:20Z

packages/compiler/src/expression_parser/lexer.ts

Kinda would have wished that we could advance through the template literal within the scanTemplateLiteralPart, but it's probably not possible.

I guess that's a related reason why we also have this.tokens now exposed, and .push-ed here?

Yeah exactly, the scanner isn't really set up to produce more than one token at a time.

Would it be possible/reasonable to keep it one token at a time (to not introduce some extra complexity around following the order etc.) and just basing the scans on state? (like you did with brace depth etc.)

We could do it with a flag, but I feel like that'll be less readable than just producing two tokens at once since we'd basically split up this logic across two places instead of keeping it together. I'm not too concerned about the tokens being exposed, because the lexer isn't exposed anywhere.

I felt like it was a little hard to reason about the order if are returning a token during scan, and suddenly we could push additional tokens in between. But it's not a super big of a deal. I do see your concerns as well.

Will defer to others here.

devversion · 2025-01-20T13:57:45Z

packages/compiler/test/expression_parser/utils/unparser.ts

OOC: Do you know why this exists if we have the serializer already?

Hmm good question. My guess is because we use the unparser in tests and it has a little bit of extra test-specific functionality around recording the spans. We may be able to consolidate them a bit though.

devversion · 2025-01-20T14:04:00Z

packages/compiler-cli/src/ngtsc/translator/src/translator.ts

Makes sense. There wasn't downleveling here before anyway right? (even with tagged template literals)

packages/compiler-cli/src/ngtsc/typecheck/src/expression.ts

Reworks the lexer's scanner to produce more than one token at a time. This can be useful for the cases where one token means the end of another one. Also cleans up the scanner by making all non-essential methods private and using strict equality everywhere.

Reworks the lexer to produce tokens for template literal expressions.

Updates the compiler to parse the template literal tokens into the new `TemplateLiteral` and `TemplateLiteralElement` AST nodes.

Makes the following cleanups in the output AST: * The `TemplateLiteral` and `TemplateLiteralElement` nodes have been renamed to `TemplateLiteralExpr` and `TemplateLiteralElementExpr` respectively for consistency and to avoid overlaps with the expression AST nodes. * The `TemplateLiteralExpr` and `TemplateLiteralElementExpr` have been refactored to be `Expression`s for correctness. This involves updating some existing code. * The `TaggedTemplateExpr` has been renamed to `TaggedTemplateLiteralExpr` for consistency.

Updates the translators that convert expression ASTs to account for template literals.

Updates the compiler to support untagged template literals inside of the expression syntax (e.g. ``hello ${world}``).

crisbeto

Feedback has been addressed.

crisbeto · 2025-01-20T14:13:14Z

packages/compiler-cli/src/ngtsc/translator/src/translator.ts

I think for the tagged ones we only downleveled "known" ones like $localize and not in a standards-based way.

crisbeto · 2025-01-20T14:26:03Z

packages/compiler/src/expression_parser/lexer.ts

Yeah exactly, the scanner isn't really set up to produce more than one token at a time.

crisbeto · 2025-01-20T14:27:19Z

packages/compiler/src/expression_parser/lexer.ts

It's because TS doesn't know that advance mutated this.peek so it narrows the value to chars.$$ and doesn't allow the comparison to chars.$LBRACE.

packages/compiler/src/expression_parser/lexer.ts

crisbeto · 2025-01-20T14:35:04Z

packages/compiler/test/expression_parser/utils/unparser.ts

Hmm good question. My guess is because we use the unparser in tests and it has a little bit of extra test-specific functionality around recording the spans. We may be able to consolidate them a bit though.

AndrewKushnir · 2025-01-21T20:04:56Z

This PR was merged into the repository by commit fe8a683.

The changes were merged into the following branches: main

Reworks the lexer to produce tokens for template literal expressions. PR Close #59230

Updates the compiler to parse the template literal tokens into the new `TemplateLiteral` and `TemplateLiteralElement` AST nodes. PR Close #59230

Makes the following cleanups in the output AST: * The `TemplateLiteral` and `TemplateLiteralElement` nodes have been renamed to `TemplateLiteralExpr` and `TemplateLiteralElementExpr` respectively for consistency and to avoid overlaps with the expression AST nodes. * The `TemplateLiteralExpr` and `TemplateLiteralElementExpr` have been refactored to be `Expression`s for correctness. This involves updating some existing code. * The `TaggedTemplateExpr` has been renamed to `TaggedTemplateLiteralExpr` for consistency. PR Close #59230

Updates the translators that convert expression ASTs to account for template literals. PR Close #59230

…9230) Updates the compiler to support untagged template literals inside of the expression syntax (e.g. ``hello ${world}``). PR Close #59230

JoostK

Paul had asked for me to take a look at this, so leaving a few after-merge comments. The lexer does feel a bit awkward with the implicit multi-token production, but it doesn't hurt readability much so no big deal, I think

JoostK · 2025-01-21T19:55:47Z

packages/compiler/src/expression_parser/ast.ts

+ const expression = i < ast.expressions.length ? ast.expressions[i] : null;
+ if (expression !== null) {
+ this.visit(expression, context);
+ }


nit:

Suggested change

const expression = i < ast.expressions.length ? ast.expressions[i] : null;

if (expression !== null) {

this.visit(expression, context);

}

if (i < ast.expressions.length) {

this.visit(ast.expressions[i], context);

}

JoostK · 2025-01-21T20:01:48Z

packages/compiler/src/expression_parser/parser.ts

+ return this.parseNoInterpolationTemplateLiteral(start);
+ } else if (this.next.isTemplateLiteralPart()) {
+ return this.parseTemplateLiteral();
+ } else if (this.next.isString() && this.next.kind === StringTokenKind.Plain) {


Is the Plain condition necessary here? It seems unexpected to me, seemingly allowing some strings to be reported as error?

It's been some time since I wrote it, but I think the idea was to avoid confusion with the template literals in case we introduce some other string token. AFAIK in practice we can't actually hit that code path without it being StringTokenKind.Plain.

JoostK · 2025-01-21T20:07:22Z

packages/compiler/src/expression_parser/parser.ts

+ this.advance();
+ const expression = this.parsePipe();
+ if (expression instanceof EmptyExpr) {
+ this.error('Template literal interpolation cannot be empty');


This error path doesn't collect the empty expression into expressions, yet a TemplateLiteral is being returned. I suspect this may cause the invariant elements.length === expressions.length + 1 to be violated.

Hmm potentially, but I think that whatever is consuming the result will report the error.

JoostK · 2025-01-21T20:08:22Z

packages/compiler/src/expression_parser/serializer.ts

+ const expression = i < ast.expressions.length ? ast.expressions[i] : null;
+ if (expression !== null) {


nit: same here for inlining the condition into the if

JoostK · 2025-01-21T20:10:57Z

packages/compiler/test/expression_parser/utils/unparser.ts

+ const expression = i < ast.expressions.length ? ast.expressions[i] : null;
+ if (expression !== null) {


The nit here too

…9230) Reworks the lexer's scanner to produce more than one token at a time. This can be useful for the cases where one token means the end of another one. Also cleans up the scanner by making all non-essential methods private and using strict equality everywhere. PR Close angular#59230

Reworks the lexer to produce tokens for template literal expressions. PR Close angular#59230

Updates the compiler to parse the template literal tokens into the new `TemplateLiteral` and `TemplateLiteralElement` AST nodes. PR Close angular#59230

…59230) Makes the following cleanups in the output AST: * The `TemplateLiteral` and `TemplateLiteralElement` nodes have been renamed to `TemplateLiteralExpr` and `TemplateLiteralElementExpr` respectively for consistency and to avoid overlaps with the expression AST nodes. * The `TemplateLiteralExpr` and `TemplateLiteralElementExpr` have been refactored to be `Expression`s for correctness. This involves updating some existing code. * The `TaggedTemplateExpr` has been renamed to `TaggedTemplateLiteralExpr` for consistency. PR Close angular#59230

) Updates the translators that convert expression ASTs to account for template literals. PR Close angular#59230

…gular#59230) Updates the compiler to support untagged template literals inside of the expression syntax (e.g. ``hello ${world}``). PR Close angular#59230

angular-automatic-lock-bot · 2025-02-22T00:10:59Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

crisbeto added action: review The PR is still awaiting reviews from at least one requested reviewer target: minor This PR is targeted for the next minor release labels Dec 18, 2024

angular-robot bot added detected: feature PR contains a feature commit area: compiler Issues related to `ngc`, Angular's template compiler labels Dec 18, 2024

ngbot bot added this to the Backlog milestone Dec 18, 2024

crisbeto force-pushed the template-literals branch from 42404fc to 44704f0 Compare December 18, 2024 09:02

angular-robot bot added area: compiler Issues related to `ngc`, Angular's template compiler and removed area: compiler Issues related to `ngc`, Angular's template compiler labels Dec 18, 2024

ngbot bot modified the milestone: Backlog Dec 18, 2024

crisbeto marked this pull request as ready for review December 18, 2024 09:03

pullapprove bot requested a review from clydin December 18, 2024 09:03

crisbeto requested review from JoostK and alxhub December 18, 2024 09:03

crisbeto commented Dec 18, 2024

View reviewed changes

JeanMeche mentioned this pull request Dec 18, 2024

Using a backtick in a template expression should give a better error message #59160

Closed

crisbeto force-pushed the template-literals branch from 44704f0 to e7fbd77 Compare January 15, 2025 11:19

devversion self-requested a review January 20, 2025 12:12

devversion reviewed Jan 20, 2025

View reviewed changes

crisbeto added 6 commits January 20, 2025 15:14

refactor(compiler): tokenize template literals

bfc3ec8

Reworks the lexer to produce tokens for template literal expressions.

refactor(compiler): produce AST for template literals

3c60ea5

Updates the compiler to parse the template literal tokens into the new `TemplateLiteral` and `TemplateLiteralElement` AST nodes.

refactor(compiler-cli): handle template literals in ngtsc

60668ab

Updates the translators that convert expression ASTs to account for template literals.

feat(compiler): support untagged template literals in expressions

6af7ff8

Updates the compiler to support untagged template literals inside of the expression syntax (e.g. ``hello ${world}``).

crisbeto force-pushed the template-literals branch from e7fbd77 to 6af7ff8 Compare January 20, 2025 14:35

crisbeto commented Jan 20, 2025

View reviewed changes

devversion approved these changes Jan 20, 2025

View reviewed changes

crisbeto added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Jan 20, 2025

crisbeto removed request for JoostK, alxhub and clydin January 20, 2025 19:36

AndrewKushnir closed this in 98998bb Jan 21, 2025

AndrewKushnir pushed a commit that referenced this pull request Jan 21, 2025

refactor(compiler): tokenize template literals (#59230)

353005b

Reworks the lexer to produce tokens for template literal expressions. PR Close #59230

AndrewKushnir pushed a commit that referenced this pull request Jan 21, 2025

refactor(compiler): produce AST for template literals (#59230)

eb7e765

Updates the compiler to parse the template literal tokens into the new `TemplateLiteral` and `TemplateLiteralElement` AST nodes. PR Close #59230

AndrewKushnir pushed a commit that referenced this pull request Jan 21, 2025

refactor(compiler-cli): handle template literals in ngtsc (#59230)

6960ec0

Updates the translators that convert expression ASTs to account for template literals. PR Close #59230

JoostK reviewed Jan 21, 2025

View reviewed changes

PrajaktaB27 pushed a commit to PrajaktaB27/angular that referenced this pull request Feb 7, 2025

refactor(compiler): tokenize template literals (angular#59230)

6f627eb

Reworks the lexer to produce tokens for template literal expressions. PR Close angular#59230

PrajaktaB27 pushed a commit to PrajaktaB27/angular that referenced this pull request Feb 7, 2025

refactor(compiler-cli): handle template literals in ngtsc (angular#59230

44991a2

) Updates the translators that convert expression ASTs to account for template literals. PR Close angular#59230

angular-automatic-lock-bot bot locked and limited conversation to collaborators Feb 22, 2025

		const expression = i < ast.expressions.length ? ast.expressions[i] : null;
		if (expression !== null) {

Support untagged template literals in expressions #59230

Support untagged template literals in expressions #59230

Uh oh!

Conversation

crisbeto commented Dec 18, 2024

refactor(compiler): allow lexer to produce multiple tokens

refactor(compiler): tokenize template literals

refactor(compiler): produce AST for template literals

refactor(compiler): clean up tagged templates in output AST

refactor(compiler-cli): handle template literals in ngtsc

feat(compiler): support untagged template literals in expressions

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jelbourn commented Dec 20, 2024

Uh oh!

sheikalthaf commented Dec 26, 2024

Uh oh!

JeanMeche commented Dec 26, 2024

Uh oh!

sheikalthaf commented Dec 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

devversion left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

crisbeto left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AndrewKushnir commented Jan 21, 2025

Uh oh!

JoostK left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JoostK Jan 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

sheikalthaf commented Dec 26, 2024 •

edited

Loading

JoostK Jan 21, 2025 •

edited

Loading