Ajax: Integrated native datatype, for XHR2 rich responseType support #1525

Closed
wants to merge 5 commits into
from

Conversation

Projects
None yet
7 participants
@tarikbenmerar

XHR2 provides a response attribute, that contains a converted response to a native object, depending on the responseType ("arraybuffer", "blob", "document", "json"). See:
https://dvcs.w3.org/hg/xhr/raw-file/tip/Overview.html#the-response-attribute
http://www.html5rocks.com/en/tutorials/file/xhr2/?redirect_from_locale=fr

I propose in this pull request, to create a native datatype and response, to manage this kind of response. Some of previous closed tickets proposed to include a binary dataType, with adhoc changes on the transport function :
http://bugs.jquery.com/ticket/11461
http://bugs.jquery.com/ticket/9992
http://bugs.jquery.com/ticket/14068

I think it is not appropriate as the response attribute deals with more than binary data, like arraybuffer, but extends to blob, document and json datatype. In my pull request, I create a native datatype to support all the four response types that are abstracted.

I think also that such improvements should be included as this is a new improvement in XHR, that is not already included in jQuery. Also, trying to use a converter in the case of ArrayBuffer for example, leads to a heavy loop for the conversion (See: http://updates.html5rocks.com/2012/06/How-to-convert-ArrayBuffer-to-and-from-String). Simple casting doesn't function.

In my case, I have used Brain dMRI NifTI data formats that are 20MB in size, and all of them need parsing after downloading, which is also a heavy computation (a hundred of milliseconds).

Also, treating a binary data as text, it is just not natural at all, as this is already the case.

In any way, I think we should find a solution, and this is my proposition. Please, take a look at the tests, for the usage. And let's start a discussion around this.

@jaubourg

This comment has been minimized.

Show comment
Hide comment
@jaubourg

jaubourg Feb 22, 2014

Member

I like what I'm seeing here. Be careful about missing leading space at the beginning of some of your comments.

I'll let others chime in.

FYI, the forthcoming $.xhr uses xhr.response and only use xhr.reponseText if the former is not available.

Member

jaubourg commented Feb 22, 2014

I like what I'm seeing here. Be careful about missing leading space at the beginning of some of your comments.

I'll let others chime in.

FYI, the forthcoming $.xhr uses xhr.response and only use xhr.reponseText if the former is not available.

@tarikbenmerar

This comment has been minimized.

Show comment
Hide comment
@tarikbenmerar

tarikbenmerar Feb 22, 2014

@jaubourg thank you for replying. I have added the leading spaces to the comments.

Regarding the forthcoming $.xhr. Please, can you give me more information (A link for example) ?

@jaubourg thank you for replying. I have added the leading spaces to the comments.

Regarding the forthcoming $.xhr. Please, can you give me more information (A link for example) ?

@jaubourg

This comment has been minimized.

Show comment
Hide comment
@jaubourg

jaubourg Feb 22, 2014

Member

Here is the bug report: http://bugs.jquery.com/ticket/14509

It's early stage for now, I'm working on it and nothing's published yet.

Member

jaubourg commented Feb 22, 2014

Here is the bug report: http://bugs.jquery.com/ticket/14509

It's early stage for now, I'm working on it and nothing's published yet.

src/ajax/xhr.js
+ // Accessing binary-data responseText throws an exception
+ // (#11426)
+ if ( (!xhr.responseType || xhr.responseType === "text") &&
+ typeof xhr.responseText === "string" ) {

This comment has been minimized.

@scottgonzalez

scottgonzalez Feb 22, 2014

Member

This line gets an extra level of indentation to differentiate from the body.

@scottgonzalez

scottgonzalez Feb 22, 2014

Member

This line gets an extra level of indentation to differentiate from the body.

src/ajax/xhr.js
@@ -89,15 +90,25 @@ jQuery.ajaxTransport(function( options ) {
xhr.statusText
);
} else {
+ // Verify the responseType attribute to avoid InvalidStateError Exception (XHR2 Spec)
+ // Support : IE9

This comment has been minimized.

@scottgonzalez

scottgonzalez Feb 22, 2014

Member

No space before the colon.

@scottgonzalez

scottgonzalez Feb 22, 2014

Member

No space before the colon.

@tarikbenmerar

This comment has been minimized.

Show comment
Hide comment
@tarikbenmerar

tarikbenmerar Feb 22, 2014

@scottgonzalez I have included your remarks. Please, take a look if everything is well.

@scottgonzalez I have included your remarks. Please, take a look if everything is well.

@tarikbenmerar

This comment has been minimized.

Show comment
Hide comment
@tarikbenmerar

tarikbenmerar Feb 22, 2014

@jaubourg, understood. I asked because I had a little issue with the responseText attribute. It generates an InvalidStateError, if the responseType is not "text" (See : https://dvcs.w3.org/hg/xhr/raw-file/tip/Overview.html#the-responsetext-attribute). So, in the code, I added a condition for that.

Also, the support of the responseType is not complete in some browsers, partcularly the one I have used. The responseType functionned well with arraybuffer in my browser, but it didn't recognize the document, json or blob. It returned undefined for the response. So, if you choose responseText, you would generate an exception.

@jaubourg, understood. I asked because I had a little issue with the responseText attribute. It generates an InvalidStateError, if the responseType is not "text" (See : https://dvcs.w3.org/hg/xhr/raw-file/tip/Overview.html#the-responsetext-attribute). So, in the code, I added a condition for that.

Also, the support of the responseType is not complete in some browsers, partcularly the one I have used. The responseType functionned well with arraybuffer in my browser, but it didn't recognize the document, json or blob. It returned undefined for the response. So, if you choose responseText, you would generate an exception.

@tarikbenmerar

This comment has been minimized.

Show comment
Hide comment
@tarikbenmerar

tarikbenmerar Feb 22, 2014

@jaubourg, I should add for the ticket you provided. I am more for a solution, that would abstract the XHR and provides a rich means of communication (Through websockets for example).

From what I have read on the ticket, there is a will to remove the abstraction, as it is making some harms to the developers, who cannot do everything they want, due to that abstractions (The case of this pull request). And that abstraction is not adding value, as we are dealing, with a single communication means, XHR.

I have seen in the jquery ajax code, that there is a notion of transports, which has the role of the communication means abstraction. But it is only used for XHR.

Do you think, that there is a way to go in that path (Abstracting the communication means) ?

I have seen in libraries such as Ember-data, such notion of transportation mechanisms through the notion of adapters.

@jaubourg, I should add for the ticket you provided. I am more for a solution, that would abstract the XHR and provides a rich means of communication (Through websockets for example).

From what I have read on the ticket, there is a will to remove the abstraction, as it is making some harms to the developers, who cannot do everything they want, due to that abstractions (The case of this pull request). And that abstraction is not adding value, as we are dealing, with a single communication means, XHR.

I have seen in the jquery ajax code, that there is a notion of transports, which has the role of the communication means abstraction. But it is only used for XHR.

Do you think, that there is a way to go in that path (Abstracting the communication means) ?

I have seen in libraries such as Ember-data, such notion of transportation mechanisms through the notion of adapters.

@jaubourg

This comment has been minimized.

Show comment
Hide comment
@jaubourg

jaubourg Feb 22, 2014

Member

There are actually two other transports in jQuery core. I know of 3 or 4 others out there.

I'm moving forward the decision that was taken by the team to remove the abstraction because it was felt the features provided didn't justify the size. I don't agree with this assessment. I'm actually convinced a simplistic solution is doomed to slowly evolve into a new $.ajax with the exact same drawbacks in the end (because $.ajax already was a simplistic, "un-designed", solution at the start) but I will respect the decision of the majority.

I'm not convinced current abstraction is good enough anyway. We've had some bad surprises with standards as of late (especially the behavior of script tag injection being changed) that contradicted some design decision we took and made an already messy situation with our ajax options even messier.

Hope that makes things a little clearer.

Member

jaubourg commented Feb 22, 2014

There are actually two other transports in jQuery core. I know of 3 or 4 others out there.

I'm moving forward the decision that was taken by the team to remove the abstraction because it was felt the features provided didn't justify the size. I don't agree with this assessment. I'm actually convinced a simplistic solution is doomed to slowly evolve into a new $.ajax with the exact same drawbacks in the end (because $.ajax already was a simplistic, "un-designed", solution at the start) but I will respect the decision of the majority.

I'm not convinced current abstraction is good enough anyway. We've had some bad surprises with standards as of late (especially the behavior of script tag injection being changed) that contradicted some design decision we took and made an already messy situation with our ajax options even messier.

Hope that makes things a little clearer.

@tarikbenmerar

This comment has been minimized.

Show comment
Hide comment
@tarikbenmerar

tarikbenmerar Feb 22, 2014

@jaubourg. Thanks, you have clarified a lot of things. Please, can you send me if it is possible a link to the jQuery transports, and the others ?

Thanks a gain. And sorry again for my questions.

@jaubourg. Thanks, you have clarified a lot of things. Please, can you send me if it is possible a link to the jQuery transports, and the others ?

Thanks a gain. And sorry again for my questions.

@jaubourg

This comment has been minimized.

Show comment
Hide comment
@jaubourg

jaubourg Feb 22, 2014

Member

Only one other in core: https://github.com/jquery/jquery/blob/master/src/ajax/script.js#L32 (forgot jsonp was just a prefilter).

I personally coded:

There's also an iframe-based transport: https://github.com/cmlenz/jquery-iframe-transport/blob/master/jquery.iframe-transport.js#L92 and I can't find it right now but there was a transport-based mock ajax solution somewhere.

There are several alternative xdr transports like https://github.com/MoonScript/jQuery-ajaxTransport-XDomainRequest/blob/master/jQuery.XDomainRequest.js#L1 and https://github.com/dkastner/jquery.iecors/blob/master/jquery.iecors.js#L1

Member

jaubourg commented Feb 22, 2014

Only one other in core: https://github.com/jquery/jquery/blob/master/src/ajax/script.js#L32 (forgot jsonp was just a prefilter).

I personally coded:

There's also an iframe-based transport: https://github.com/cmlenz/jquery-iframe-transport/blob/master/jquery.iframe-transport.js#L92 and I can't find it right now but there was a transport-based mock ajax solution somewhere.

There are several alternative xdr transports like https://github.com/MoonScript/jQuery-ajaxTransport-XDomainRequest/blob/master/jQuery.XDomainRequest.js#L1 and https://github.com/dkastner/jquery.iecors/blob/master/jquery.iecors.js#L1

@tarikbenmerar

This comment has been minimized.

Show comment
Hide comment
@tarikbenmerar

tarikbenmerar Feb 22, 2014

Thanks, thanks a lot. It was just a rich discussion with you @jaubourg. I have learned a lot of things :).

Thanks, thanks a lot. It was just a rich discussion with you @jaubourg. I have learned a lot of things :).

@jaubourg

This comment has been minimized.

Show comment
Hide comment
@jaubourg

jaubourg Feb 22, 2014

Member

De rien, mon ami. Bon week-end ;)

Member

jaubourg commented Feb 22, 2014

De rien, mon ami. Bon week-end ;)

@tarikbenmerar

This comment has been minimized.

Show comment
Hide comment
@tarikbenmerar

tarikbenmerar Feb 23, 2014

Merci, merci :). A vous aussi.

Merci, merci :). A vous aussi.

test/unit/ajax.js
+ strictEqual(data, "0123\n", "Check for response content");
+ }
+ }]);
+

This comment has been minimized.

@scottgonzalez

scottgonzalez Feb 24, 2014

Member

This block has missing spaces inside parens and brackets all over the place.

@scottgonzalez

scottgonzalez Feb 24, 2014

Member

This block has missing spaces inside parens and brackets all over the place.

@tarikbenmerar

This comment has been minimized.

Show comment
Hide comment
@tarikbenmerar

tarikbenmerar Feb 24, 2014

@scottgonzalez I have added the missing spaces. Is everything correct now ?

@scottgonzalez I have added the missing spaces. Is everything correct now ?

test/unit/ajax.js
+ deepEqual( [array[0], array[1], array[2], array[3]], [0, 1 ,2 , 3], "Check for response content" );
+ }
+ }, {
+ url: url("data/native.txt"),

This comment has been minimized.

@scottgonzalez

scottgonzalez Feb 24, 2014

Member

missing spaces

@scottgonzalez

scottgonzalez Feb 24, 2014

Member

missing spaces

test/unit/ajax.js
+ ok( data instanceof ArrayBuffer, "The data retrieved is an ArrayBuffer instance" );
+ var array = new Uint8Array( data );
+ strictEqual( array.length, 4, "Check for response length" );
+ deepEqual( [array[0], array[1], array[2], array[3]], [0, 1 ,2 , 3], "Check for response content" );

This comment has been minimized.

@scottgonzalez

scottgonzalez Feb 24, 2014

Member

missing spaces inside brackets

@scottgonzalez

scottgonzalez Feb 24, 2014

Member

missing spaces inside brackets

This comment has been minimized.

@scottgonzalez

scottgonzalez Feb 24, 2014

Member

incorrect spacing before commas

@scottgonzalez

scottgonzalez Feb 24, 2014

Member

incorrect spacing before commas

test/unit/ajax.js
@@ -1999,6 +1999,29 @@ module( "ajax", {
).always( start );
});
+ ajaxTest( "jQuery.ajax() - Native datatype", 8, [{
+ url: url("data/native.bin"),

This comment has been minimized.

@scottgonzalez

scottgonzalez Feb 24, 2014

Member

missing spaces

@scottgonzalez

scottgonzalez Feb 24, 2014

Member

missing spaces

@tarikbenmerar

This comment has been minimized.

Show comment
Hide comment
@tarikbenmerar

tarikbenmerar Feb 24, 2014

@scottgonzalez please take a look at the last changes, and confirm.

@scottgonzalez please take a look at the last changes, and confirm.

@scottgonzalez

This comment has been minimized.

Show comment
Hide comment
@scottgonzalez

scottgonzalez Feb 24, 2014

Member

Looks good.

Member

scottgonzalez commented Feb 24, 2014

Looks good.

@tarikbenmerar

This comment has been minimized.

Show comment
Hide comment
@tarikbenmerar

tarikbenmerar Feb 24, 2014

Thanks @scottgonzalez :). @jaubourg @scottgonzalez I wanted to add a feature to this pull request, but I'd prefer to see it merged to add something else.

It's about providing the native responseXML, through this native datatype. I'll discuss it in detail when the current pull request is merged.

Thanks @scottgonzalez :). @jaubourg @scottgonzalez I wanted to add a feature to this pull request, but I'd prefer to see it merged to add something else.

It's about providing the native responseXML, through this native datatype. I'll discuss it in detail when the current pull request is merged.

@Mithgol Mithgol referenced this pull request in Stuk/jszip Feb 28, 2014

Closed

How to zip a pdf or ppt file using jszip? #109

@dmethvin dmethvin added this to the 1.12/2.2 milestone Mar 1, 2014

@dmethvin

This comment has been minimized.

Show comment
Hide comment
@dmethvin

dmethvin Mar 1, 2014

Member

I like this too. Since it's a feature I think it's appropriate for 1.12/2.2. We'll also need some documentation for this before it lands, who's up for that? You can do a pull request over at https://github.com/jquery/api.jquery.com/ .

To me the $.xhr discussion is separate, I think we'll want to support XHR2 there obviously but it's a different API.

Member

dmethvin commented Mar 1, 2014

I like this too. Since it's a feature I think it's appropriate for 1.12/2.2. We'll also need some documentation for this before it lands, who's up for that? You can do a pull request over at https://github.com/jquery/api.jquery.com/ .

To me the $.xhr discussion is separate, I think we'll want to support XHR2 there obviously but it's a different API.

@tarikbenmerar

This comment has been minimized.

Show comment
Hide comment
@tarikbenmerar

tarikbenmerar Mar 1, 2014

@dmethvin I am ready to provide any documentation needed. I need just some direction, and what is required to get this pull request merged. I am a little disturbed with the process to follow.

I want also to discuss an improvement to this idea. I wanted to add the responseXML to the native datatype, as the current xml responses are provided through the parsing of the textual response.

This was necessary as some bugs were observed with the native responseXML, but providing some freedom to the developer is also good. So, the native datatype would provide the response attribute if it's defined or the responseXML if it's defined.

What do you think ?

@dmethvin I am ready to provide any documentation needed. I need just some direction, and what is required to get this pull request merged. I am a little disturbed with the process to follow.

I want also to discuss an improvement to this idea. I wanted to add the responseXML to the native datatype, as the current xml responses are provided through the parsing of the textual response.

This was necessary as some bugs were observed with the native responseXML, but providing some freedom to the developer is also good. So, the native datatype would provide the response attribute if it's defined or the responseXML if it's defined.

What do you think ?

@dmethvin

This comment has been minimized.

Show comment
Hide comment
@dmethvin

dmethvin Mar 1, 2014

Member

I am not sure. My main concern with the documentation is about discoverability. If you go to https://api.jquery.com/jQuery.ajax/ where would this fit into the page, if at all? How will people know this exists?

Member

dmethvin commented Mar 1, 2014

I am not sure. My main concern with the documentation is about discoverability. If you go to https://api.jquery.com/jQuery.ajax/ where would this fit into the page, if at all? How will people know this exists?

@tarikbenmerar

This comment has been minimized.

Show comment
Hide comment
@tarikbenmerar

tarikbenmerar Mar 3, 2014

@dmethvin some carefull modifications are needed. First of all, there should be a description of the native datatype, and what kind of responses it provides.

Secondly, the intelligent guess depends not only on the MIME type, but also on the XHR responseType field (The native response is guessed when the responseType field is defined and doesn't equal to "text"). Lastly, an example or two are necessary and would clarify the usage of the data type.

I have cloned the jQuery API, and I will start writing and pull a request ASAP.

@dmethvin some carefull modifications are needed. First of all, there should be a description of the native datatype, and what kind of responses it provides.

Secondly, the intelligent guess depends not only on the MIME type, but also on the XHR responseType field (The native response is guessed when the responseType field is defined and doesn't equal to "text"). Lastly, an example or two are necessary and would clarify the usage of the data type.

I have cloned the jQuery API, and I will start writing and pull a request ASAP.

@tarikbenmerar tarikbenmerar referenced this pull request in jquery/api.jquery.com Mar 14, 2014

Closed

Integrate documentation for the ajax native datatype #463

@tarikbenmerar

This comment has been minimized.

Show comment
Hide comment
@tarikbenmerar

tarikbenmerar Mar 14, 2014

@dmethvin @jaubourg @scottgonzalez, please take a look at jquery/api.jquery.com#463 for the documentation, that I have began writing.

@dmethvin @jaubourg @scottgonzalez, please take a look at jquery/api.jquery.com#463 for the documentation, that I have began writing.

@ProgerXP

This comment has been minimized.

Show comment
Hide comment
@ProgerXP

ProgerXP Jul 9, 2014

Any update on this guys? Why is this feature doomed to ignorance since 2012? I would really love to see responseType implemented in near future (not next two years!).

ProgerXP commented Jul 9, 2014

Any update on this guys? Why is this feature doomed to ignorance since 2012? I would really love to see responseType implemented in near future (not next two years!).

@tarikbenmerar

This comment has been minimized.

Show comment
Hide comment
@tarikbenmerar

tarikbenmerar Jul 9, 2014

Hello @ProgerXP, I am the author of this feature. It is planned to be integrated in jquery 1.12 (See milestone on the right), that for now haven't a release date, but it should be for the following months. I have worked a little on the doc as it is necessary, but for now I didn't had feedbacks regarding that (See jquery/api.jquery.com#463).

I should resume working on the doc. Your feedbacks are wecome.

Hello @ProgerXP, I am the author of this feature. It is planned to be integrated in jquery 1.12 (See milestone on the right), that for now haven't a release date, but it should be for the following months. I have worked a little on the doc as it is necessary, but for now I didn't had feedbacks regarding that (See jquery/api.jquery.com#463).

I should resume working on the doc. Your feedbacks are wecome.

@tarikbenmerar

This comment has been minimized.

Show comment
Hide comment
@tarikbenmerar

tarikbenmerar Jul 9, 2014

@dmethvin, can you guide me on this ? I have started working on the doc, but didn't have any feedback. What should I do ?

@dmethvin, can you guide me on this ? I have started working on the doc, but didn't have any feedback. What should I do ?

@dmethvin

This comment has been minimized.

Show comment
Hide comment
@dmethvin

dmethvin Jul 9, 2014

Member

@tarikbenmerar We've been brainstorming in a gist here and a Google Doc here, please feel free to read and comment with your thoughts. It seems like this feature could be done pretty simply with an incoming option for responseType, assuming the design doesn't process the response by default (which it doesn't at the moment).

Member

dmethvin commented Jul 9, 2014

@tarikbenmerar We've been brainstorming in a gist here and a Google Doc here, please feel free to read and comment with your thoughts. It seems like this feature could be done pretty simply with an incoming option for responseType, assuming the design doesn't process the response by default (which it doesn't at the moment).

@dduponchel dduponchel referenced this pull request in Stuk/jszip Jul 14, 2014

Closed

Can't generate zip file of images #151

@tarikbenmerar

This comment has been minimized.

Show comment
Hide comment
@tarikbenmerar

tarikbenmerar Oct 11, 2014

Hello @dmethvin, sorry I didn't have time to focus on this topic. So, I am asking myself if jQuery ajax will exist in the next version with $.xhr as the underlying transport mechanism or not. I am really interrested that this feature will exist. At least, to have my code to work (and not rely on a custom code I am using). And I think, there will be people interrested in it as they are comfortable with ajax mechanism for now, untill there is a transition $.xhr. Please, should I invest on the documentation ?

Hello @dmethvin, sorry I didn't have time to focus on this topic. So, I am asking myself if jQuery ajax will exist in the next version with $.xhr as the underlying transport mechanism or not. I am really interrested that this feature will exist. At least, to have my code to work (and not rely on a custom code I am using). And I think, there will be people interrested in it as they are comfortable with ajax mechanism for now, untill there is a transition $.xhr. Please, should I invest on the documentation ?

@dmethvin

This comment has been minimized.

Show comment
Hide comment
@dmethvin

dmethvin Oct 11, 2014

Member

Yes, $.ajax() will be there and won't go away for years, if ever. We're just looking for a simpler interface that will support more advanced features, and that is $.xhr().

Member

dmethvin commented Oct 11, 2014

Yes, $.ajax() will be there and won't go away for years, if ever. We're just looking for a simpler interface that will support more advanced features, and that is $.xhr().

@tarikbenmerar

This comment has been minimized.

Show comment
Hide comment
@tarikbenmerar

tarikbenmerar Oct 21, 2014

Hello @dmethvin. OK thanks, so should I work on the documentation of this feature ? I available ready to provide the remaining documentation for the feature to be ready for the next jquery version. I am asking myself when the next jquery version will be available, could you tell me please ?

Also, I have left some feedbacks on your $.xhr GIST, perhaps you didn't receive a notification.

Thanks in advance.

Hello @dmethvin. OK thanks, so should I work on the documentation of this feature ? I available ready to provide the remaining documentation for the feature to be ready for the next jquery version. I am asking myself when the next jquery version will be available, could you tell me please ?

Also, I have left some feedbacks on your $.xhr GIST, perhaps you didn't receive a notification.

Thanks in advance.

@dmethvin

This comment has been minimized.

Show comment
Hide comment
@dmethvin

dmethvin Oct 21, 2014

Member

@tarikbenmerar Unfortunately, Github doesn't seem to give notices on gists. Drives me crazy!

We'd want to pick these changes into the 1.x-master branch as well. I see you've built in IE9 compat, any thoughts about how we'd treat this from a code/docs standpoint there?

Member

dmethvin commented Oct 21, 2014

@tarikbenmerar Unfortunately, Github doesn't seem to give notices on gists. Drives me crazy!

We'd want to pick these changes into the 1.x-master branch as well. I see you've built in IE9 compat, any thoughts about how we'd treat this from a code/docs standpoint there?

@Mithgol

This comment has been minimized.

Show comment
Hide comment
@Mithgol

Mithgol Oct 21, 2014

Offtopic comment: you might want to subscribe to the issue isaacs/github#21 where the (absent) gist notifications were discussed.

Mithgol commented Oct 21, 2014

Offtopic comment: you might want to subscribe to the issue isaacs/github#21 where the (absent) gist notifications were discussed.

@tarikbenmerar

This comment has been minimized.

Show comment
Hide comment
@tarikbenmerar

tarikbenmerar Oct 24, 2014

Hello @dmethvin, I have analysed the differences in code between 1.x-master and master, it shouldn't be difficult to integrate this feature. So, I will create another pull request specifically for the 1.x-master.

This should be the same for the documentation.

For the gist, I have seen that for the third remark this has been taken by other contributors, and I am for those approaches. Please, consider the option of using lightweight thenables that can be coarsed into promises of any library. Tell me if I can provide an example.

And for default settings, as you're dealing with xhr objects, you can easily create a factory, for generating xhrs with specific defaults.

Hello @dmethvin, I have analysed the differences in code between 1.x-master and master, it shouldn't be difficult to integrate this feature. So, I will create another pull request specifically for the 1.x-master.

This should be the same for the documentation.

For the gist, I have seen that for the third remark this has been taken by other contributors, and I am for those approaches. Please, consider the option of using lightweight thenables that can be coarsed into promises of any library. Tell me if I can provide an example.

And for default settings, as you're dealing with xhr objects, you can easily create a factory, for generating xhrs with specific defaults.

liliakai added a commit to signalapp/Signal-Desktop that referenced this pull request Oct 30, 2014

Use jquery with native transport support
The jquery mainline is lagging on support for responseType:
'arraybuffer', an XHRHTTPRequest Level 2 feature (whatever that means).
We use this responseType to transfer attachments.

I rebased jquery/jquery#1525 on the 2.1.1
release and cut 2.1.1-ajax-nativetransport in my own fork.
@dmethvin

This comment has been minimized.

Show comment
Hide comment
@dmethvin

dmethvin Dec 4, 2014

Member

I'm closing this for reasons similar to gh-1541 in that it adds complexity to $.ajax for one transport and requires additional documentation to an API call that is already the largest surface area. It would be great to see a fetch-based plugin that we could use as a simple replacement for these applications and eventually integrate into jQuery itself.

Member

dmethvin commented Dec 4, 2014

I'm closing this for reasons similar to gh-1541 in that it adds complexity to $.ajax for one transport and requires additional documentation to an API call that is already the largest surface area. It would be great to see a fetch-based plugin that we could use as a simple replacement for these applications and eventually integrate into jQuery itself.

@dmethvin dmethvin closed this Dec 4, 2014

@tarikbenmerar

This comment has been minimized.

Show comment
Hide comment
@tarikbenmerar

tarikbenmerar Dec 4, 2014

@dmethvin OK Dave. I should just say that it would be impossible to integrate it into a third party plugin, because there needs change in the code (AJAX code currently interprets only textual data). If you want to provide the required feature in the plugin, you should manually copy paste from a string to an array buffer, a significant overhead (That's whay XHR2 is here) when you have 20 mega bytes, as in fact I do.

I am still thinking that it is a needed feature (signalapp/Signal-Desktop@e156c2c). People are hardcoding AJAX requests to benefit from this basic feature.

In any case, ..... I know I cannot influence this open source software. I'd wished I'd continue to integrate comfortably the merged feature in my PhD research code.

Thanks,

@dmethvin OK Dave. I should just say that it would be impossible to integrate it into a third party plugin, because there needs change in the code (AJAX code currently interprets only textual data). If you want to provide the required feature in the plugin, you should manually copy paste from a string to an array buffer, a significant overhead (That's whay XHR2 is here) when you have 20 mega bytes, as in fact I do.

I am still thinking that it is a needed feature (signalapp/Signal-Desktop@e156c2c). People are hardcoding AJAX requests to benefit from this basic feature.

In any case, ..... I know I cannot influence this open source software. I'd wished I'd continue to integrate comfortably the merged feature in my PhD research code.

Thanks,

@dmethvin

This comment has been minimized.

Show comment
Hide comment
@dmethvin

dmethvin Dec 4, 2014

Member

@tarikbenmerar My comment in gh-1541 is proposing a new API, not additional changes to $.ajax. Documenting the additions is at least as difficult as the code, and that work has yet to be done. A simpler XHR-only (or fetch-only) API would be much easier to document.

Member

dmethvin commented Dec 4, 2014

@tarikbenmerar My comment in gh-1541 is proposing a new API, not additional changes to $.ajax. Documenting the additions is at least as difficult as the code, and that work has yet to be done. A simpler XHR-only (or fetch-only) API would be much easier to document.

@tarikbenmerar

This comment has been minimized.

Show comment
Hide comment
@tarikbenmerar

tarikbenmerar Dec 4, 2014

@dmethvin sorry, I didn't know that such standard already exists (Fetch standard). I have taken a look at the W3C documentation, it has support for Array Buffer in a simple way. Now, if you mean simpler API, I guess it is $.xhr. Isn't it ?

In this case, is there any additional thing I can contribute ? Just to know, if my responsibility ends here or if I can contribute something.

@dmethvin sorry, I didn't know that such standard already exists (Fetch standard). I have taken a look at the W3C documentation, it has support for Array Buffer in a simple way. Now, if you mean simpler API, I guess it is $.xhr. Isn't it ?

In this case, is there any additional thing I can contribute ? Just to know, if my responsibility ends here or if I can contribute something.

@dmethvin

This comment has been minimized.

Show comment
Hide comment
@dmethvin

dmethvin Dec 4, 2014

Member

We did some brainstorming but decided it was better to allow the solution to proceed as a plugin at its own pace, rather than being tied to our release schedule. There is some discussion in various places (here, here and here and here) but we haven't finalized the design and nobody is implementing it at the moment.

Member

dmethvin commented Dec 4, 2014

We did some brainstorming but decided it was better to allow the solution to proceed as a plugin at its own pace, rather than being tied to our release schedule. There is some discussion in various places (here, here and here and here) but we haven't finalized the design and nobody is implementing it at the moment.

@tarikbenmerar

This comment has been minimized.

Show comment
Hide comment
@tarikbenmerar

tarikbenmerar Dec 4, 2014

@dmethvin OK, I will take a look at those links (I have seen some of them). If I am seeing it well, I should create a plugin for this solution, it will be another transport that manages native dataType, isn't it (with some additional converters and responseFields of course). ?

@dmethvin OK, I will take a look at those links (I have seen some of them). If I am seeing it well, I should create a plugin for this solution, it will be another transport that manages native dataType, isn't it (with some additional converters and responseFields of course). ?

@dmethvin

This comment has been minimized.

Show comment
Hide comment
@dmethvin

dmethvin Dec 5, 2014

Member

I would think it starts as a standalone plugin based on the fetch polyfill, or perhaps on XHR2. There will be more bikeshedding on the API than actual code in the file I think.

Member

dmethvin commented Dec 5, 2014

I would think it starts as a standalone plugin based on the fetch polyfill, or perhaps on XHR2. There will be more bikeshedding on the API than actual code in the file I think.

@tarikbenmerar

This comment has been minimized.

Show comment
Hide comment
@tarikbenmerar

tarikbenmerar Feb 3, 2015

@dmethvin had some free time and worked on a plugin https://github.com/acigna/jquery-ajax-native. The source code is here : https://github.com/acigna/jquery-ajax-native/blob/master/src/jquery-ajax-native.js . I had to copy past a large part of the xhr ajaxTransport as there were no alternative. I hope there is a better way to add this support in the future.

@dmethvin had some free time and worked on a plugin https://github.com/acigna/jquery-ajax-native. The source code is here : https://github.com/acigna/jquery-ajax-native/blob/master/src/jquery-ajax-native.js . I had to copy past a large part of the xhr ajaxTransport as there were no alternative. I hope there is a better way to add this support in the future.

@tarikbenmerar

This comment has been minimized.

Show comment
Hide comment
@tarikbenmerar

tarikbenmerar Feb 5, 2015

@ProgerXP here is for the plugin : https://github.com/acigna/jquery-ajax-native, if you want to use the feature now.

@ProgerXP here is for the plugin : https://github.com/acigna/jquery-ajax-native, if you want to use the feature now.

@mgol mgol removed this from the 3.0.0 milestone Oct 17, 2015

@dalbani dalbani referenced this pull request in swagger-api/swagger-ui Oct 19, 2015

Closed

Work around issues with content type sniffing mechanism #1663

@tarikbenmerar tarikbenmerar referenced this pull request in acigna/jquery-ajax-native Oct 20, 2017

Merged

Add `$.getBlob` wrapper #2

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