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

parse() no longer passes XHR object #1939

Closed
dmotz opened this issue Dec 14, 2012 · 8 comments
Closed

parse() no longer passes XHR object #1939

dmotz opened this issue Dec 14, 2012 · 8 comments

Comments

@dmotz
Copy link

dmotz commented Dec 14, 2012

First off, congrats and thanks for the bounty of great changes in 0.9.9.

One issue I'm encountering with a project previously based on 0.9.2 is that parse no longer passes the XHR object with the models.

The use case here is that the client side is fetching a paginated collection and the remote API returns the total length of the collection in the XHR's headers. The app can then display the total count to the user and lazily fetch extra pages.

Is there a better way to implement this behavior from the client side without relying on the XHR passed to parse()?

I wanted to clarify before reverting the behavior in Backbone.

@caseywebdev
Copy link
Collaborator

The removal of the xhr object was to promote the agnostic-ness of Backbone regarding server side communication. If you're using Socket.io, for example, that xhr object doesn't make any sense. What you should do is pass all the extraneous data you need in the response body, along with the collection data.

{
  "page": 5,
  "ofPages": 25,
  "models": [{},{},{}]
}

And then

parse: function (data) {
  // use data.page and data.ofPages as needed
  return data.models;
}

@gsamokovarov
Copy link
Contributor

You can make your response to carry this meta-data. For example a bulk response can look something like this:

{
  meta: {
    next: '/api/next/resources/list',
    prev: '/api/prev/resources/list',
    limit: 5,
    offset: 5,
    count: 5
  },
  objects: [{id: 6}, {id: 7}, {id: 8}, {id: 9}, {id: 10}]
}

@gsamokovarov
Copy link
Contributor

Nah, @caseywebdev beat me up to it :)

@wyuenho
Copy link
Contributor

wyuenho commented Dec 14, 2012

I've been thinking about this issue as well. There are many public APIs that support pagination by way of sending down headers, e.g. Github. Those aren't APIs that users of Backbone have control with so changing the server API is out of the question. Removing xhr will make old code that overrides parse no longer works. Is the answer now at of 0.9.9 that people should override sync also for parsing headers?

@caseywebdev
Copy link
Collaborator

Working on a solution...stay tuned.

@dmotz
Copy link
Author

dmotz commented Dec 14, 2012

I understand the desire for parse() to stay transit agnostic (I use Backbone with localStorage frequently), but on the other hand, sending metadata in headers seems to be somewhat common.

For others trying to deal with this change in 0.9.9 with an API they don't control, the workaround is less elegant but fairly simple by overriding the collection's sync method:

sync: function() {
  var _this, xhr;
  _this = this;
  xhr = Backbone.Collection.prototype.sync.apply(this, arguments);
  // pass xhr to first reset event:
  this.once('reset', function() {
    _this.onReset(xhr);
  });
  return xhr;
},

onReset: function(xhr) {
  // stash some arbitrary headers here:
  this.count = xhr.getResponseHeader('Count')
}

@wyuenho
Copy link
Contributor

wyuenho commented Dec 16, 2012

Hmmm. Removing xhr seems to create more trouble than its worth. I can see only 2 ways of getting to the xhr, one is to override sync like this,

sync: function (method, model, options) {
  var success = options.success;
  options.success = function (reps, status, xhr) {
    // do something with xhr before parsing
    if (success) success(resp, status, xhr);
  };
  return Backbone.Collection.prototype.sync.call(this, method, model, options);
}

the other is to use Deferred like this:

col.fetch().done(function (resp, status, xhr) {
// do something with `xhr`
});

Unfortunately, this method only works with jQuery at the moment as Zepto does not yet return a Deferred and like @dmotz 's, it processes the headers after parsing so you can't conditionally parse your response data according to the information provided in the headers.

If xhr was left in parse, old code won't break and there's no need to do contortions like the above methods. I really can't think of a reason of xhr causing trouble. If xhr is not used, then it's not used. It's just a convenience for users, no need to be a purist. I think it should go back into parse.

@caseywebdev
Copy link
Collaborator

parse should be passed the options object. The default Backbone.sync method should add the xhr the the options hash so it's accessible that way. This method keeps all AJAX specific code inside the default Backbone.sync function, while still allowing access to the xhr object. This also opens a door for other useful information to be passed to parsed through overridden Backbone.sync methods. I've opened a PR at #1951.

jashkenas added a commit that referenced this issue Dec 17, 2012
Fix #1939 - `parse` receives `options`
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

5 participants