Deprecate callback API

[felixfbecker]

The discussion has been made on #111

As said in the iconic blog post by isaac:

The basic point here is that “async” is not some magic mustard you smear all over your API to make it fast. Asynchronous APIs do not go faster. They go slower. However, they prevent other parts of the program from having to wait for them, so overall program performance can be improved.

You are providing an async API that is not actually async and is therefor misleading. I therefor think it should be deprecated and removed from the docs.

[jfromaniello]

You are talking about the verify api right?

[felixfbecker]

Yes.

[felixfbecker]

Citing from #111 (comment)

Separating into sync/async functions - Crypto functions in jws are not async, so as it was mentioned streaming could be used but it does not make a lot of sense based on the payloads.

CPU intensive async calls (crypto) make use of libuv's event loop to simulate asynchrony (they are of course working in another thread). We don't have the option to simulate that here. The best choice would probably be to get rid of the callbacks.

[calebmer]

Agreed, this would clear up much confusion 👍

[graingert]

@jfromaniello is this project alive? There's loads of open issues that look like easy fixes.

[uniqname]

If this were done, there would be no option that would avoid an error being thrown against a valid use of the verify api. The arguments for removing the "async" version are good ones, but there ought to be a way of checking the validity of a JWT without throwing. As is, the "sync" version of verify is an assertion, rather than a check.

[fiddur]

@uniqname: That's a good point, a verify semantically should say true or false, while decode would give payload. There is something to be said for convenience, though.

We can consider this for a next major version, there are a lot of smaller breaking changes already planned for a coming version, but imo, if it is synchronous in the implementation, it makes sense to remove the async.

[fiddur]

An alternative would of course be to let the asynchronous calls use https://github.com/ronomon/crypto-async, but would be an extra dependency...

[PeppeL-G]

Another point of view:

At the moment, this package might not take advantage of the benefits with asynchronous operations, but if it will start to do that in the future, then all of us who are using the asynchronous functions from the API now can benefit from those changes without making any changes to our own code.

But if that will/can never happen, then it's better to just use the synchronous functions from the API.

[graingert]

Callers can just use Promise.resolve if they don't know if an API will become asynchronous

[ziluvatar]

Since v8.3.0 verify can behave async when fetching the key for the verification, the callback parameter is needed to allow the async behavior for the current API.

See: #480