elbywan / wretch

A tiny wrapper built around fetch with an intuitive syntax. :candy:
MIT License
4.83k stars 98 forks source link

Problem sending blobs #75

Closed onel closed 4 years ago

onel commented 4 years ago

This might be more of a question than a bug. I'm trying to send a blob using wretch and the it looks like only {} is being sent.

Example:

let blob = new Blob()
wretch('https://reqres.in/api/users/2').options({
  mode: 'cors'
}).put(blob)

Am I doing something wrong here?

elbywan commented 4 years ago

Hi @onel,

Am I doing something wrong here?

The thing is, .put(body) has a check on the type of the body argument. When the type is an object, it assumes that the body is JSON data and attempts to serialize it with an application/json content type.

In your case the typeof (new Blob()) === 'object' so it applies the "wrong" logic.

I could add a check specifically for Blob, but it would only work in the context of a web browser because Blob is not part of the nodejs standard lib.

So I think the correct way to send a Blob would be to use the body() method instead:

const b = new Blob([1, 2, 3])
wretch('...').body(b).put()//.res()/json()…
onel commented 4 years ago

Thanks for the details @elbywan Well, a couple of things about that:

  1. in the readme is mentions that .put() is a equivalent to calling .body().put() so I assumed the behaviour is the same. Maybe the doc should mention the difference or, make the .put/post() behave the same. Regarding the latter, the problem might be the same with TypedArrays
  2. Should wretch assume that the body should be a JSON? Would it work better if it looks at the content type header, and stringify if needed? If the user hasn't called .content() yeah, sure it can do that (see if it's an object) but right now even when specifically setting the header it still makes it a JSON.
elbywan commented 4 years ago

in the readme is mentions that .put() is a equivalent to calling .body().put() so I assumed the behaviour is the same.

The Readme actually states that the body is set in a opinionated way.

Excerpt:

// This shorthand:
wretch().post({ json: 'body' }, { credentials: "same-origin" })
// Is equivalent to:
wretch().json({ json: 'body'}).options({ credentials: "same-origin" }).post()

Maybe the doc should mention the difference or, make the .put/post() behave the same.

Indeed it is not clear enough and I will add a note to make it more explicit. Thanks for pointing this out!

Should wretch assume that the body should be a JSON?

Assuming that the payload is a JSON (which is much more common than a Blob or TypedArray) is a very handy shortcut to use. (wretch().put() instead of wretch().json().put())

If the user hasn't called .content() yeah, sure it can do that (see if it's an object) but right now even when specifically setting the header it still makes it a JSON.

True 👍.

I will make the change and remove the JSON assumption if the Content-Type header has been set already.

elbywan commented 4 years ago

Just published the version 1.7.2 with the fix 📦.

onel commented 4 years ago

Awesome. Thanks for the quick fix :+1: This is a great lib

elbywan commented 4 years ago

Awesome. Thanks for the quick fix 👍 This is a great lib

Thanks! 😄

Closing the issue since the fix was published in the latest version.

RBrNx commented 3 years ago

Hey @elbywan, apologies for re-opening this but I felt it was relevant to the conversation. The changes made here are great, especially when compared to other fetch wrapper libs that don't support Blobs at all.

Unfortunately this only checks for the existence of a content-type: application/json header in the headers that are passed to the initial wretch() constructor, it does not account for headers that are passed by the .put() method.

It's fairly easy to workaround this by just passing the headers to the wretch constructor, but I thought it was an issue worth raising.

Works - Blob is sent without being stringified

wretch(s3PutObjectUrl, {
    headers: {
        'Content-Type':'image/jpeg',
        'Cache-Control': 'max-age=31557600',
    },
}).put(fileBlob),

Doesn't Work - Blob is stringified before being sent

wretch(s3PutObjectUrl).put(fileBlob, {
    headers: {
        'Content-Type':'image/jpeg',
        'Cache-Control': 'max-age=31557600',
    },
}),
elbywan commented 3 years ago

Hi @RBrNx,

apologies for re-opening this

No worries. 😄

Unfortunately this only checks for the existence of a content-type: application/json header in the headers that are passed to the initial wretch() constructor, it does not account for headers that are passed by the .put() method.

Absolutely, good catch, thanks for reporting this! I've just released a new version 1.7.5 with a fix.