Fixed serialization of param names in setters from JS files #55393
Conversation
typescript-bot
added
the
For Uncommitted Bug
typescript-bot
added
For Backlog Bug
| get x(): number; | ||
| } | ||
| export class G { | ||
| set x(args: number); |
There was a problem hiding this comment.
this could be generated as args_0 (or smth like that) - but I don't think it's worth the added code complexity
| const isRest = parameterDeclaration && isRestParameter(parameterDeclaration) || getCheckFlags(parameterSymbol) & CheckFlags.RestParameter; | ||
| const dotDotDotToken = isRest ? factory.createToken(SyntaxKind.DotDotDotToken) : undefined; | ||
| const name = parameterDeclaration ? parameterDeclaration.name ? | ||
| function parameterToParameterDeclarationName(parameterSymbol: Symbol, parameterDeclaration: ParameterDeclaration | JSDocParameterTag | undefined, context: NodeBuilderContext) { |
There was a problem hiding this comment.
I decided to extract some useful bits to separate helpers instead of reusing symbolToParameterDeclaration. To reuse symbolToParameterDeclaration in full I'd have to introduce some extra parameters there (like preserveType, preserveQuestionToken and I don't think it's worth it. And even with that, I'd have to check if it would for bizarre setters declared using rest params and potentially put even more work into supporting that.
| }); | ||
|
|
||
| Debug.assert(setter && isFunctionLikeDeclaration(setter)); | ||
| const paramSymbol: Symbol | undefined = getSignatureFromDeclaration(setter).parameters[0]; |
There was a problem hiding this comment.
This annotation isn't enforced, it acts as a documentation hint~. The problem is that indexed access here won't return | undefined and TypeScript prefers the expression type here over the declared type (it's a known design decision). In practice, this array might be empty and I handle it in the implementation to make the truthiness check less weird I decided to annotate this here.
| /*modifiers*/ undefined, | ||
| /*dotDotDotToken*/ undefined, | ||
| "arg", | ||
| paramSymbol ? parameterToParameterDeclarationName(paramSymbol, getEffectiveParameterDeclaration(paramSymbol), context) : "value", |
There was a problem hiding this comment.
I decided to use "value" as a new fallback here to make it more consistent with:
TypeScript/src/compiler/transformers/declarations.ts
Lines 897 to 903 in 202eff7
| /*modifiers*/ undefined, | ||
| /*dotDotDotToken*/ undefined, | ||
| "arg", | ||
| paramSymbol ? parameterToParameterDeclarationName(paramSymbol, getEffectiveParameterDeclaration(paramSymbol), context) : "value", |
There was a problem hiding this comment.
I decided to use "value" as a new fallback here to make it more consistent with:
TypeScript/src/compiler/transformers/declarations.ts
Lines 897 to 903 in 202eff7
|
@jakebailey reordered this, let me know how it feels now |
There was a problem hiding this comment.
LGTM; https://github.dev/microsoft/TypeScript/pull/55393/files shows the diff a lot better still even than github at this point. Wish github could use that algorithm somehow :(
|
|
fixes #55391