cferdinandi / atomic

A tiny, Promise-based vanilla JS Ajax/HTTP plugin with great browser support.
MIT License
540 stars 78 forks source link

Make it possible to provide FormData transparently #60

Open ajorias opened 6 years ago

ajorias commented 6 years ago

Hey,

I really like the simplicity of atomic.js. Yesterday I ran into a problem, though.

I want to upload a file via a FormData. However if I provide FormData to atomic.js (in contrast to just a simple object) it fails.

Would it be possible to enhance atomic.js so that if provided data is of type FormData atomic just passes it through to the xhr request?

cferdinandi commented 6 years ago

Absolutely, and this is an easy fix. I'll try to do this this week.

For my own reference on checking for the FormData object type: https://codepen.io/pen/

ajorias commented 6 years ago

Awesome. I added

    if (obj instanceof FormData) return obj;

to the param() function but that's no quit enough because of the default header

        'Content-type': 'application/x-www-form-urlencoded'

in case of a FormData it was best for me to let the request set the proper multipart content type and especially set the 'boundary' correctly so that the parameters arrived properly on the server.

Side note: In my local atomic.js version I added support for 'timeout' settings via options like:

    // Add timeout TODO: this was added by AJ (clubcooee) as of 07th of february 2018.
    if (settings.timeout) {
        request.timeout = settings.timeout;

Just before the .send()

cferdinandi commented 6 years ago

I like that too! Great ideas!

cferdinandi commented 6 years ago

I added FormData support and an option for timeout in the just released v4.1.0. Thanks for suggesting these!

AugustMiller commented 5 years ago

@cferdinandi Heya! Thanks for the lightweight library!

I think this issue persists—there's no way to un-set the Content-Type header specified in the defaults object (per @ajorias's note), at request-time.

When using FormData as data, the request is missing the boundary piece of the header, which makes the data unreadable, server-side. There may be more relaxed requirements in some server environments, but I found that the browser was at least able to do its auto-magic when this was removed from the dist files.

Would you be willing to revisit? I'm happy to issue a PR with this removed, but I'm weary that it would be breaking for others' uses. 😬

👋

cferdinandi commented 5 years ago

@AugustMiller I'm maybe missing something, but... can't you update the Content-Type header using the public options?

AugustMiller commented 5 years ago

Ordinarily, yeah—but the boundary value is generated by the browser, and as far as I can tell, can't be spoofed/reproduced without completely customizing how the body is “serialized,” which defeats the point 😉

Speaking specifically about the latter half of this:

content-type: multipart/form-data; boundary=----WebKitFormBoundarySBQKzaTBps2gkQaA

…which the browser sets automatically when XMLHttpRequest is given FormData, but not when the header has already been explicitly set.

Hope this makes sense?

Edit: Thanks for the quick response! 💞

cferdinandi commented 5 years ago

Ah, you actually need a way to "unset" items, then?

AugustMiller commented 5 years ago

I think so? I mean, this is probably the means to a backwards-compatible solution. 👍

In fiddling, I tried to unset by passing null, but it actually appended/joined the two settings together, rather than removing it—resulting incontent-type: multipart/form-data, null 🤔

cferdinandi commented 5 years ago

Yea, I'll need to add a new method to support this behavior.