src: refactor call js wrapper

[JckXia]

The visual studio compiler seems to have an issue with deducting the type of call (#1237), which prevents us from using the std::enable_if statement to handle cases where CallJs isn't provided by the user. This PR aims to work around this issue.

[mhdawson]

LGTM

[mhdawson]

This looks reasonable to me. @KevinEady could you take a look as well?

[KevinEady]

Hi @mhdawson / @JckXia ,

The concern of doing this if check at run-time is this conditional will now be executed for every call of the TFSN, whereas before it was determined at compile-time. This may add an amount -- albeit negligible -- of additional computation needed per every call. This was the reasoning of using template metaprogramming.

I remember we needed to wait to land the the TypedTSFN until Node 10 was decommissioned because the std::enable_if was not supported in that Node 10's minimum GCC compiler version. I suppose we did not check for Visual Studio 2017 🤦 Check the original PR here #687

[mhdawson]

We discussed in last team meeting and @vmoroz will try out using the std in a slightly different way that he has used as a work around before.

[vmoroz]

I have addressed this issue in the new PR #1245. It seems that we had two problems:

• The function declaration has wrong order of "modifiers": the static inline modifiers must be before the return type.
  • BTW, as the best practice we must never use static keyword for functions in header files - it produces very inefficient compiled code - each cpp file will have their own copy of such function. I am fixing all such uses in PR src, test: fix errors and warnings in VS 2017 #1245.
• It also seems that VS 2017 does not allow comparison of function pointers with nullptr. We must cast nullptr to the function type to get it passed.

@JckXia feel free to get the fix from my PR to this one. Then, I will rescope my PR to have just warning fixes in VS 2017.

[JckXia]

Thanks @vmoroz! I updated my PR to incorporate this workaround.

[vmoroz]

LGTM

[KevinEady]

Running the CI for windows-2017 on Jenkins.

Node Version	Link	Status
14	https://ci.nodejs.org/job/node-test-node-addon-api-new/6718/	✅
16	https://ci.nodejs.org/job/node-test-node-addon-api-new/6720/	✅
18	https://ci.nodejs.org/job/node-test-node-addon-api-new/6721/	✅

[KevinEady]

LGTM!

[JckXia]

Landed as ad7ff92