Promises are increasingly common in our newer JS apis and for good reason - they are a great tool, reduce callback mess and so on. They are quite simple in concept, but also seem really easy to get wrong.
There is a great article about common mistakes and pitfalls with promises from pouchdb:
We have a problem with promises
I really suggest you read the whole thing even if you think you are entirely comfortable with promises - but a part I found particularly powerful to pick out was this:
What can we do here? There are three things:
- return another promise
- return a synchronous value (or undefined)
- throw a synchronous error
That's it. Once you understand this trick, you understand promises.
There is an eslint plugin which will allow some best practices in this article to be enforced in the codebase - eslint-plugin-promise. When i've used it on some hobby projects I feel its really helped me make code cleaner and better to understand (and also understand where i'm going wrong).
So, I am proposing that we enable eslint-plugin-promise in Moodle development for 3.3 (as a warning). I have created MDL-57139 for that and I welcome comments on it.
Some Pros:
- I think it can help developers understand promises better, I truly think it helped me get my head round them better
- Helps encourage design direction towards more promise-y APIs and not mixing with callbacks
- Can detect edge cases we are not handling or where there are real errors
Some Cons:
- It's quite opinionated, will cause existing not necessarily wrong code to throw warnings
- When your async code is updating the UI, the always-return approach can seem quite needless (I think its worth it for the greater benefit)
- We are using jQuery.Deferred (which compatible with Promises/A+ spec since jquery 3.1/Moodle 3.2) - but does mean there are things like .done() which are not detected by this lint rule (we could warn against .done() use if feeling particularly aggressive)
Personally I think it will provide us a big win. You can see a work in progress patch for core code in MDL-57139.
I welcome your comments!