Treat NoSubstitutionTemplateLiteral like StringLiteral in more places #26330
Conversation
ghost
force-pushed
the
nosubstitutiontemplateliteral
branch
from
4816ac8 to
04d7355
Compare
ghost
force-pushed
the
nosubstitutiontemplateliteral
branch
from
04d7355 to
924890a
Compare
src/compiler/utilities.ts
Outdated
| @@ -2566,6 +2566,10 @@ namespace ts { | |||
| || kind === SyntaxKind.NumericLiteral; | |||
There was a problem hiding this comment.
what happens if you add || isNoSubstitutionTemplateLiteral(node) directly to isStringOrNumericLiteral ? Maybe you can just do that plus maybe change the name to be more accurate.
There was a problem hiding this comment.
The only remaining uses are in getPropertyNameOfBindingOrAssignmentElement. That function has no return type, but changing it and recompiling shows a number of errors. Looks like the return type does need to be PropertyName | undefined which excludes NoSubstitutionTemplateLiteral.
ghost
force-pushed
the
nosubstitutiontemplateliteral
branch
from
a8488af to
40972ba
Compare
| const kind = node.kind; | ||
| return kind === SyntaxKind.StringLiteral | ||
| || kind === SyntaxKind.NumericLiteral; | ||
| export function isStringOrNumericLiteralLike(node: Node): node is StringLiteralLike | NumericLiteral { |
There was a problem hiding this comment.
The name implies that this includes BigInt literals once they are added to the language. I don't know if that is correct for all call sites
There was a problem hiding this comment.
It looks like in most cases it would be -- { [0n]: true } is the same as { 0: true } so as long as IntLiteral#text doesn't include n this would work. @sheetalkamat is working on bigints.
There was a problem hiding this comment.
@Andy-MS #25886 I think is the nearly complete implementation at this point (unless we have multiple implementations at the ready). But yeah, I guess indexing with bigint is fine - it's just odd, since in the typesystem that needs to go under the number index signature since 0n and 0 have the same string representation. Probably worth testing in #25886, at least! It's probably not a concern for this PR directly, since it'll need to be dealt with at the point they're added.
This is an immediate fix to the particular repro from #26321 but doesn't solve the general case. Still, don't see any reason not to treat these the same.