Fix emit issue regarding null/undefined in type annotations #11653
Conversation
| @@ -287,7 +287,7 @@ namespace ts { | |||
|
|
|||
| case SyntaxKind.Constructor: | |||
| // TypeScript constructors are transformed in `visitClassDeclaration`. | |||
There was a problem hiding this comment.
is comment above still relevant?
| if (node.questionToken | ||
| || node.type | ||
| || subtreeFlags & TransformFlags.ContainsDecorators | ||
| || isThisIdentifier(name)) { |
There was a problem hiding this comment.
can you add a test to check this condition?
There was a problem hiding this comment.
The only new functionality is the test for node.type, which is covered by transformsElideNullUndefinedType
| emitExpressionList(node, node.arguments, ListFormat.CallExpressionArguments); | ||
| } | ||
|
|
||
| function emitNewExpression(node: NewExpression) { | ||
| write("new "); | ||
| emitExpression(node.expression); | ||
| emitTypeArguments(node, node.typeArguments); |
There was a problem hiding this comment.
I can see why this is necessary, but do we have cases today when we need to emit type arguments or type annotations - asking because It will be nice to test this functionality
There was a problem hiding this comment.
Primarily it helped me catch other possible future errors.
|
👍 |
|
|
||
| class C<T> { | ||
| m0(): null { return null; } | ||
| m1(): undefined { return undefined; } |
There was a problem hiding this comment.
can you also add tests for null and undefined in type position for parameters for methods, arrow functions, function expressions, and setters.
There was a problem hiding this comment.
That's what transformsElideNullUndefinedType does.
There was a problem hiding this comment.
Sorry, misread your comment. I'll add tests for a few more scenarios.
When we compute and aggregate transform flags for a node, we were not detecting
nullorundefinedas a type. As a result, the TypeScript transformation did not remove the type annotation for several nodes.This change fixes the detection logic to correct the emit.
Fixes #11295
// CC: @mhegazy, @vladima