Conversation
…plate expressions.
@@ -3516,8 +3517,8 @@ declare namespace ts { | |||
function updateCall(node: CallExpression, expression: Expression, typeArguments: ReadonlyArray<TypeNode> | undefined, argumentsArray: ReadonlyArray<Expression>): CallExpression; | |||
function createNew(expression: Expression, typeArguments: ReadonlyArray<TypeNode> | undefined, argumentsArray: ReadonlyArray<Expression> | undefined): NewExpression; | |||
function updateNew(node: NewExpression, expression: Expression, typeArguments: ReadonlyArray<TypeNode> | undefined, argumentsArray: ReadonlyArray<Expression> | undefined): NewExpression; | |||
function createTaggedTemplate(tag: Expression, template: TemplateLiteral): TaggedTemplateExpression; | |||
function updateTaggedTemplate(node: TaggedTemplateExpression, tag: Expression, template: TemplateLiteral): TaggedTemplateExpression; | |||
function createTaggedTemplate(tag: Expression, typeArguments: NodeArray<TypeNode>, template: TemplateLiteral): TaggedTemplateExpression; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is an API breaking change. we need to document it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the other option is to put it at the end, or have multiple overloads. check with @rbuckton
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I wanted to get the conversation rolling on this one. Every time we change the AST, these factory functions get pretty annoying to update. I wonder whether VMs have gotten good at optimizing the "named arguments" pattern for options-bag style APIs.
src/compiler/factory.ts Outdated
@@ -1032,17 +1032,19 @@ namespace ts { | |||
: node; | |||
} | |||
export function createTaggedTemplate(tag: Expression, template: TemplateLiteral) { | |||
export function createTaggedTemplate(tag: Expression, typeArguments: NodeArray<TypeNode>, template: TemplateLiteral) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the API would be easier to use if the parameter is typed as ReadonlyArray
and it's converted to a NodeArray
in the function body.
this also applies to the update function below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call!
…ate'/'updateTaggedTemplate'.
src/compiler/checker.ts Outdated
@@ -17749,7 +17749,11 @@ namespace ts { | |||
let typeArguments: NodeArray<TypeNode>; | |||
if (!isTaggedTemplate && !isDecorator && !isJsxOpeningOrSelfClosingElement) { | |||
if (isTaggedTemplate) { |
weswigham Apr 19, 2018 •edited
LoadingUh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm pretty sure this duplicates the logic in the else if
below, no? You should just need to add to the CallExpression
cast below. (I'd add a CallLikeExpressionWithTypeArguments
, that's CallLikeExpression
sans decorators, which are the only one without them)
@MartinJohns it's on my backlog, sorry. 😞 @sergey-shandar Not sure what you're asking about, but any problems or authoring questions should be filed as a new issue or as a StackOverflow question respectively. |
@DanielRosenwasser I think you mean @sergeysova. |
| Q | A | ------------------------ | --- | Fixed Issues? | #7747 (partly) | : Bug Fix? | | Major: Breaking Change? | | Minor: New Feature? | Yes | Tests Added + Pass? | Yes | Documentation PR | | Any Dependency Changes? | | License | MIT @JamesHenry This changes the AST format. CC @DanielRosenwasser for review. Supports parsing type arguments on tagged template calls. Should wait on microsoft/TypeScript#23430 to be merged so we're sure we have the final syntax.
This pull request allows users to pass generic type arguments to tagged template strings.
Fixes #11947
Background
Tagged templates are a form of invocation introduced in ECMAScript 2015. Like call expressions, generic functions may be used in a tagged template and TypeScript will infer the type arguments utilized:
However, in some cases there are no inference candidates
or, type arguments cannot be inferred because TypeScript is conservative in its inferences
Parser changes
The core change is a new intermediate grammar production between MemberExpression and CallExpression/NewExpression
Then CoverCallExpressionAndAsyncArrowHead (which is the grammar production that currently transitions a CallExpression into a MemberExpression) no longer references MemberExpression Arguments, but instead looks more like:
Similarly, NewExpression no longer references
MemberExpression
, and instead goes forWithin our mechanics for MemberExpression still use the following production for tagged template expressions (which you can see in
parseMemberExpressionRest
)This way we always try to parse out a template after a MemberExpression, even when there are no type arguments.