Bug Tracker

Opened 6 years ago

Closed 5 years ago

#14509 closed feature (migrated)

Create $.xhr

Reported by: jzaefferer Owned by: jaubourg
Priority: high Milestone: 1.12/2.2
Component: ajax Version: 1.10.2
Keywords: Cc:
Blocked by: Blocking:

Description

From the Amsterdam team meeting:

Create $.xhr which is based on $.ajax but without any of the legacy options (async:false) or things specific for non-xhr (jsonp/script)

Options to keep:

  • beforeSend (expose the native xhr so we can cover the removed xhrFields option)*
  • cache (possibly rename to something like noCache)*
  • data (if not a string: will use default $.param and append to url for GET requests, otherwise * send as request body, as today)
  • headers
  • method (aka ‘type’)
  • username
  • password
  • url
  • timeout

Options to remove:

  • accept (use headers)
  • async (use a different method for sync requests)
  • contents (do custom parsing yourself)
  • contentType (use headers)
  • context (use $.proxy)
  • converters (global hash of convertors or do it yourself)
  • crossDomain (non-xhr)
  • dataFilter (do it yourself)
  • dataType (jq will use response’s content-type)
  • global (everything should be consistent)
  • ifModified (make it a plugin)
  • isLocal (expose localProtocols as a global config array instead)
  • jsonp (non-xhr)
  • jsonpCallback (non-xhr)
  • processData (who knows? no one is sending XML documents anymore, right?)
  • scriptCharset (non-xhr)
  • statusCode (handle the responses yourself)
  • mimeType (don’t override stuff)
  • scriptCharset (non-xhr)
  • success/error/complete (legacy)
  • traditional (convert the data option to a string with $.param(.., true) yourself)
  • xhr
  • xhrFields (use beforeSend instead)

As the first step, this would delegate to $.ajax. Eventually it should be possible to build jQuery with just this interface, without any of the other methods like $.get, $.getJSON or even $.ajax.

Change History (15)

comment:1 Changed 6 years ago by timmywil

Component: unfiledajax
Priority: undecidedhigh
Status: newopen

comment:2 Changed 6 years ago by Rick Waldron

*applause*

comment:3 Changed 6 years ago by jaubourg

Owner: set to jaubourg
Status: openassigned

comment:4 Changed 6 years ago by markelog

I would suggest to present this method as a shorthand method like jQuery.get|post|getScript. And therefore do not rename "cache" or "type" options but change signature of "beforeSend" for jQuery.ajax too.

"expose localProtocols as a global config array instead" and "eventually it should be possible to build jQuery with just this interface" should have different tickets

comment:5 in reply to:  4 ; Changed 6 years ago by scottgonzalez

Replying to markelog:

I would suggest to present this method as a shorthand method like jQuery.get|post|getScript. And therefore do not rename "cache" or "type" options but change signature of "beforeSend" for jQuery.ajax too.

But it's not a shorthand method. It's a complete replacement of jQuery.ajax() and having lots of parameters is a bad API.

"expose localProtocols as a global config array instead" and "eventually it should be possible to build jQuery with just this interface" should have different tickets

Except that it's not possible to build this as it's intended without those. The goal is to leave jQuery.ajax() as-is and create a new, leaner, simpler API.

comment:6 in reply to:  5 ; Changed 6 years ago by markelog

It's a complete replacement of jQuery.ajax()

It could not be a "complete replacement" of jQuery.ajax since it does not presented with same functionality, like you couldn't use jsonp or script transports and it does not give any alternatives for them (and it shouldn't given method's name), you couldn't use $.getJSON as a shorthand method for jQuery.xhr since you have to do the conversion yourself and so on – list could continue.

having lots of parameters is a bad API.

No arguments there.

Except that it's not possible to build this as it's intended without those

Not quite, localProtocols as a global config, could be implemented first and jQuery.ajax could start using it.

As for build option – you obviously need to implement this method first, which is this task purpose and only then you could make it a build option, hence word "eventually" in the quote.

The goal is to leave jQuery.ajax() as-is and create a new, leaner, simpler API

You could achieve that if jQuery.xhr would be presented as shorthand and jQuery.ajax would be considered a last resort method. I fear if we choose the other route it could lead to source of confusion.

comment:7 in reply to:  6 ; Changed 6 years ago by scottgonzalez

Replying to markelog:

It could not be a "complete replacement" of jQuery.ajax since it does not presented with same functionality

This is just a misunderstanding of the wording. It's not a replacement for the complete functionality of jQuery.ajax(). It's a complete replacement in the sense that jQuery.ajax() is removed and jQuery.xhr() is added. One of the main goals is to completely get rid of jQuery.ajax(), which is why jQuery.xhr() can't be a shorthand method.

Not quite, localProtocols as a global config, could be implemented first and jQuery.ajax could start using it.

I suppose, but I don't think there are any plans to change the APIs around jQuery.ajax(). The proposed path is not to touch jQuery.ajax() at all.

The goal is to leave jQuery.ajax() as-is and create a new, leaner, simpler API

You could achieve that if jQuery.xhr would be presented as shorthand and jQuery.ajax would be considered a last resort method. I fear if we choose the other route it could lead to source of confusion.

I'm not sure what the source of confusion would be, but I'm happy to talk through any potential confusion. If you're using jQuery.xhr(), you shouldn't be using jQuery.ajax(). The docs would make this clear.

comment:8 in reply to:  7 Changed 6 years ago by markelog

completely get rid of jQuery.ajax()

Oh my, even if jQuery.xhr could have same functionality that jQuery.ajax has, but with better API, even then, it would be... bold i guess, even just to deprecate jQuery.ajax.

The proposed path is not to touch jQuery.ajax() at all.

Designation of what localProtocols are, made inside ajax module, which is not a public API, consequance of jQuery.xhr introduction is, as it said in ticket description, to make it a public property.

How to expose it (jQuery.ajaxSettings? What format?) is a question that i would recommend to dealt before jQuery.xhr implementation, because it would affect jQuery.ajax.

Just like convertors option that could be passed to jQuery.ajax and is defined in jQuery.ajaxSettings

comment:9 Changed 6 years ago by scottgonzalez

The exposure of localProtocols doesn't have to affect jQuery.ajax(), but it can, if we want it to.

I would expect that jQuery.ajaxSettings wouldn't be used at all in jQuery.xhr(). Changing the actual default values for Ajax requests seems really bad to me.

comment:10 Changed 6 years ago by dmethvin

There should definitely not be a way to change the default settings, since that has the potential to create havoc in a modular app just like jQuery.ajaxSettings does.

It would also be good to clearly document for each method whether the data is sent on the URL or alternatively in the content body. There is some ambiguity about whether methods like DELETE can have a body or not as I recall, there's a ticket around here somewhere.

comment:11 Changed 6 years ago by jzaefferer

The ticket description outlines the options to keep, though we should consider adjusting some of those as well. Angualar's $http service seems like a good reference. Specifically, they avoid the confusion of data by having a separate params option: params get appended as query string, data as message body, without any ambiguity.

Another aspect to consider is the response object. Angular puts 'data', 'status', 'headers' and 'config' properties on a single response object, in order to fit it into a promise.

comment:12 Changed 6 years ago by dmethvin

Yehuda mentioned this code, which (among other things) adapts $.ajax to an rsvp.js Promise which is Promises/A+ compliant:

https://github.com/instructure/ic-ajax/blob/master/main.js

comment:13 Changed 6 years ago by dmethvin

Milestone: None1.12/2.2

comment:14 Changed 6 years ago by dmethvin

#15053 is a duplicate of this ticket.

comment:15 Changed 5 years ago by m_gol

Resolution: migrated
Status: assignedclosed
Note: See TracTickets for help on using tickets.