-
Notifications
You must be signed in to change notification settings - Fork 27.3k
feat(ngResource): Add $abort method to objects returned by $resource "class" actions #5613
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -202,6 +202,8 @@ function shallowClearAndCopy(src, dst) { | |
| * rejection), `false` before that. Knowing if the Resource has been resolved is useful in | ||
| * data-binding. | ||
| * | ||
| * - `$abort`: a method to abort the request if it has not completed yet. | ||
| * | ||
| * @example | ||
| * | ||
| * # Credit card resource | ||
|
|
@@ -303,7 +305,7 @@ function shallowClearAndCopy(src, dst) { | |
| * </pre> | ||
| */ | ||
| angular.module('ngResource', ['ng']). | ||
| factory('$resource', ['$http', '$q', function($http, $q) { | ||
| factory('$resource', ['$http', '$q', '$timeout', function($http, $q, $timeout) { | ||
|
|
||
| var DEFAULT_ACTIONS = { | ||
| 'get': {method:'GET'}, | ||
|
|
@@ -501,6 +503,17 @@ angular.module('ngResource', ['ng']). | |
| } | ||
| }); | ||
|
|
||
| // If parameters do not contain `timeout` which is a promise, create it, | ||
| // so that later this call can be aborted by resolving this promise. | ||
| var timeout = httpConfig.timeout; | ||
| var timeoutDeferred; | ||
| if (!timeout || !timeout.then) { | ||
| timeoutDeferred = $q.defer(); | ||
| httpConfig.timeout = timeoutDeferred.promise; | ||
| // If timeout is specified in milliseconds, use it to abort via promise | ||
| if (timeout) $timeout(timeoutDeferred.resolve, timeout); | ||
| } | ||
|
|
||
| if (hasBody) httpConfig.data = data; | ||
| route.setUrlParams(httpConfig, | ||
| extend({}, extractParams(data, action.params || {}), params), | ||
|
|
@@ -557,6 +570,7 @@ angular.module('ngResource', ['ng']). | |
| // - return the instance / collection | ||
| value.$promise = promise; | ||
| value.$resolved = false; | ||
| if (timeoutDeferred) value.$abort = timeoutDeferred.resolve; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why should this be done only for I'm not a big fan of
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. and to be more precise, it's not just me. there is no consensus about how to handle cancelation in promise-based apis. currently there is no secure/reliable way to do it without compromising guarantees that the promise api already offers.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I get the point about no consensus. I'm not saying this is The Solution. I just want it to be here in case you guys decide this one is the best. And I do think we need a way to prevent success callbacks from happening when user changed his mind while the request was in progress. Regarding
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also it looks like there's a passing test for |
||
|
|
||
| return value; | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there is no need for this hunk. if
timeoutproperty is set on thehttpConfigobject, this object is going to be passed into$httpwhich will do exactly what you did here.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I disagree. There are three options: no property, numeric value or promise value. Latter case I ignore. First case obviously needs handling. However even when numeric value comes in, I cannot just pass it further, as in this case I will not be able to cancel the request. So I have this small code duplication here, but it gives me access to deferred, and it gives me means to cancel the request.