-
Notifications
You must be signed in to change notification settings - Fork 12.8k
Suggest completions for type arguments of expressions #61758
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
base: main
Are you sure you want to change the base?
Suggest completions for type arguments of expressions #61758
Conversation
This internal utility function and corresponding type weren't in use, and `hasTypeArguments`'s guarding was buggy: - `node` could be a `NodeWithTypeArguments`, which is not a member of `HasTypeArguments` - else branch narrowing was wrong: `typeArguments` is optional, so it can be undefined while `node` is assignable to `HasTypeArguments`
Looks like you're introducing a change to the public API surface area. If this includes breaking changes, please document them on our wiki's API Breaking Changes page. Also, please make sure @DanielRosenwasser and @RyanCavanaugh are aware of the changes, just as a heads up. |
/** @internal @knipignore */ | ||
export function hasTypeArguments(node: Node): node is HasTypeArguments { | ||
return !!(node as HasTypeArguments).typeArguments; | ||
} |
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 internal utility function was unused, and I noticed it had buggy guarding semantics which tripped me up when I tried to use it:
node
could be aNodeWithTypeArguments
, which is not a member ofHasTypeArguments
- else branch narrowing was wrong:
typeArguments
is optional, so it can be undefined whilenode
is still assignable toHasTypeArguments
The former is trivially fixable while the latter is not, so I opted to purge this to avoid future confusion. Let me know if I should do something else instead.
export type HasTypeArguments = | ||
| CallExpression | ||
| NewExpression | ||
| TaggedTemplateExpression | ||
| JsxOpeningElement | ||
| JsxSelfClosingElement; |
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 removed this along with the hasTypeArguments
guard, but I think this type is public and if there's a chance this might cause breakage somewhere I could restore it (even if the internal hasTypeArguments
function remains purged).
If I do restore it, perhaps NodeWithTypeArguments
should be added to the union?
Previously, `getTypeArgumentConstraint` could only find constraints for type arguments of generic type instantiations. Now it additionally supports type arguments of the following expression kinds: - function calls - `new` expressions - tagged templates - JSX expressions - decorators - instantiation expressions
d01431a
to
e4bac38
Compare
Completions are now suggested within type arguments of the following expression types (based on type parameter constraints):
f<…>()
)new
expressions (new Foo<…>()
)tag<…>`blah`
)<Component<…>/>
)@decorator<…> class {}
)f<…>
)Fixes #61751.
Limitations
:
in a type argument literal) still don't happen, so No completions fromtsserver
for property values in generic type arguments #56299 should remain open.I might take a stab at this, but it'll be a separate effort from the present changeset.EDIT: I took a stab at this and have something that works, but it's stacked atop this branch so I'll see where this pull request goes before opening another one.getCompletionData
incompletions.ts
). This needs fixing to fully address the specific examples in No string completions in type arguments where constrained to string literal union #52898 and No IntelliSense for generic literal types #34771. EDIT: I also have a (stacked) fix for this.This comment may also be useful reading.