Skip to content
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

Switch Request and Response generics #1130

Closed
delvedor opened this issue Mar 27, 2020 · 4 comments · Fixed by #1132
Closed

Switch Request and Response generics #1130

delvedor opened this issue Mar 27, 2020 · 4 comments · Fixed by #1132

Comments

@delvedor
Copy link
Member

In #1119 we introduced a big change in how the type definitions work, and I'm very happy with the result, the code is more idiomatic and it's easier to use the TypeScript features.
While working on #970, I've noticed that there is still something we can do to improve even further the developer experience.
By reading this issue tracker and around on the internet, people is more interested in the response types rather than the request types, which in most of the cases are good enough with the body set to Record<string, any>.
Currently, the generics allow you to specify the request body, the response body and the context in this order:

client.search<RequestBody, ResponseBody, Context>(...)

I'm wondering if it does make more sense to have the ResponseBody generic as the first parameter instead since it will be used more often for defining Source objects and so on. While my OCD screams when seeing about this, I think it makes way more sense given that people will use the Response types more often.

client.search<ResponseBody, RequestBody, Context>(...)

What do you think?

cc @Mpdreamz @joshdover @restrry @nreese @dgieselaar @fox1t @villasv

@fox1t
Copy link

fox1t commented Mar 27, 2020

I absolutely agree to have generics in order of importance for developers. ResponseBody is what is used buy the rest of the code and therefore it deserves to be put first.

@Mpdreamz
Copy link
Member

Agreed my gut reaction wants request before response however I think

client.search<SearchResponse<Document>>({
   body: {
      query: { match_all: {} }
   }
});

Is a much nicer experience for folks who develop their search in e.g the kibana console then port it over to the client and look to get type safety when programming against the response.

The alternative would be to force the user to specify the body they just wrote is untyped:

client.search<any, SearchResponse<Document>>({
   body: {
      query: { match_all: {} }
   }
});

@delvedor
Copy link
Member Author

I've opened #1132 with the changes proposed in this issue so you can see how the client will change and maybe try it out!

@villasv
Copy link
Contributor

villasv commented Mar 31, 2020

By reading this issue tracker and around on the internet, people is more interested in the response types rather than the request types

TBH I'm more interested in typing the body parameters because they help so much when people are not 100% familiar with Elasticsearch APIs, allowing the IDE to provide rich auto-complete. Still, I understand that return values are much more relevant to keep my own code well typed.

So all things considered, I'm indifferent as to which one comes first. I'm always specifying both. But it makes sense to me that lots of people may only specify the return types and so it should come first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants