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

Clobber XMLHttpRequest #6

Open
ligaz opened this issue Apr 10, 2015 · 5 comments
Open

Clobber XMLHttpRequest #6

ligaz opened this issue Apr 10, 2015 · 5 comments

Comments

@ligaz
Copy link

ligaz commented Apr 10, 2015

Currently the plugin requires a brand new Http API to be used instead of familiar ones like jQuery's Ajax or Angular's $http.It would be great if this plugin clobbers the root XMLHttpRequest so that everything works out of the box

@jprichardson
Copy link

Nah, I'm not in favor of this. Not sure if XMLHttpRequest can be clobbered, but if it can, this is something that the consumer of the plugin should do. I'm in favor of APIs, plugins, that cause no surprises and clobbering XMLHttpRequest out of the box could produce unintended side-effects.

@EddyVerbruggen
Copy link

I like the idea of it being a drop-in replacement per Stefans suggestion, but that would not be ideal for everybody. A strong argument against it is that the plugin doesn't support all HTTP methods.

Like I said, I like the general idea, so perhaps we can provide a JS adapter as part of the plugin distribution which users can drop into their project if they like.

@ligaz
Copy link
Author

ligaz commented Apr 14, 2015

A strong argument against it is that the plugin doesn't support all HTTP methods.

Both native frameworks support setting the HTTP method so I guess this is doable.

I like the idea of having the clobbering configurable either via config.xml preference or with JavaScript.

@rob3c
Copy link

rob3c commented Aug 15, 2016

Hi, I see that support for PUT and DELETE was added in the last few months. Does this mean we can manually clobber XMLHttpRequest reliably now, or are there still issues with doing this?

@rob3c
Copy link

rob3c commented Aug 24, 2016

Well, I forked the project and dug around a bit and figured I'd share some observations before I immediately forget everything. :-)

It turns out, it's not just about supporting more HTTP methods. There's also an overall lack of flexibility in terms of content types. Currently, the supported methods assume the content type is application/x-www-form-urlencoded when encoding the body regardless of what you set in the header.

There is a fork of the original cordovaHTTP project here that shows how to enable application/json in order to use JSON payloads. It's nice to see how to extend the project in a way to get JSON payloads working for POSTs, and I added JSON support for PUTs as well in a parallel manner in my secureHTTP fork.

However, that implementation requires adding a new postJson function on the cordovaHTTP javascript api that's implemented in a new CordovaHttpPostJson java class. Unfortunately, in terms of more general XHR-like support, structuring things as separate classes per http method per content type seems like it'll explode with permutations and be hard to support.

However, all of the separate http method classes pretty much have the same code in them, so it may be simpler to reduce the duplication and consolidate behavior in to a single class (maybe keeping the upload/download stuff separate) that accept an http verb parameter and dynamically encodes the body based on content-type. My java is pretty rusty, but I may give it a shot if I have time. I wanted to mention it here tho in case anyone interested in this who's more versed in java wants to give it a go.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants