-
Notifications
You must be signed in to change notification settings - Fork 17
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
Adds version compatibility checks to Queries #521
Conversation
bb60e34
to
65aee7e
Compare
65aee7e
to
9a4d09b
Compare
}); | ||
|
||
it("callsQueryAcceptsObject", { timeout: 10_000 }, async () => { | ||
const { resp } = await tsServer.sendQuickInfoRequest({ |
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 pretty cool, so how do you imagine we'd expand these types of tests? I'm just trying to understand the scope of things we'd be looking for
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 how vscode does stuff basically so anything you can visually see in vscode we can test this way
@@ -45,7 +45,7 @@ export type QuerySignatureFromDef<T extends QueryDefinition<any, any>> = | |||
|
|||
export type QueryParameterType< | |||
T extends Record<any, QueryDataTypeDefinition<any, any>>, | |||
> = NOOP<PartialByNotStrict<NotOptionalParams<T>, OptionalQueryParams<T>>>; |
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.
Why remove the NOOP? I remember when you explained this to me, it was to help expand the type out right,e.g. when you hovered over it, or am I misremembering?
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.
NOOP, while producing a simpler intellisense was also stripping out the mapped jsdoc. We will have to find another way possibly!
Additionally, I have added mapped description to query params and a TEST!! to ensure we don't break it.
Fixes #505