request.js icon indicating copy to clipboard operation
request.js copied to clipboard

Possibility to use custom reviver in deserialization?

Open hansmbakker opened this issue 3 years ago • 4 comments
trafficstars

Crosspost from https://github.com/octokit/graphql.js/issues/414 since the request/response logic is done here

Is it possible to use custom revivers in the deserialization, so that Date strings could be serialized back directly to Date objects?

See https://stackoverflow.com/a/35923990/1114918 for reference.

Should I maybe do something with a custom fetch implementation?

hansmbakker avatar Oct 03 '22 16:10 hansmbakker

I see response.json() is hardcoded - is there a way I could override this behavior?

https://github.com/octokit/request.js/blob/5b03156ccb208ac0106305c5e5d8d2d8cbb431b3/src/fetch-wrapper.ts#L138-L142

hansmbakker avatar Oct 03 '22 16:10 hansmbakker

you can pass your a custom fetch function as { request: { fetch } } option

image

gr2m avatar Oct 03 '22 23:10 gr2m

@gr2m I saw how I could supply a custom fetch implementation, but I am wondering whether it is solved that way?

I found https://stackoverflow.com/questions/45425169/intercept-fetch-api-requests-and-responses-in-javascript on how to create a fetch modification, but the getResponseData() function I mentioned will still be called for the response - even if I already do deserialization in my modified fetch. This confuses me - I guess double deserialization won't work.

Could you explain how you would see that working?

hansmbakker avatar Oct 04 '22 08:10 hansmbakker

I got it now, please ignore my comment, it doesn't resolve your problem.

So what you want is in the JSON you receive from GitHub's API, all timestamp strings should be turned into Date objects? As in iterate through all keys of the JSON recursively, look for keys and/or values that look like time stamps, and turn them into JSON objects?

You could implement that with a request hook, using options.request.hook in @octokit/request options. But better to use https://github.com/octokit/core.js/ and its .hook API as it doesn't interfere with other request hooks that might have been previously registered.

Something like this

octokit.hook.wrap("request", async (request, options) => {
  const response = await request(options);
  
  // TODO: turn all timestamp strings into Date objects in `response`
  
  return response
});

I wonder why you want that though. It feels like a intransparent side effect that you might trip over easily in the future. I'd rather turn the timestamps into Date objects as needed.

gr2m avatar Oct 04 '22 18:10 gr2m

Closing as original request has been fulfilled

wolfy1339 avatar Jun 27 '24 18:06 wolfy1339