A better way to detect native Promise?

[watson]

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

async-listener/index.js

Lines 260 to 274 in 0c2e2ec

	if (instrumentPromise) {
	// shoult not use any methods that have already been wrapped
	var promiseListener = process.addAsyncListener({
	create: function create() {
	instrumentPromise = false;
	}
	});
	// should not resolve synchronously
	global.Promise.resolve(true).then(function notSync() {
	instrumentPromise = false;
	});
	process.removeAsyncListener(promiseListener);
	}

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 😢