angular-promise-buttons icon indicating copy to clipboard operation
angular-promise-buttons copied to clipboard

ES6 promises not working

Open bouncehead13 opened this issue 7 years ago • 6 comments

I'm currently using ES6 with my Angular 1.5.X code. The promise button library is failing to resolve the promises because of this code block.

mVal.finally(function() {
    promiseDone = true;
    handleLoadingFinished(btnEl);
});

mVal.finally is not a function with native ES6 promises. I noticed in your angular 2 version, you check for finally and then have a fallback.

if (promise.finally) {
    promise.finally(resolveLoadingState);
} else {
    promise
        .then(resolveLoadingState)
        .catch(resolveLoadingState);
}

Is it possible to have this approach included into this repo as well.

bouncehead13 avatar Jun 19 '17 02:06 bouncehead13

Yes definitely! Thanks for reporting and digging into this! Would you like to create a PR for this?

johannesjo avatar Jun 19 '17 07:06 johannesjo

PR created for you to review #44

Update: Noticed I hurt the code coverage. Not being as familiar with your tests, do you any free time to improve that for me?

bouncehead13 avatar Jun 20 '17 02:06 bouncehead13

Thank you very much. I will do that!

johannesjo avatar Jun 20 '17 11:06 johannesjo

Awesome! I took some time to learn the tests before creating the PR, thought I almost had it, but hit a road block trying to create a native Promise in replace of $q.defer().

Definitely available to help if need be.

bouncehead13 avatar Jun 20 '17 12:06 bouncehead13

I wonder how do you make es 6 promises work for you with angular1? I might have hit the same problem as you did.

I added unit tests for them in these two commits: https://github.com/johannesjo/angular-promise-buttons/commit/5f8e5611053ee9d35cbb228d7128aba9159a5814 https://github.com/johannesjo/angular-promise-buttons/commit/af61d1d6437aade0c4b9a6b717ba137a9ddb9a61

But I'm unable to get the unit tests to react to the promise being resolved no matter how many scope.$apply() I'm adding.

johannesjo avatar Jun 20 '17 12:06 johannesjo

  1. I just pulled the latest released code into my app and it's working great!
  2. For the unit tests, the second commit listed, did that not work? Reading the code, it looks right.

bouncehead13 avatar Jun 22 '17 12:06 bouncehead13