-
Notifications
You must be signed in to change notification settings - Fork 10
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
Fix intellisense for aggregations and fill in more number types #43
Conversation
Generate changelog in
|
@@ -69,7 +73,9 @@ export interface BaseObjectSet< | |||
// OsdkObjectFrom<K, O, PropertyKeysFrom<O, K>> | |||
// >; | |||
|
|||
aggregateOrThrow: <const AO extends AggregateOpts<O, K, any>>( | |||
aggregateOrThrow: < | |||
AO extends AggregateOpts<O, K, AggregationClause<O, K>>, |
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 change made it make suggestions again
@@ -38,7 +39,8 @@ export type AggregationClause< | |||
K | |||
>[P]["type"] extends "string" | |||
? StringAggregateOption | StringAggregateOption[] | |||
: ObjectTypePropertyDefinitionsFrom<O, K>[P]["type"] extends "double" | |||
: ObjectTypePropertyDefinitionsFrom<O, K>[P]["type"] extends | |||
"double" | "integer" | "float" | "decimal" | "byte" | "long" | "short" |
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.
Just make more types work that I suspect will work
? NumericAggregateOption | NumericAggregateOption[] | ||
: ObjectTypePropertyDefinitionsFrom<O, K>[P]["type"]; | ||
: never; |
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 old one was wrong because you cant have an aggregation of { geohash: "geopoint" }
. Switching to never to be more accurate.
No description provided.