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

Add Backbone.wrapError and Backbone.wrapSuccess methods #2051

Closed
wants to merge 1 commit into from
Closed

Add Backbone.wrapError and Backbone.wrapSuccess methods #2051

wants to merge 1 commit into from

Conversation

eastridge
Copy link
Contributor

Added per the discussion in: #2031

Note that I choose to add the methods directly on the Backbone object as Backbone.sync is frequently overridden.

@tgriesser
Copy link
Collaborator

Another option to consider, if adding additional functions on the Backbone object isn't desired:

...    
// Make the request, allowing the user to override any Ajax options.
var xhr = options.xhr = Backbone.ajax(_.extend(params, options), success, error);
...

// Set the default implementation of `Backbone.ajax` to proxy through to `$`.
Backbone.ajax = function(params, success, error) {
  return Backbone.$.ajax.call(Backbone.$, params);
};

so that someone could still access the original success & error functions by wrapping/replacing Backbone.ajax

@eastridge
Copy link
Contributor Author

I'm not keen on that particular solution since it would require detecting which position the arguments are in (as to support jQuery.ajax(url, options)). Open to it if others are opposed to cluttering the Backbone object.

@jashkenas
Copy link
Owner

Please elaborate on the use case that we need these hooks for.

@eastridge
Copy link
Contributor Author

@jashkenas Backwards compatibility with 0.9.2 so we can prevent an error event from being triggered in certain cases (we have a difficult to work with service layer). See:

see #2031

@braddunbar
Copy link
Collaborator

In what cases do you want to ignore error events?

@eastridge
Copy link
Contributor Author

It turns out we are going to completely override sync. If this is the core team's preference for how edge cases such as ours would be handled go ahead and close out. I'd like to add though that this behavior was possible to override in 0.9.2 and we found it quite useful, so others might as well and that the removal of wrapError in 0.9.9 seems like a regression.

@braddunbar
Copy link
Collaborator

Backbone.wrapError was always an undocumented api, but I agree that if it was widely used we should provide a replacement. Would you mind explaining the cases in which you were ignoring error events?

@eastridge eastridge closed this Jan 3, 2013
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

Successfully merging this pull request may close these issues.

None yet

4 participants