verify: remove process.nextTick

[ilyapx]

running synchronous code asynchronously using process.nextTick
has a negative latency impact

[ziluvatar]

Hi @ilyapx, thanks for the PR, I'll mark it to be merged in next major release, in case, somehow someone is expecting to get the result in the next tick.

[LinusU]

I think that this has a very negative impact on users of this library. When passing a callback to a function there is an implicit contract that the function falls into one of two categories. 1) function that uses the callback n number of times, here the callback is generally not even called callback, e.g. forEach, map. 2) functions that performs io or computations and then later returns an answer.

When using functions that are in category 2 you expect the callback to be called later, since you expect for some work to be done while the event-loop keeps ticking on. Not making this very clear to the user generally leads to releasing zalgo (blog post by isaacs). That is very bad.

I very much feel that the best thing to do would be to remove the callback entirely since it serves absolutely no purpose. There is no work being computed in another thread. There is no asynchronous IO taking place. In fact, everything will be computed before the function have returned, wether you are passing a callback or not.

The very fact that the callback parameter exists is something that has been confusing me when using this library a number of times. Since nothing asynchronous is happening it makes about as much sense as adding a callback to a function that concatenates two strings together.

function concatenate (a, b, cb) {
  cb(null, `${a}${b}`)
}

[ilyapx]

I agree,
this fix resolves the performance issue without changing the API, because it rather hard to make changes to the API.

[LinusU]

[...] because it rather hard to make changes to the API.

It was released as a breaking change though...

Also, have there been any measuring of the performance? I cannot imagine this being the bottleneck in any normal app at all? Especially under load this will defer back to the event loop so it could potentially make the throughput worse under heavy load due to starving of the event loop. Anyhow, I don't think it will make any difference at all...

[ilyapx]

I arrived to this code after doing benchmarks on our app and seeing unreasonable delays in the execution, still small but yet unreasonable.

this was second PR to fix the performance issues, the first was https://github.com/auth0/express-jwt/pull/157/files

[wallali]

Might be useful to bear in mind: http://blog.izs.me/post/59142742143/designing-apis-for-asynchrony