Skip to content
This repository has been archived by the owner on Dec 1, 2023. It is now read-only.

Pass parsed JSON result into resolve/reject #18

Closed
JosephSilber opened this issue Jul 1, 2015 · 7 comments
Closed

Pass parsed JSON result into resolve/reject #18

JosephSilber opened this issue Jul 1, 2015 · 7 comments

Comments

@JosephSilber
Copy link
Contributor

Currently only the monkey-patched success/error/always methods get the parsed JSON result. Using the standard then callback only gives you the raw xhr request.

Can we please also resolve/reject the actual promise with the parsed data?

Replacing this:

if (this.status >= 200 && this.status < 300) {
    resolve(this);
} else {
    reject(this);
}

with something more like this:

complete(this.status >= 200 && this.status < 300 ? resolve : reject, this);

function complete(completer, xhr) {
    completer({ xhr: xhr, result: parseReq(xhr)[0] });
}

This way you can get the result directly in the callback, without parsing it again yourself:

this.$http.get('users').then(function(response) {
    this.users = response.result;
}.bind(this));

And if you're using ES6, parameter destructuring makes this really nice:

this.$http.get('users').then(({ result: users }) => { this.users = users });

The reason I really want this is because I don't want to use the monkey-patched success/error callbacks. They're nice to use occasionally, but they break the promise chain (once we get proper Promises/A+ compliant promises, chaining then calls becomes a reality).

In addition, if we'll implement request/response interceptors (à la angular), we'll definitely need to properly chain promises.

@steffans
Copy link
Member

steffans commented Jul 1, 2015

I was thinking about using the window.fetch() method for doing all xhr request, which already comes with a Promise based API and also returns a response object. For older Browser a polyfill can be used and the latest Chrome/Opera have it already implmented. (See: https://github.com/github/fetch)

@JosephSilber
Copy link
Contributor Author

@steffans window.fetch currently has no way of aborting a request mid-flight, which to me is a non-starter.

They're talking about adding it at a future date, but there's too much infighting as to whether a promise should be cancellable.

@steffans
Copy link
Member

steffans commented Jul 1, 2015

Im curios why would i even want cancel a request in mid-flight using abort()? Normally i debounce the user input properly instead.

@JosephSilber
Copy link
Contributor Author

@steffans of course I debounce, but to a very low rate (usually 150-250ms) because I want it to feel very responsive - I want to get the user results as soon as possible. This means you get a lot of false positives, if the user stops typing for the slightest instant.

Also, some people choose to use throttling + debouncing. They want users to see results as they type.


Aborting the request is not just about saving server resources (which in most cases it doesn't); it's about preventing race conditions. If your first callback fires after the second one, the results displayed for the user are not the final results that should be shown.


There are other cases for aborting requests (witness the discussion they're having about fetch), but this is the biggest one by far.

@JosephSilber
Copy link
Contributor Author

@steffans
Copy link
Member

@JosephSilber I just updated the promise as you suggested now the parsed data is also available when using then callback.

this.$http.get('users').then(function(response) {
    this.users = response.data;
}.bind(this));

@JosephSilber
Copy link
Contributor Author

Thanks, but I'm not sure modifying the actual xhr response object is such a good idea; that's why I proposed passing in an object with separate properties for the raw object and the parsed data.

Mutating existing data being passed around in a promise chain is looking for trouble (this line just looks wrong - that should use the newly returned promise, not modify the content inline).

When we implement response interceptors, this will be even more of a problem. Imagine one of your interceptors firing off an asynchronous call to something, but by the time the call completes the data has already been tampered with 😦

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants