Skip to content
This repository has been archived by the owner on Sep 18, 2019. It is now read-only.

Added ifCached function #51

Merged
merged 4 commits into from
Oct 15, 2016
Merged

Added ifCached function #51

merged 4 commits into from
Oct 15, 2016

Conversation

Tommatheussen
Copy link
Contributor

As discussed, the .ifCached method uses the same callback method as the .then method.

Right now nothing happens when useLegacyPromiseExtensions is set to false, the application will just throw a general error saying the .cached function does not exist for the method (when it is used). I noticed in the angular code itself they throw the legacy errors like this:

var $httpMinErr = minErr('$http');
var $httpMinErrLegacyFn = function(method) {
  return function() {
    throw $httpMinErr('legacy', 'The method `{0}` on the promise returned from `$http` has been disabled.', method);
  };
};

(https://github.com/angular/angular.js/blob/v1.5.x/src/ng/http.js#L12)

Which they invoke using:

promise.success = $httpMinErrLegacyFn('success');

(https://github.com/angular/angular.js/blob/v1.5.x/src/ng/http.js#L1006)

I will try and see if it's possible to import this method, but it doesn't seem like it. If not, do you think it's worth making it the same kind of error, or should a generic thrown error be sufficient?

@shaungrady
Copy link
Owner

shaungrady commented Oct 14, 2016

I'd say just do it like below. I'll add info about it to the docs and add a link in the error.

throw new Error('The method `cached` on the promise returned from `$http` has been disabled.')

Can you write a couple tests please? One testing ifCached and one testing useLegacyPromiseExtensions set to false and cached throwing the error. You can see how I wrote another test about that here: https://github.com/shaungrady/angular-http-etag/blob/master/test/httpDecorator.js#L286-L296

@Tommatheussen
Copy link
Contributor Author

I'll try to add tests this weekend, as well as the error being thrown.

Copy link
Owner

@shaungrady shaungrady left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, thank you so much!

@shaungrady shaungrady merged commit c6c5c2f into shaungrady:master Oct 15, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants