fix(compiler-cli): interpret string concat calls

[crisbeto]

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.

[petebacondarwin]

Nice addition @crisbeto.

I "think" that this change to TS will not break $localize processing in @angular/localize because even in ES5 mode a tagged template handler will not be a simple string, but instead will continue to be a function call with the "message parts" added as parameters.

That being said, we should consider changing the output of $localize processing to follow this same approach (rather than using +) to get the same level of spec compliance. For context the following localized message $localize `hello ${name}!`; will be translated to, say, "bonjour " + name + "!"; - and I guess it should be translated to "bonjour".concat(value).concat("!");.

[petebacondarwin]

All the tests here appear to test the element access form: a['concat'].
I assume that the changes also support property access form a.concat too? If so, can we add tests to prove it?

[crisbeto]

I think that these tests don't have any typings so using the .concat access throws a type error. All the other tests use the same trick.

[crisbeto]

This change wasn't prompted by $localize, but rather because of an ngcc test that has a component template with interpolations: https://github.com/angular/angular/blob/master/packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts#L539.

I caught it in my PR to add support for TS 4.5: #44164

[JoostK]

Maybe we should consider changing the ngcc integration tests to not rely on the workspace TS version, as ngcc will not have to deal with TS >=4.4 emit output in real life. This isn't the first time where we need to adapt ngtsc to support ngcc and I'd like to reduce the complexity/effort around TS updates going forward.

[petebacondarwin]

Maybe we should consider changing the ngcc integration tests to not rely on the workspace TS version, as ngcc will not have to deal with TS >=4.4 emit output in real life. This isn't the first time where we need to adapt ngtsc to support ngcc and I'd like to reduce the complexity/effort around TS updates going forward.

I think in this case, this is actually a valuable contribution to the compiler generally, expanding the syntaxes understood by our static evaluation of TS.

[crisbeto]

The test revealed a legitimate issue though. It meant that something like this would throw a compiler error where it previously didn't:

const name = '...';

@Component({
  template: `Hello ${name}`
})

[petebacondarwin]

But only if you were targeting es5 I think. And even then, I think the Angular compilation happens before the downleveling.

[JoostK]

That's indeed only relevant when ngtsc is processing JS code, which is only the case with ngcc. Regular compilations perform analysis using the original TS source code and see the template literal as written by the user.

With this commit, the user would be allowed to write:

const name = '...';

@Component({
  template: 'Hello '.concat(name),
})

TBH I'm not convinced that we should support this (but we already do support Array.prototype.concat and Array.prototype.slice which were added to support ngcc, but they're also available to regular ngtsc compiles)

[crisbeto]

IMO we still need to support cases like this as long as ngcc exists in the codebase.

[petebacondarwin]

IMO we still need to support cases like this as long as ngcc exists in the codebase.

That is only necessary if people are able to build ViewEngine formatted libraries using TS 4.5, which is not the case, unless we updated 12.2.x to support TS 4.5.

But I do think it is not a bad idea to support this construct. It would also allow us to do things like:

const prefix = 'moo';

@Component({selector: previx.concat('-my-comp'), ...})
...

(I think...)

[alxhub]

I agree with @JoostK's assessment that this is not strictly necessary, because there is no way that ngcc could encounter a package in the wild that's built with TS 4.5, since such a package would need to be built with Angular 12 or lower, and that doesn't support TS 4.5.

That said, supporting this is easy, and it unblocks the 4.5 update work while we think about changing how we generate ngcc's test code (which we should also do).

[jessicajaniuk]

This PR was merged into the repository by commit 48ca7dc.

[angular-automatic-lock-bot]

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.}