Pr1701 #2130
Pr1701 #2130
Conversation
|
Looks awesome, thanks! |
| * }); | ||
| * </pre> | ||
| * | ||
| * # Response interceptors (DEPRECATED) | ||
| * | ||
| * Before you start creating interceptors, be sure to understand the |
There was a problem hiding this comment.
Can remove these two comments since they were added above
|
The PR looks almost good, the biggest problems I see are:
|
myApp.factory('myAroundInterceptor', function($rootScope, $timeout) {
return function(configPromise, responsePromise) {
return {
request: configPromise.then(function(config) {
return config
});
response: responsePromise.then(function(response) {
return 'ha!';
}
});
}
myApp.config(function($httpProvider){
$httpProvider.aroundInterceptors.push('myAroundInterceptor');
});
|
LGTM. can you tests this on DFA before merging? We should understand the consequences of requiring the extra digest in tests... |
|
Awesome, guys. This is an excellent improvement over @inukshuk's initial work, and I'm glad to see him contributing to this PR as well. Good work! |
|
To give credit where credit is due, this is all @mhevery and @IgorMinar here – there are just some leftovers from the previous commit : ) |
ghost
assigned
|
Ah, don't destroy my dreams of getting on the contributors list one day, @inukshuk...! ;) |
|
Impressive work, guys. Clever use of promise chaining to wrap the server request. Can't wait to start using this. |
|
I'm looking forward for this release. |
|
Any word on this PR's readiness for merging? |
|
Cool. Any release date known for this feature? |
|
Will this patch be included in the upcoming 1.2.0 release? |
|
There is a bug in this PR where the error callback of injected responseInterceptors were not getting called. I have a fix in https://github.com/jbdeboer/angular.js/commits/pr-2130-fix Other than that everything appears to be happy and we should be able to land this PR very soon. |
|
Landed in 4ae4681 |
|
Yes!! Great work. Can't wait to upgrade! |
|
Looking at this, I don't understand the decision to require the extra @c0bra was able to fix this by immediately triggering a new tick (see https://gist.github.com/c0bra/5594851) |
No description provided.