sindresorhus / ky

🌳 Tiny & elegant JavaScript HTTP client based on the Fetch API
MIT License
11.91k stars 342 forks source link

Options are overwritten if json option is used #507

Closed ThanosDi closed 3 weeks ago

ThanosDi commented 1 year ago

Based on this line it seems that the original options are being overwritten with {body: this._options.body}.

this.request = new globalThis.Request(this.request, {body: this._options.body});

This causes issues with 3rd party libraries(for example highlight.io) when they try to copy the request headers from the options rather than the request object.

Is this intended?

sholladay commented 12 months ago

I think it was written that way for simplicity back when this code was only modifying the options object, before we started using a Request instance to help process the options and headers.

Now we should be able to pass the JSON body directly to the Request constructor instead of using the options object as an intermediary.

I support this change, but we'll need to make sure it doesn't cause regressions with retry requests. Retries have to recreate the request and reapply the options, but I can't remember if this code runs again on retries. If it doesn't, the retried request won't have a stringified body anymore.

We do have some retry tests, but I'm not sure if they'll catch that kind of bug.