This repository has been archived by the owner on Apr 12, 2024. It is now read-only.
WIP - feat($resource): add support for cancelling requests using promises #13058
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously, it was not possible to use a promise as value for the
`timeout` property of an action's `config`, because that value would be
copied before being passed to `$http`.
This commit introduces a special value for the `timeout`, namely
`'promise'`. Setting an action's `timeout` configuration property to
`'promise'`, will create a new promise and pass it to `$http` for each
request. The associated deferred, is attached to the resource instance,
under `$timeoutDeferred`.
Example usage:
```js
var Post = $resource('/posts/:id', {id: '@id'}, {
query: {
method: 'GET',
isArray: true,
timeout: 'promise'
}
});
var posts = Post.query(); // GET /posts
...
// Later we decide to cancel the request
// in order to make a new one
posts.$timeoutDeferred.resolve();
posts.query({author: 'me'}); // GET /posts?author=me
...
```
Fixes angular#9332
Closes angular#13050
|
This is a POC to get some feedback. Still missing/pending (if we decide to move this forward):
|
|
I'm not qualified to review, but my opinion is that this is an excellent solution! Thanks a lot! |
|
Not quite (e.g. see #12657 (comment) and next couple of comments). This is adding better support for cancelling resource requests. |
|
I see! The feature is a good idea. I am not sure of the API, though. |
|
I don't feel too strongly about the proposed API, but I would rather avoid the boiler plate of having to pass a promise every time (especially if we can easily avoid it :)). I'm definitely open to better ideas though. |
|
Let's have a think about it. |
|
Here's another idea (thinking loud):
(I wonder what other libs do. Have to take a look around.) |
|
@gkalpak I like the sound of that better |
|
From a quick look to other libs (Restangular, Angular Restmod, ModelCore, angular-restResource), they don't seem to have any special handling in place. |
|
Good stuff @gkalpak |
gkalpak
added a commit
to gkalpak/angular.js
that referenced
this pull request
Oct 30, 2015
Introduced changes:
- Deprecate passing a promise as `timeout` (for `$resource` actions).
It never worked correctly anyway.
Now a warnign is logged (using `$log.debug()`) and the property is
removed.
- Add support for a boolean `cancellable` property to actions'
configuration, the `$resource` classes `options` of the
`$resourceProvider`'s defaults.
If true, the `$cancelRequest` method (added to all returned values for
non-instance calls) will abort the request (if it's not already
completed or aborted).
If there is `timeout` specified on the action's configuration, the value
of `cancellable` is ignored.
Example usage:
```js
var Post = $resource('/posts/:id', {id: '@id'}, {
get: {
method: 'GET',
cancellable: true
}
});
var currentPost = Post.get({id: 1});
...
// A moment later the user selects another post, so
// we don't need the previous request any more
currentPost.$cancelRequest();
currentPost = Post.get({id: 2});
...
```
Fixes angular#9332
Closes angular#13050
Closes angular#13058
gkalpak
added a commit
to gkalpak/angular.js
that referenced
this pull request
Nov 23, 2015
Introduced changes:
- Deprecate passing a promise as `timeout` (for `$resource` actions).
It never worked correctly anyway.
Now a warnign is logged (using `$log.debug()`) and the property is
removed.
- Add support for a boolean `cancellable` property to actions'
configuration, the `$resource` classes `options` of the
`$resourceProvider`'s defaults.
If true, the `$cancelRequest` method (added to all returned values for
non-instance calls) will abort the request (if it's not already
completed or aborted).
If there is `timeout` specified on the action's configuration, the value
of `cancellable` is ignored.
Example usage:
```js
var Post = $resource('/posts/:id', {id: '@id'}, {
get: {
method: 'GET',
cancellable: true
}
});
var currentPost = Post.get({id: 1});
...
// A moment later the user selects another post, so
// we don't need the previous request any more
currentPost.$cancelRequest();
currentPost = Post.get({id: 2});
...
```
Fixes angular#9332
Closes angular#13050
Closes angular#13058
gkalpak
added a commit
to gkalpak/angular.js
that referenced
this pull request
Nov 23, 2015
Introduced changes:
- Deprecate passing a promise as `timeout` (for `$resource` actions).
It never worked correctly anyway.
Now a warning is logged (using `$log.debug()`) and the property is
removed.
- Add support for a boolean `cancellable` property in actions'
configuration, the `$resource` factory's `options` parameter and the
`$resourceProvider`'s `defaults` property.
If true, the `$cancelRequest` method (added to all returned values for
non-instance calls) will abort the request (if it's not already
completed or aborted).
If there is a numeric `timeout` specified on the action's configuration,
the value of `cancellable` will be ignored.
Example usage:
```js
var Post = $resource('/posts/:id', {id: '@id'}, {
get: {
method: 'GET',
cancellable: true
}
});
var currentPost = Post.get({id: 1});
...
// A moment later the user selects another post, so
// we don't need the previous request any more
currentPost.$cancelRequest();
currentPost = Post.get({id: 2});
...
```
BREAKING CHANGE:
Using a promise as `timeout` is no longer supported and will log a
warning. It never worked the way it was supposed to anyway.
Before:
```js
var deferred = $q.defer();
var User = $resource('/api/user/:id', {id: '@id'}, {
get: {method: 'GET', timeout: deferred.promise}
});
var user = User.get({id: 1}); // sends a request
deferred.resolve(); // aborts the request
// Now, we need to re-define `User` passing a new promise as `timeout`
// or else all subsequent requests from `someAction` will be aborted
User = $resource(...);
user = User.get({id: 2});
```
After:
```js
var User = $resource('/api/user/:id', {id: '@id'}, {
get: {method: 'GET', cancellable: true}
});
var user = User.get({id: 1}); // sends a request
instance.$cancelRequest(); // aborts the request
user = User.get({id: 2});
```
Fixes angular#9332
Closes angular#13050
Closes angular#13058
petebacondarwin
pushed a commit
to petebacondarwin/angular.js
that referenced
this pull request
Nov 23, 2015
Introduced changes:
- Deprecate passing a promise as `timeout` (for `$resource` actions).
It never worked correctly anyway.
Now a warning is logged (using `$log.debug()`) and the property is
removed.
- Add support for a boolean `cancellable` property in actions'
configuration, the `$resource` factory's `options` parameter and the
`$resourceProvider`'s `defaults` property.
If true, the `$cancelRequest` method (added to all returned values for
non-instance calls) will abort the request (if it's not already
completed or aborted).
If there is a numeric `timeout` specified on the action's configuration,
the value of `cancellable` will be ignored.
Example usage:
```js
var Post = $resource('/posts/:id', {id: '@id'}, {
get: {
method: 'GET',
cancellable: true
}
});
var currentPost = Post.get({id: 1});
...
// A moment later the user selects another post, so
// we don't need the previous request any more
currentPost.$cancelRequest();
currentPost = Post.get({id: 2});
...
```
BREAKING CHANGE:
Using a promise as `timeout` is no longer supported and will log a
warning. It never worked the way it was supposed to anyway.
Before:
```js
var deferred = $q.defer();
var User = $resource('/api/user/:id', {id: '@id'}, {
get: {method: 'GET', timeout: deferred.promise}
});
var user = User.get({id: 1}); // sends a request
deferred.resolve(); // aborts the request
// Now, we need to re-define `User` passing a new promise as `timeout`
// or else all subsequent requests from `someAction` will be aborted
User = $resource(...);
user = User.get({id: 2});
```
After:
```js
var User = $resource('/api/user/:id', {id: '@id'}, {
get: {method: 'GET', cancellable: true}
});
var user = User.get({id: 1}); // sends a request
instance.$cancelRequest(); // aborts the request
user = User.get({id: 2});
```
Fixes angular#9332
Closes angular#13050
Closes angular#13058
petebacondarwin
added a commit
to petebacondarwin/angular.js
that referenced
this pull request
Nov 23, 2015
Introduced changes:
- Deprecate passing a promise as `timeout` (for `$resource` actions).
It never worked correctly anyway.
Now a warning is logged (using `$log.debug()`) and the property is
removed.
- Provide a `cancelRequest` static method on the Resource that will abort
the request (if it's not already completed or aborted).
If there is a numeric `timeout` specified on the action's configuration,
this method will have no effect.
Example usage:
```js
var Post = $resource('/posts/:id', {id: '@id'}, {
get: {
method: 'GET'
}
});
var currentPost = Post.get({id: 1});
...
// A moment later the user selects another post, so
// we don't need the previous request any more
Post.cancelRequest(currentPost);
currentPost = Post.get({id: 2});
...
```
BREAKING CHANGE:
Using a promise as `timeout` is no longer supported and will log a
warning. It never worked the way it was supposed to anyway.
Before:
```js
var deferred = $q.defer();
var User = $resource('/api/user/:id', {id: '@id'}, {
get: {method: 'GET', timeout: deferred.promise}
});
var user = User.get({id: 1}); // sends a request
deferred.resolve(); // aborts the request
// Now, we need to re-define `User` passing a new promise as `timeout`
// or else all subsequent requests from `someAction` will be aborted
User = $resource(...);
user = User.get({id: 2});
```
After:
```js
var User = $resource('/api/user/:id', {id: '@id'}, {
get: {method: 'GET'}
});
var user = User.get({id: 1}); // sends a request
User.cancelRequest(instance); // aborts the request
user = User.get({id: 2});
```
Fixes angular#9332
Closes angular#13050
Closes angular#13058
petebacondarwin
added a commit
to petebacondarwin/angular.js
that referenced
this pull request
Nov 23, 2015
Introduced changes:
- Deprecate passing a promise as `timeout` (for `$resource` actions).
It never worked correctly anyway.
Now a warning is logged (using `$log.debug()`) and the property is
removed.
- Provide a `cancelRequest` static method on the Resource that will abort
the request (if it's not already completed or aborted).
If there is a numeric `timeout` specified on the action's configuration,
this method will have no effect.
Example usage:
```js
var Post = $resource('/posts/:id', {id: '@id'}, {
get: {
method: 'GET'
}
});
var currentPost = Post.get({id: 1});
...
// A moment later the user selects another post, so
// we don't need the previous request any more
Post.cancelRequest(currentPost);
currentPost = Post.get({id: 2});
...
```
BREAKING CHANGE:
Using a promise as `timeout` is no longer supported and will log a
warning. It never worked the way it was supposed to anyway.
Before:
```js
var deferred = $q.defer();
var User = $resource('/api/user/:id', {id: '@id'}, {
get: {method: 'GET', timeout: deferred.promise}
});
var user = User.get({id: 1}); // sends a request
deferred.resolve(); // aborts the request
// Now, we need to re-define `User` passing a new promise as `timeout`
// or else all subsequent requests from `someAction` will be aborted
User = $resource(...);
user = User.get({id: 2});
```
After:
```js
var User = $resource('/api/user/:id', {id: '@id'}, {
get: {method: 'GET'}
});
var user = User.get({id: 1}); // sends a request
User.cancelRequest(instance); // aborts the request
user = User.get({id: 2});
```
Fixes angular#9332
Closes angular#13050
Closes angular#13058
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously, it was not possible to use a promise as value for the
timeoutproperty of an action'sconfig, because that value would be copied before being passed to$http.This commit introduces a special value for the
timeout, namely'promise'.Setting an action's
timeoutconfiguration property to'promise', will create a new promise and pass it to$httpfor each request. The associated deferred, is attached to the resource instance, under$timeoutDeferred.Example usage:
Fixes #9332
Closes #13050