Yet another, but with sync

[domenic]

The readme confuses me. The example claims that .then is synchronous when no option is passed to the constructor, which wouldn't be compliant. It also uses a .done() method that is not documented.

[gobwas]

Hello @domenic, I've updated the package. Please, check it =)

[ForbesLindesay]

@gobwas I've had a look, and your implementation is not spec compliant. In order to make your library spec compliant you must pass a {sync: false} option. I get what you're trying to do, and it may be potentially valuable, but it isn't Promises/A+ compliant.

If you want to make it spec compliant, you must make sync default to false. I think this would probably be the clearer API anyway, since then users are explicit about wanting special synchronous promises.

P.S. Although not required for Promises/A+, it is a general best practice (and required to match ES6 promises) that your resolve and reject methods should never throw an error and should just ignore subsequent calls.

[gobwas]

Hi @ForbesLindesay! Thank you for your revision, but, I think, you are talking about already old version. Now it is a different API, without options to Promise =) Could you re-check it?

P.S. I dont understand what do you mean about error throwing in resolve or reject methods? I do not throw anything there.

[ForbesLindesay]

Tests show the option being passed here, here and here. I admit it doesn't look like the options are actually used in the code though.

Tests also show you having to catch the errors here and here. The source code also shows resolve throwing an error here.

[gobwas]

@ForbesLindesay yes, I just forget to remove it from the adapter. There are no this option in source code. Ive updated it.

And about try catch from:

The tests are not structured to deal with that, and if your implementation has the potential to throw exceptions—e.g., perhaps it throws when trying to resolve an already-resolved promise—you should wrap direct calls to your implementation in try/catch when writing the adapter.

So, it is optional. I will think about removing this throwing.

Thanks!

[domenic]

Thanks! Merged as 0dc7d2b76a004cb8df0ab29a990ee42226c23cab.

[gobwas]

Thanks! 🍻

[ForbesLindesay]

@gobwas

So, it is optional. I will think about removing this throwing.

It's optional for the Promises/A+ spec, so optional for inclusion in this list. It is not optional for though ES6 spec though, or any of the proposed constructor specs. As such, users will not expect it to throw.