Adding always method to $q service.

[laurent22]

The current implementation of $q doesn't support the always() method. This is a very useful method since it can avoid creating duplicate code when you do not care about the outcome of a promise. Here's some sample code:

// Cover the app with an overlay to disable user input. We
// want to remove it whether the initialization was successful
// or not.
LoadingOverlay.start();
Auth.initialize().then(function() {
    LoadingOverlay.stop();
}, function() {
    LoadingOverlay.stop(); // duplicate code!!
});

With an always method it could be simplified to just this:

LoadingOverlay.start();
Auth.initialize().always(function() {
    LoadingOverlay.stop();
});

The proposed implementation is to add an always() method to the promise object. This method simply re-uses the existing then() method, which means it won't bloat the $q implementation.

If there's any interest in this implementation, I'd be glad to add the tests for it.

[pkozlowski-opensource]

This would be a valuable addition provided that implementation would be short and wouldn't add too many bytes to the framework. I have found myself recently in a need of something like `always'. Having said this I'm perfectly fine with:

LoadingOverlay.start();
Auth.initialize().then(LoadingOverlay.stop, LoadingOverlay.stop);

Also we should stick to the naming and semantics used by the original q library:
https://github.com/kriskowal/q/wiki/API-Reference#promisefinallycallback

So, I would say - why not - provided that we can have a really concise implementation. Tests and doc updates are mandatory of course.

[laurent22]

I'm also using something like Auth.initialize().then(LoadingOverlay.stop, LoadingOverlay.stop); at the moment, however the finally/always method would be useful for more complex finalization code. Basically if it's more than two lines, a finalize() method is useful.

Regarding the naming of the method, I've just found this issue, where a user found out that finally doesn't work in Android browser: kriskowal/q#200

The solution is to either put the keyword inside a string somepromise['finally'](...) or use the fin() alias. I'm not a big fan of this design as it essentially means finally should never be used directly since it can fail randomly depending on the browser. jQuery solved this by using the non-reserved keyword always. What do you think? Should we use always like jQuery or the less compatible but more spec-compliant finally?

I've started looking into the test and doc updates, I should push an update soon.

[laurent22]

I have now implemented the full finally spec. For the most part, I am still re-using the current then() API but with additional checks to make sure the callback result is handled properly. I have also updated the doc and added the tests. Please have a look at the changes when you have a moment.

As I mentioned earlier, I have a preference for the name always but of course I am open to discussing it.

[petebacondarwin]

This looks good. Can you make sure the following tasks are completed:

• Contributor signed CLA now or in the past (if you just signed, leave a comment here with your real name for cross reference)
• Feature improves existing core functionality
• API is compatible with existing Angular apis and relevant standards (if applicable)
• PR doesn't contain a breaking change
• PR contains unit tests
• PR contains e2e tests (if suitable)
• PR contains documentation update
• PR passes all tests on Travis (sanity)
• PR passes all tests on ci.angularjs.org (cross-browser compatibility)
• PR is rebased against recent master
• PR is squashed into one commit per logical change
• PR's commit messages are descriptive and allows us to autogenerate release notes (required commit message format)
• All changes requested in review have been implemented

[petebacondarwin]

I think we need a core team member to approve the name "always" over "finally". Other than that can you make sure the other tasks are completed.

[joekilner]

I think "always" is better semantically. To me, this is pretty clear what it does:

getPromise().always(doSetup).then(succeed, orFail).always(doCleanup);

if you call it finally it doesn't read so well and the order of execution is less clear:

getPromise().finally(doSetup).then(succeed, orFail).finally(doCleanup);

[petebacondarwin]

I agree but do @IgorMinar or @mhevery - have a view?

[IgorMinar]

let's go with always. it reads better and is compatible with all browsers.

[laurent22]

@petebacondarwin, ok great I have now signed the CLA (Laurent C.).

I have also squashed the commits into one and updated the commit message to follow the proper format.

[petebacondarwin]

Great! Thanks @laurent22
I have merged this in at 6605adf

[marcospgp]

"let's go with always. it reads better and is compatible with all browsers."

I can only find a reference to .finally in the docs, what happened?

Edit: To anyone with the same doubt: f078762