Skip to content
This repository has been archived by the owner on Jun 28, 2024. It is now read-only.

Support generic type in ServiceClientResponse #106

Open
cberg-zalando opened this issue Apr 22, 2020 · 4 comments
Open

Support generic type in ServiceClientResponse #106

cberg-zalando opened this issue Apr 22, 2020 · 4 comments

Comments

@cberg-zalando
Copy link
Member

To avoid type clashes due to changes in perron's typings, I looked into bumping the version in translation-manager from 0.11.1 to 0.11.4.
Due to the change from any to Buffer | string | object, this would now require adding type casts throughout the code.

Do you see any problem, if we change the API to extend ServiceClientResponse with a generic type, which can be then also passed via the ServiceClient::request method?

@ruiaraujo
Copy link
Contributor

We can add a generic type that default to the union. @cberg-zalando will you push a PR?

@cberg-zalando
Copy link
Member Author

Sure, I can do this. To be backwards compatible, it is just a question what kind of type to assign by default to the generic parameter. I would propose any, which would actually restore backwards comptability to the release before 0.11.4.

@shuhei
Copy link
Contributor

shuhei commented Apr 22, 2020

I agree to keep the API compatible in 0.11.x. So, any as the default sounds good to me.

I also thought of adding more restriction like:

request<T extends (Buffer | string | object) = any>(options) : Promise<T>;

but it doesn't work as expected somehow.

Also, the parsed JSON type can be a boolean, number or an array in addition to string and object. So, we need a bit more consideration if we want a narrower type than any.

@cberg-zalando
Copy link
Member Author

I just gave this a revisit as I just didn't have any time to look into this issue before. I have a WIP commit, but I have run into a problem.

If add the generic type <P = any> to request(options: ServiceClientRequestOptions) method, I run into an issue that due to the change introduced in #105, TypeScript fails due to P and string | Buffer having nothing in common.

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

No branches or pull requests

3 participants