internals.isFunction returns false if the provided functionToCheck is an async function

[carnesen]

The function internals.isFunction

hapi-auth-jwt2/lib/index.js

Line 38 in f2560e6

internals.isFunction = function (functionToCheck) {

uses an expression like this ({}).toString.call(functionToCheck) to determine whether the provided functionToCheck is a function. If functionToCheck is an async function (e.g. async function () {}), the expression evaluates to '[object AsyncFunction]', which is not equal to '[object Function]' so isFunction returns false. It should return true for an async function too.

[nelsonic]

@carnesen how are you creating your asnyc function?

[carnesen]

Thanks for the reply. To answer your question, I prefer async function declarations

async function doStuff () {}

Seems to me though internals.isFunction returns false for doStuff as it does for async () => {} (async arrow function expression) and async function () {} (async function expression).

I'd say the fix here is to add a || getType === '[object AsyncFunction]' to the conditional like lodash.isFunction does https://github.com/lodash/lodash/blob/534296bba4a9bb03d9e59a5153c2bd20a7af631b/isFunction.js#L19

Shall I submit a PR for it?

[nelsonic]

@carnesen async function is a Draft Spec: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/async_function
Just because browser vendors have started implementing against the draft spec, doesn't mean we should be using the feature in our code ...

For the record I like the functionality, and if anyone has time to PR an update to our isFunction checker (with tests) we'll merge it in for "future-proofing",
but we really shouldn't be using a "Draft" feature;
what's the rush to move into the house full of "wet pain"...?

[carnesen]

@nelsonic I don't know why that Mozilla page still says async/await is in "draft" status. The finalized spec was accepted into the the 2017 standard of the language last month http://2ality.com/2016/02/ecmascript-2017.html . Furthermore it's definitely implemented in the "current" version of Node.js (8), which is all we really care about here. In fact async/await has been enabled by default in the Chrome V8 engine that powers Node.js since last November. In other words, the paint is dry on async/await.

[nelsonic]

@carnesen I think the reason that async function is still considered draft is because it's been accepted for ES8 see: https://tc39.github.io/ecma262/#sec-async-function-definitions
which version of Node.js are you using that already has it? (v8.1.4?)

[carnesen]

Everything I've heard about async/await says that it's really truly part of the language spec now, e.g. https://hackernoon.com/es8-was-released-and-here-are-its-main-new-features-ee9c394adf66 . In Node.js async/await has been available (and not behind a flag) since 7.6+. I try to stick to the "long-term support" versions of Node.js, and waited until Node.js 8 was released a month or so ago.

[nelsonic]

@carnesen node v.8.x will enter "LTS" in October 2017. see: https://github.com/nodejs/LTS

Again, if you have time please Pull Request an update to internals.isFunction thanks! 😉