-
Notifications
You must be signed in to change notification settings - Fork 89
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
remove Document
and Documents
#1424
Conversation
@@ -327,10 +327,10 @@ class Index<T extends Record<string, any> = Record<string, any>> { | |||
* @param parameters - Parameters applied on a document | |||
* @returns Promise containing Document response | |||
*/ | |||
async getDocument<T = Record<string, any>>( | |||
async getDocument<D extends Record<string, any> = 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 do we need to keep extending here? Maybe there is a case that I don't see. Is D = T
not enough ?
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.
T
extends Record<string, any>
on index creation but you can override it by Index<{}>().getDocument<number>()
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 see, but imagine I want to get Movie
documents
type Movie = {
id: number
name: string
}
Then, overwrite the type of index
by doing like you said client.index('x').getDocument<Movie>(1)
.
In this case Movie
is doing to be D
. But why does D
needs to be extended from Record<string, any>
? I'm not expecting any other field in my Movie
.
Sorry if I'm misunderstanding something
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.
your type Movie
already extends Record<string, any>
it's valid to call getDocument<Movie>()
with your type example
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.
Ah I see! Thanks :)
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.
In the same vain, should we have added that as well in search
?
Line 102 in b1ed904
async searchGet<D = T>( |
async searchGet<D extends Record<string, any>> = 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.
yes
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.
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.
LGTM 🔥
bors merge |
Pull Request
Related issue
Fixes #1421
What does this PR do?
remove
Document
andDocuments
PR checklist
Please check if your PR fulfills the following requirements:
Thank you so much for contributing to Meilisearch!