Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make it possible to provide FormData transparently #60

Open
ajorias opened this issue May 19, 2018 · 10 comments
Open

Make it possible to provide FormData transparently #60

ajorias opened this issue May 19, 2018 · 10 comments

Comments

@ajorias
Copy link

ajorias commented May 19, 2018

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
Copy link
Owner

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
Copy link
Author

ajorias commented May 19, 2018

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
Copy link
Owner

I like that too! Great ideas!

@cferdinandi
Copy link
Owner

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

@AugustMiller
Copy link

AugustMiller commented Sep 9, 2019

@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
Copy link
Owner

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

@AugustMiller
Copy link

AugustMiller commented Sep 9, 2019

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
Copy link
Owner

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

@AugustMiller
Copy link

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
Copy link
Owner

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants