Skip to main content

Bug Tracker

Side navigation

#3087 closed enhancement (fixed)

Opened June 25, 2008 07:43PM UTC

Closed August 07, 2008 01:08PM UTC

Last modified August 07, 2008 07:51PM UTC

Request: minor refactor of "ajax" function to have a separate 'factory' method for creating XHR instances

Reported by: shadedecho Owned by: flesler
Priority: minor Milestone: 1.3
Component: ajax Version: 1.2.6
Keywords: ajax Cc:
Blocked by: Blocking:
Description

The "ajax" function is kind of monolithic in that there are some pieces of its functionality that would prvoide some very helpful extensibility if they were separated out into other functions.

Namely, the creation of the XHR instance is embedded deep in this function, which makes overriding of that behavior (for unit testing, extensions with other XHR compatible objects, etc) nearly impossible without re-implementing the rest of the "ajax" function's logic.

Other frameworks (Dojo, YUI, Prototype, and ExtJS, for instance) all have "factory" functions which their various ajax mechanisms use to create the XHR instance to use for the request. Something very similar would be helpful with jQuery.

For instance, it could be something like:

function XHRTransport() {

return new XMLHttpRequest();

}

Obviously, that's too simple, because you'd want the already present logic of instantiating the ActiveX object for IE browsers, etc. But you get the idea, a function that returns the XHR instance and then can be used inline inside of the ajax function, without any further changes, just replacing that block of code with a call to this function.

This was discussed on the dev list and the suggestion was made to put in a ticket here, possibly for a patch or for consideration in a subsequent release.

Attachments (2)
  • ajax_transport.diff (1.0 KB) - added by choan June 26, 2008 12:15AM UTC.

    Makes the Ajax transport replaceable

  • custom-xhr.patch (1.3 KB) - added by flesler August 06, 2008 10:15PM UTC.

    New proposition

Change History (19)

Changed June 25, 2008 09:06PM UTC by flesler comment:1

component: coreajax
need: ReviewTest Case

Hi shadedecho, thanks for adding the ticket.

Note that you're not saying why do you think this is important, argumenting that ''"other frameworks do"'' is not enough to justify a modification.

Please add a test case where you really need to override this behavior.

Also, if you have a patch in mind, feel free to attach it.

Changed June 26, 2008 12:23AM UTC by choan comment:2

As I offered to provide a patch, here it goes (all test passed).

For the "why", I think this change would be useful in mocking the Ajax transport for unit testing.

Changed June 26, 2008 06:13PM UTC by shadedecho comment:3

>>(for unit testing, extensions with other XHR compatible objects, etc)

That was my attempt at providing a brief "why". Choan has suggested the "unit testing" argument, which I just echo'd. Let me explain the other one in more detail:

http://flxhr.flensed.com/

flXHR is designed to be a drop-in replacement for the browser's native XHR object. That is, it is API-compatible with the native XHR, so it can be dropped-in to existing code which knows how to speak with and deal with that interface, and it should work seamlessly just like XHR. However, it provides a number of very helpful extensions, including (because it's based on Flash) direct, cross-domain communication (with server policy authorization), more robust error callback handling, built-in timeouts, and convenience methods for configuration.

If you look at Demos 7-10 on this page: http://flxhr.flensed.com/demo.php

You'll see that I provide proof-of-concept simple demos where I adapt the other major JS frameworks (Dojo, ExtJS, YUI, and Prototype) to swap in the flXHR for the native XHR, and the rest of the framework's mechanics work properly against it.

Basically, my hope is that this can become a viable alternative to the other cross-domain "workarounds" like IFRAMEs and dynamic SCRIPT tags, because I think it's more efficient and I think it's more secure. In that way, perhaps flXHR could eventually become an optional or default "plugin" of sorts for jQuery (and other frameworks) which removes the need to rely on the other wonky workarounds and allows more direct code because you only ever have to speak "XHR-API". :)

You can see also on that Demo page that I have a placeholder at Demo 11 for if I can find a way to swap in flXHR into jQuery similarly to how I adapt the others.

Personally, I *strongly* prefer using jQuery over any other framework, so it's been quite a burr in my side while developing flXHR that I've been unable to adapt jQuery to use flXHR, while I can adapt the other competitors just fine.

Of course, you can consider that either a flaw or a feature, but I just believe flexibility is better for all involved.

Changed June 26, 2008 10:50PM UTC by choan comment:4

shadedecho, does my path provide enough hooks to replace the XHR transport with your fiXHR?

Changed June 27, 2008 10:43AM UTC by shadedecho comment:5

Yes, I was able to use your patch to be able to make a patched version of 1.2.6, which I updated the demos on my page with, and have as available for download for anyone trying to use flXHR with jQuery.

Any idea on if/when this patch might make it into an official jQuery release?

Changed June 30, 2008 03:09PM UTC by flesler comment:6

owner: → flesler
status: newassigned

I'm really not sure, I'll ask to other developers in the team.

It isn't a big change (that's a good thing), but doesn't seem like something that many people would need (bad thing).

Creating a different XHR that respects all the API seems very specific to me.

Let me ask around.

Changed June 30, 2008 04:40PM UTC by joern comment:7

resolution: → wontfix
status: assignedclosed

Unit-testing works well without this modification. Extracting the factory method for just one user isn't enough.

Changed July 03, 2008 02:41AM UTC by shadedecho comment:8

I think that's a really poor decision. It isn't just for "one user"... I've got well over a thousand people using my library now, more everyday, and many of them have emailed asking for better support with jQuery. I had hoped there was light at the end of the tunnel given the patch provided in this ticket. Now I have to tell them that jQuery devs decided flXHR was irrelevant. That's fine, it's your right to do so.

I would have thought such a small change (like 5 or 6 lines total) with the potential to benefit all jQuery users and other libraries would not have received such a casual dismissal.

I think it's a shame. It makes me feel like my choice of supporting jQuery the last several years in all my sites, in deference to other frameworks, was perhaps a bad choice, given that the other ones play nicely with others and cleary jQuery does not.

Sorry to have wasted your time. I'll move on to some other framework which is more friendly.

Changed July 03, 2008 03:17PM UTC by john comment:9

Replying to [comment:8 shadedecho]:

I responded on your blog, but I'll post it here, as well, for posterity:

Why not just replace the XMLHttpRequest object with your custom one? You wouldn’t have to modify any libraries and the user would get the full benefit.

I’ve created fake XMLHttpRequest objects in the past - simply saving a reference to the old object and adding in my new methods (to support cross-domain requests). It certainly sounds like something similar can be done here.

“But on principle alone, I think their poor social skills are something that should at least be called out and kept to task.”

I think that’s just being mean. We discussed this extensively on the mailing list - with you - and the only possible use case we could come up with was possibly doing mock unit testing (external to your library). But once it was determined that not even unit testing was a valid use case there really wasn’t much left.

A point to consider is that for every single change we make to the API (and every new API point we add) we have to document and support it indefinitely - if we were to ever change it or remove it we would have to provide alternatives. We never take API changes lightly - which is why we always look for multiple use cases whenever we add a new one. At this point, this change does not warrant it. Especially since when alternatives exist.

Sorry to have wasted your time. I'll move on to some other framework which is more friendly.

I'm confused - you didn't ask a single other framework to add anything - how are we any more or less friendly than them? We took the time to talk with you extensively on the mailing list, and in here, and considered your patch and recommendation. Considering that a viable alternative exists and that no other use cases exist it's increasingly hard to consider this API change.

We are absolutely still open to reconsidering - we're always open and love to be proven wrong. I hope you'll reconsider and keep the discussion going with us.

Changed July 03, 2008 08:06PM UTC by shadedecho comment:10

I also commented in response on the blog, apologizing for unintentionally coming across as mean with some poor choice of words on my part. For posterity sake, this blog thread is located here:

http://www.flensed.com/fresh/?p=10


I addressed several of your comments on my blog response, so I won't duplicate them here as well. But to address a few here that were not addressed over there:

>> "...and considered your patch and recommendation..."

I didn't make the patch. It came from another user on your dev list, I assume someone who's familiar with your dev processes, I don't really know. They obviously felt (at least at first) there was some merit to my request. I was thrilled to see that someone "on the inside" (at least from my perspective) understood where I was coming from.

I would not have been so presumptuous as to tell jQuery how to change their library, I was just asking if there was a simple way to make the XHR instantiation part alterable. I was glad to see that according to the patch, it seemed like it was a simple change, and hoped that this would be enough to prevent it from having too much push back.

I certainly was not (and am not) trying to start a salvo over the matter.

>> "I'm confused - you didn't ask a single other framework to add anything - how are we any more or less friendly than them?"

I have corresponded with other frameworks before in the past, on other topics, and even when questions or suggestions to them were not accepted, I didn't come away from the experience feeling like I'd been dismissed simply because I was "just one person" asking.

Moreover, I didn't have to ask this request of the other frameworks, because I assume somewhere along the way, someone already had similar thoughts of flexibility, extensibility, cooperation, interoperation, etc, and they chose to create an XHR factory method already. Because jQuery is still my favorite library, I definitely was interested in pursuing if there was a nicer, cleaner way of adapting my project into jQuery, not really just for my benefit, but so that flXHR and jQuery could co-exist out in the world and not have to be mutually exclusive for anyone who chooses to use one or both.


My biggest reason for the request was not just for flXHR, but more in general to fight for a landscape in open source software that allows the best interoperability. Moreover, on the specific topic of XHR, and how it's changing in the new generation of browsers, I'm attempting to make the point (and prove it) that the best/better way is to consolidate around a common, well-known API (native XHR), and allow drop-in replacements (hot-swappable, essentially) to deal with the various issues that arise, such as cross-domain security. But that effort will only ever be as good as the cooperation level I can achieve with all the other major players who are already in the game.

For the other frameworks, this question was much simpler. For jQuery, I had hoped it would not be a difficult battle.

Again, I apologize for some of my reactionary wording, it was not meant to be insulting or mean, but rather to draw a clear distinction about the opinions I had, and the impression I developed from my brief foray into the jQuery dev world.

Changed July 03, 2008 09:47PM UTC by shadedecho comment:11

As stated on my blog, in response to John's request, I will re-join the effort on the jquery-dev list to explore ways to make $.ajax more flexible in a broader way that works for all involved. I believe that will not only benefit flXHR, but it'll benefit lots of other projects and the open-source community as a whole.

Changed August 03, 2008 01:52PM UTC by joern comment:12

resolution: wontfix
status: closedreopened

#3213 could be a usecase for this refactoring (the patch proposed there isn't appropiate).

Changed August 03, 2008 04:31PM UTC by cmcnulty comment:13

I completely agree with Joern. After I submitted #3213, I did a bit more research and found this ticket, which I agree is a better solution. I also found #2167 and #2128 which I think are additional use cases for this ticket.

I'm glad to see this has been updated from willnotfix!

-Charles

Changed August 04, 2008 02:19PM UTC by flesler comment:14

need: Test CaseCommit
resolution: → fixed
status: reopenedclosed

Applied at [5805].

Changed August 05, 2008 02:06AM UTC by shadedecho comment:15

I'm really pleased to see this improvement patch. Thanks again, Ariel!

Changed August 06, 2008 10:23PM UTC by joern comment:16

resolution: fixed
status: closedreopened

The patch looks good. Here is the option description for the wiki:

{{APIOption|xhr|Function|Callback for creating the XMLHttpRequest object. Defaults to the ActiveXObject when available (IE), the XMLHttpRequest otherwise. Override to provide your own implementation for XMLHttpRequest or enhancements to the factory.<nowiki></nowiki>}}

Changed August 07, 2008 01:07PM UTC by flesler comment:17

Applied at [5807].

Changed August 07, 2008 01:08PM UTC by flesler comment:18

resolution: → fixed
status: reopenedclosed

Changed August 07, 2008 07:51PM UTC by flesler comment:19

Here's the manager plugin:

http://plugins.jquery.com/project/XHR