fix(39744): make template literals more spec compliant #45304
Conversation
typescript-bot
added
the
For Milestone Bug
|
|
|
@rbuckton I don't think this is a 4.4 regression, but it might be worth shipping in 4.5 nonetheless. |
Apparently we don't have any tests that use template literals that emit source maps, because I would have expected to see source map changes. Can you add a test that includes source map emit so that we can verify the source map output?
|
Rebased to resolve conflicts and added a test which emits source map. Is this test adequate? While resolving conflicts, I removed the code added in #46287, but this shouldn't be a problem since tsc wouldn't omit empty "head" string literals with this PR. |
The only downside is that this increases emit size by 6 characters for every interpolation, but there's really no other way to do this correctly without some increase in emit output for <=ES5.
These changes add support for interpreting `String.prototype.concat` calls. We need to support it, because in TypeScript 4.5 string template expressions are transpiled to `concat` calls, rather than string concatenations. See microsoft/TypeScript#45304.
These changes add support for interpreting `String.prototype.concat` calls. We need to support it, because in TypeScript 4.5 string template expressions are transpiled to `concat` calls, rather than string concatenations. See microsoft/TypeScript#45304. PR Close #44167
These changes add support for interpreting `String.prototype.concat` calls. We need to support it, because in TypeScript 4.5 string template expressions are transpiled to `concat` calls, rather than string concatenations. See microsoft/TypeScript#45304. PR Close #44167
| function addTemplateHead(expressions: Expression[], node: TemplateExpression): void { | ||
| if (!shouldAddTemplateHead(node)) { | ||
| return; | ||
| expression = factory.createCallExpression( |
There was a problem hiding this comment.
Curious - why create the concat call inside the loop of template spans instead of outside the loop?
In other words, why does a${x}b${y}c emit "a".concat(x, "b").concat(y, "c") instead of emitting "a".concat(x, "b", y, "c")?
There was a problem hiding this comment.
@leebyron i believe the observable order of exceptions from ToString calls may require the chained calls; babel had the same change (please double check me)
There was a problem hiding this comment.
Aha, yes thank you for this.
There's a test included in this change that would break based on that idea
class C {
counter: number;
constructor() {
this.counter = 0;
}
get foo() {
this.counter++;
return {
toString: () => this.counter++,
};
}
}
const c = new C;
export const output = \`\${c.foo} \${c.foo}\`;
"".concat(c.foo).concat(c.foo) would run: get foo(), toString(), get foo(), toString()
and
"".concat(c.foo, c.foo) would run: get foo(), get foo(), toString(), toString()
The first one is what you'd expect to see from a template interpolation
These changes add support for interpreting `String.prototype.concat` calls. We need to support it, because in TypeScript 4.5 string template expressions are transpiled to `concat` calls, rather than string concatenations. See microsoft/TypeScript#45304. PR Close angular#44167
Fixes #39744
This PR makes the (compiled) template literals more spec compliant. As pointed out in the issue, currently the runtime semantics of the template literals is changed when transpiled. It now makes use of
String.prototype.concat()instead of+operator.I added an evaluation test as I thought it was impossible to test the runtime semantics otherwise. Let me know if there's a more appropriate folder I should place this kind of test in, or even this isn't necessary.