Use queueMicrotask() for PerformanceObserver callbacks #660
Conversation
Replace `Promise.resolve().then()` with `queueMicrotask()` for more explicit and efficient microtask scheduling
tunetheweb
left a comment
There was a problem hiding this comment.
Sorry missed this. I think this is OK and better practice, even if functionally equivalent.
@philipwalton any opinions?
|
I'm fine with this change, given this project now supports Baseline widely available. I guess a separate question is whether or not this line is even still needed... |
Oh seems uncertain the underlying bug is fixed reading https://bugs.webkit.org/show_bug.cgi?id=247863 ? And, if so, when it was fixed. I guess to be safe we should wait 30 months? Anyway, let's merge this for now. |
Yeah, I just tried and I'm still seeing the bug in Safari, though it does appear to be fixed in Safari Tech Preview. But I agreed that we'll need to wait a while to remove this. (#277) |
This PR replaces
Promise.resolve().then(){ withqueueMicrotask()when scheduling thePerformanceObserver` callback.This change makes the intention of microtask queuing clear and documents the Safari workaround, and also reduces the overhead of creating unnecessary promises.
queueMicrotask()is part of the Baseline widely available set, so this change should not introduce any new browser compatibility issues.