Got: Ky-like response transformations

Created on 9 Dec 2018  路  5Comments  路  Source: sindresorhus/got

What problem are you trying to solve?

174 #318 #383 #467 #511 #600

Describe the feature

Usage:

got(url, options).json();     // a Promise returning JSON
got(url, options).buffer();   // a Promise returning Buffer
got(url, options).fromData(); // a Promise returning From Data

Let options.json be only responsible for the request. To parse the response you'll need to set options.responseType to json or just call got(...).json()

Checklist

  • [x] I have read the documentation and made sure this feature doesn't already exist.
enhancement

Most helpful comment

I'd ask the community if it's unclear what convenience options they'd like to have or remove the option and see what issues pop up.

Well, it has been discussed in 6 issues mentioned above. I think it's enough to make a decision :)
No need to rush, there's lots of time though :P

I'd specify a deserialization or parsing function and optionally add some convenient way to define them.

Very good idea! I've been thinking about this for a while:

got.modifiers.register('json', promise => {
    return promise.then(response => {
        response.body = JSON.parse(response.body);
        return response;
    });
});

got(url, options).json();

What do y'all think about that?

I feel preserving the positive momentum you've got going in improving got is much more valuable than the above

Thank you, but I have to disagree - your help is no less valuable than mine, so please don't lower your confidence :)

All 5 comments

If we go for this, we could potentially remove the encoding option since got(...) and got(...).buffer() would do the same.

Not sure what this has to do with the json option 馃 . That option is there for convenience. If the common use case is to send and receive JSON, that's what I would have it cover. Whether that's through Ky-like response transformations seems beside the point. I'd ask the community if it's unclear what convenience options they'd like to have or remove the option and see what issues pop up.

Secondly, you're proposing to split deserializing from serializing and content-type or accept header setting. I proposed this in #174 back in 2016. I still quite like that idea, although at the time Sindre felt it was too imaginary a problem.

As for the form of this interface, I get that Ky does it that way since it lives in browser land. I've always felt using methods to consume response objects containing streams, reading them to completion and then deserializing using a function unique to that particular method is quite ugly. Besides, got's promise interface already buffers the stream. I'd specify a deserialization or parsing function and optionally add some convenient way to define them.

Disregard my points at your leisure, I feel preserving the positive momentum you've got going in improving got is much more valuable than the above @szmarczak 馃憣 馃槡.

I'd ask the community if it's unclear what convenience options they'd like to have or remove the option and see what issues pop up.

Well, it has been discussed in 6 issues mentioned above. I think it's enough to make a decision :)
No need to rush, there's lots of time though :P

I'd specify a deserialization or parsing function and optionally add some convenient way to define them.

Very good idea! I've been thinking about this for a while:

got.modifiers.register('json', promise => {
    return promise.then(response => {
        response.body = JSON.parse(response.body);
        return response;
    });
});

got(url, options).json();

What do y'all think about that?

I feel preserving the positive momentum you've got going in improving got is much more valuable than the above

Thank you, but I have to disagree - your help is no less valuable than mine, so please don't lower your confidence :)

Well, it has been discussed in 6 issues mentioned above. I think it's enough to make a decision :)
Hm. I agree!

Regarding the register function. Looks good 馃憣. A small note is I would prefer an interface that only lets you pass a function for a given content type once. I would, therefore, gravitate towards some sort of option. Maps are like simple functions and I love simple functions haha.

It would also obliviate the need to call the parser. After all, why pass us the parser if we don't do the parsing for you?

Something like:

got({
  registerParsers: {
    'application/json': JSON.parse
  }
})

I am still not sure about the register function. It's like calling an afterResponse hook but just with a function. Also, the only use case I have come up with is parsing the body. I think there's a better solution...

Your proposal sounds very good! I'd just modify it a bit :)

const instance = got.extend({
    modifiers: {
        'json': JSON.parse
    }
});

await instance(url).json();

@sindresorhus Any thoughts?

Was this page helpful?
0 / 5 - 0 ratings

Related issues

framerate picture framerate  路  4Comments

carvallegro picture carvallegro  路  4Comments

AxelTerizaki picture AxelTerizaki  路  3Comments

alvis picture alvis  路  3Comments

quocnguyen picture quocnguyen  路  4Comments