-
Notifications
You must be signed in to change notification settings - Fork 12.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow type arguments in generic tagged templates #23430
Conversation
…plate expressions.
Beautiful! Thank you :) |
Thank you so much! It really made my day. :) |
Can you please also update the spec according to the new changes? |
@@ -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) { |
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)
No update of the spec. :-( |
What about passed functions? const Example = styled.div`
font-size: ${p => p.size}rem;
flex-direction: ${p => p.row ? 'row' : 'column'};
` |
@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) | Patch: 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.