GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||||||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||||||||||||||||||||||||||||||||||
GeezerVison is showing github.com — Allow type arguments in generic tagged templates by DanielRosenwasser · Pull Request #23430 · microsoft/TypeScript Conversation
|
|
Beautiful! Thank you :) |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Thank you so much! It really made my day. :) |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Can you please also update the spec according to the new changes? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| 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.
The reason will be displayed to describe this comment to others. Learn more.
this is an API breaking change. we need to document it
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem 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 Ron Buckton (@rbuckton)
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem 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.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| } | ||
|
|
||
| export function createTaggedTemplate(tag: Expression, template: TemplateLiteral) { | ||
| export function createTaggedTemplate(tag: Expression, typeArguments: NodeArray<TypeNode>, template: TemplateLiteral) { |
There was a problem 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.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Good call!
Sorry, something went wrong.
There was an error while loading. Please reload this page.
…ate'/'updateTaggedTemplate'.
There was an error while loading. Please reload this page.
|
Mohamed Hegazy (@mhegazy), the API is purely additive now, minimizing breaking changes. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| let typeArguments: NodeArray<TypeNode>; | ||
|
|
||
| if (!isTaggedTemplate && !isDecorator && !isJsxOpeningOrSelfClosingElement) { | ||
| if (isTaggedTemplate) { |
There was an error while loading. Please reload this page.
There was a problem 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)
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
No update of the spec. :-( |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
What about passed functions? const Example = styled.div` font-size: ${p => p.size}rem; flex-direction: ${p => p.row ? 'row' : 'column'}; ` |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Martin Johns (@MartinJohns) it's on my backlog, sorry. 😞 Sergey Shandar (@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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Daniel Rosenwasser (@DanielRosenwasser) I think you mean Sova (@sergeysova). |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| 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.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.