Ember.ajax

[marcioj]

I did talk with @wycats, and I had an ok to implement this.

If this PR is usefull, I still have some doubts, like:

• Is the package jquery-extensions apropriated ?
• The ajax request isn't just a RSVP.Deferred, is actually a Ember.Object with Ember.Evented and Ember.DeferredMixin, so in some moment the destroy must be called?

Thanks in advance.

[stefanpenner]

missing .

[stefanpenner]

I've made some comments.

I am excited to a built-in ajax with run-loops included. There are some unfortunate annoyances with jQuery.ajax that we will likely want to iron over, but this is a good starting place.

a sibling Ember.getJSON using this same pattern would also be good.

[marcioj]

Thanks for the review @stefanpenner, I will fix these issues.

[krisselden]

I have an issue with dropping acess to headers and status on resolve

[krisselden]

@wycats see above comment. Jquery API does not map onto the single arg resolution. Maybe we could have a lower level API that resolved with the jqxhr object?

[stefanpenner]

@kselden you know i am +1 for this.

[marcioj]

@kselden +1

Maybe an AjaxEvent with the properties:

{
  data: /* data returned from server */,
  textStatus: /* the text status of the performed request*/,
  errorThrown: /* In case of failure requests */,
  xhr: /*the jquery ajax object*/
}

And this same event will be present in on and then callbacks:

Ember.ajax('/posts').then(function(evt) {
  var posts = evt.data;
  // do something with posts
});

Ember.ajax('/posts').on(500, function(evt) {
  alert(evt.errorThrown);
});

I think that just put all in a single object is ok, since this is a low level api to make ajax, and all info needed is important.

@stefanpenner I will make the max effort to finish this tonight, because this is important to 1.0 IMHO.
In the last night, I have some troubles to test the Ember.assert inside of the RSVP.Deferred, because the assert is overrided in the tests, so if you have some tip or sample in the ember tests, could be usefull to me.

[krisselden]

You can have a high level one, like getJSON() that drops the headers and resolves just with the JSON. but it is important to balance jQuery compatibility with flexibility. There are APIs that put info you need (like rate limit remaining) in the headers and it is not an ok solution to me just to drop it because 3 args doesn't map onto RSVP's one arg.

[marcioj]

@kselden I intend to make a getJSON when finish the ajax. Don't know if this belongs to same PR ...

Mostly of jquery ajax options can be setup in Ember.ajax(), so with the beforeSend is possible to set the headers, and in the fulfilled callback, its possible to retrieve the response using the jquery xhr object:

Ember.ajax('/posts', {
  beforeSend: function (request) {
    request.setRequestHeader("Authority", authorizationToken);
  }
}).then(function(evt) {
  evt.xhr.getResponseHeader('some header');
});

[stefanpenner]

we likely want to assert if no URL is present.

[stefanpenner]

This is beginning to take great shape. Remember to squash you commits once your done.

[stefanpenner]

although not as 'easy' as jQuery's value this I believe is much simpler as it introduces a nice symmetry between fulfillment and rejection handlers. In addition it provides the normally "lost" meta data.

the downside today is that the following no longer works

model: function(){
   return Ember.ajax('/posts')
}

Rather a transformation must occure.

function getData(event) {
  return event.data;
}

model: function(){
   return Ember.ajax('/posts').then(getData);
}

Would it make sense for the "higher level" getJSON to fulfill with only the value, not the entire event? Or should we stick to the event + transform.

[krisselden]

I was under the impression that @wycats was concerned about leveraging the API people know. I think the higher level getJSON should resolve with a single value, if you need the headers or status, you can drop down to ajax

[marcioj]

Both are right. Ember.ajax isn't designed to pass the data directlly, instead it will retrieve all the info in a single object.

Ember.getJSON('/posts').then(function(evt) {
  // evt.xhr, evt.data etc
});

Without this, users will fallback in jQuery.ajax, because they could not do something with the Ember.ajax. E.g extract some header from xhr object.

But the next implementation, maybe Ember.getJSON, will be aware of json payload, so the following will work:

 model: function() {
   return Ember.getJSON('/posts');
 }

This will be the common case. Ember.ajax, is intended to rare ones.

I am just concerned about the Ember.getJSON, because this is just to retrieve json data. I think that we need something to send json. With the Ember.ajax will be possible, but the headers application/json will need to be setup all the time. So I am thinking in some Ember.sendJSON(url, [method], [options]):

// defaults to get
Ember.sendJSON('/posts').then(function(posts) {
  // posts => [ { id: 1, title: rails is omasake ... } ]
}).fail(function(evt) {
  // same like Ember.ajax. evt.xhr, evt.data etc
});

A post request

Ember.sendJSON('/posts', 'POST').then(function(post) {
  // post => { id: 1, title: rails is omasake ... }
}).fail(function(evt) {
  // same like Ember.ajax. evt.xhr, evt.data etc
});

Ember.getJSON(url) can be an alias to Ember.sendJSON(url, 'GET').

Thoughts?

[stefanpenner]

My current feeling is the evented aspects of this api should be held off until post 1.0.

The ajax + runloop + fulfilled and rejected event objects feel good, and solve the motivating issue.

The on(200 etc seem to be sugar on top, that I would feel comfortable entering ember post 1.0 as an experimental feature, this would allow us to iterate and feel out before it enters any final api.

(cc @wycats @kselden)

[marcioj]

Ember.sendJSON included, please give a look.

(cc @wycats @kselden @stefanpenner )

[stefanpenner]

im still unsure about the evented stuff, and even more unsure to use RSVP's internal event system.

[stefanpenner]

extra indentation

[marcioj]

@stefanpenner thanks for all your help in this PR. I will wait for your decisions.

[stefanpenner]

@marcioj bro you rocked this. Sorry for the massive amounts of suggestions and pestering.

[stefanpenner]

some discussion with @wycats and @kselden has lead to hopefully our last request:

sendJson -> getJson
and remove the evented stuff for now. Once we implement feature flag stuff, we can do a proper proposal/experiment cycle with it behind a flag.

We just want to be ultra-safe this weekend.

[stefanpenner]

I don't think this chains correctly. You want to return the downstream promise. not the AjaxRequest.

it is likely a good idea to ensure proper chaining semantics and basic error propgration still work correctly.

Ember.ajax().then(function(){
  throw new Error();
}).fail(reason){
   // should be the thrown error from the previous fulfillment handler.
})

[marcioj]

👍

[marcioj]

Thanks @markstory @stefanpenner

[wagenet]

See also #3046.

[wagenet]

This will have to be updated to follow these guidelines: http://emberjs.com/guides/contributing/adding-new-features/. We'll also have to make a decision regarding the overlap with #3046.

[stefanpenner]

@wagenet i'll champion this one (although i wont look into it till this weekend, so if someone else wants to drive it before then, they are welcome to)

[wagenet]

@stefanpenner Can you shut down #3046 if appropriate?

[wagenet]

@stefanpenner ping.

[marcioj]

@wagenet I think that I will close this in favor of #3046, the tchak version is better, but doesn't have tests. If you want I can add some tests. What do you think?

/cc @stefanpenner

[wagenet]

@marcioj Thanks for submitting this. I think you're right that it probably makes sense to just center around a single effort.