Fix the awaitable collect() calls.

[farcaller]

In case if non-native promises are used, the Promise object won't be instanceof Promise. That's still valid in JS, as Promise is something that implements then(). It's always safe to cast a maybe-promise into native Promise with Promise.resolve().

[farcaller]

Is anyone actually using non-native promises now?

Funnily, yes. ClojureScript's Promesa uses non-native Promises (and that's how I ended up going down this rabbit hole).

[zbjornson]

What do you think about doing this on the consumer side though? i.e. in your custom collect() function, return Promise.resolve(/*the Promesa promise */).

[farcaller]

That's what I'm doing right now. I think it's just a bit of an unexpected behavior from the library to say it takes a promise, but then actually taking a very specific-exactly-JSPromise promise.

I guess, it holds true for the 95% promises flying around, though? Maybe it's worthwhile to mention it in the docs then, to not incur the runtime costs.

[farcaller]

I'm giving this another thought and I wonder: is this actually the use case where the code must be extremely performant? I'd expect that metrics page would be scraped once in a while, or pushed once in a while, where "a while" is dozens of seconds. At that scale, optimizing for microtask scheduling seems a bit futile, is it not?

[zbjornson]

is this actually the use case where the code must be extremely performant?

prom-client gets a surprising number of PRs to improve its performance. I think it's valid to want observability frameworks to be as low of overhead as possible. If you have 100s of metrics with collect() functions, that could have an impact.

I'm +1 on a doc-only change that says the return type needs to be a native Promise. The typescript types are already correct fwiw: they say Promise and not PromiseLike.