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

Parse request for then callback #7

Closed
JosephSilber opened this issue Jun 7, 2015 · 3 comments
Closed

Parse request for then callback #7

JosephSilber opened this issue Jun 7, 2015 · 3 comments

Comments

@JosephSilber
Copy link
Contributor

Currently, the callback is called with 3 parameters: callback(result, status, xhr). This is supplied by the parseReq function.

However, this is only done for the success, error & always methods (and by extension, the callback passed to the initial call). The actual promise returned still resolves to just the raw xhr object.


I think it would make more sense to pipe the promise directly into parseReq:

promise = promise.then(parseReq, function(request) {
    var promise = new Promise(function(resolve, reject) {
         reject.apply(promise, parseReq(request));
    }));

    return promise;
});

Then the aliases become just that: aliases.

var aliases = Object.create(null, {
    'success' : 'then',
    'error'   : 'catch',
    'always'  : 'finally',
});

for (var alias in aliases) {
    promise[alias] = promise[aliases[alias]].bind(promise);
}

The only issue here is that sadly most current promise implementations do not support multi-value fulfillment. We can either pass an array to then, or just pass the parsed result.

@JosephSilber
Copy link
Contributor Author

Actually, after giving this more thought, I don't think it really makes any sense to fulfill with an array.

So instead, here's what I propose:

We keep the current callbacks as is, but we only pass the result to then. I think it makes way more sense than passing it the xhr directly. 99% of the time the result is really all you want.

@JosephSilber
Copy link
Contributor Author

On further thought1, if we'll implement request/response interceptors (à la angular), we'll probably want the actual xhr object in there.


1 sorry if I'm spamming someone's inbox here...

@JosephSilber
Copy link
Contributor Author

Closing in favor of #18

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

1 participant