A better way to detect native Promise?

[watson]

trafficstars

Right now the presence of native Promise is detected based on how the code is currently instrumented and how it behaves when used:

https://github.com/othiym23/async-listener/blob/0c2e2eca492e8fa6d4534e03e164fe6f9efdf565/index.js#L260-L274

I was thinking that there might be a better way of detecting if the current Promise implementation is native or not - simply by checking if global.Promise is a native function or not.

The simplified approach to check this would rely on the result of String(global.Promise) which produces the following string:

function Promise() { [native code] }

The presence of { [native code] } gives it away.

But as the blog post Detect if a Function is Native Code with JavaScript and the is-native module shows, there are some edge cases that should be checked.

So my question is, could we simply not require is-native or something similar and replace line 257-274 with the following snippet?

var isNative = require('is-native')
var instrumentPromise = isNative(global.Promise)

[hayes]

If I recall correctly, I did it this way because there was a common Pollyfill that that had a toString method that ended up looking the same. I don't remember if it achieved this through function.bind, or if it had a custom .toString method, but the end result was that that was not a good option.

[hayes]

Is there a case where the current approach is causing issues?

[hayes]

I could see an argument for combining the 2 methods in case the Pollyfill is implemented using an uninstrumented asynchronizer

[watson]

Ah yes you are correct - if a function is bound it will be detected as native 😞 ... because obviously it is native. It will however not have the name Promise as bound functions are anonymous (I'm not sure if it's possible to give them a name). But as you said, one can always overwrite the toString function, so it's not to be trusted.

I must admit that the reason why I proposed the change is very selfish. I'm using 99% of the code from index.js internally in the opbeat module so neither process.addAsyncListener nor process.removeAsyncListener are available. So I was hoping to make the code independant of those functions and if is-native provided the same functionality, it would also make the code easier to maintain - two birds with one stone 😉

The main reason why I'm using my own implementation instead of adding this module as a dependency is to optimise the wrapCallback function for my specific use-case. It's not pretty, but it's a tiny bit more performant. Not my proudest work, but the event loop is a messy place 😢