Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TAG Review Request: queueMicrotask #294

Closed
2 tasks
fergald opened this issue Jul 20, 2018 · 16 comments
Closed
2 tasks

TAG Review Request: queueMicrotask #294

fergald opened this issue Jul 20, 2018 · 16 comments
Assignees
Labels
Progress: pending external feedback The TAG is waiting on response to comments/questions asked by the TAG during the review

Comments

@fergald
Copy link

fergald commented Jul 20, 2018

Hello TAG!

I'm requesting a TAG review of:

Further details (optional):

We'd prefer the TAG provide feedback as (please select one):

  • open issues in our Github repo for each point of feedback
  • open a single issue in our Github repo for the entire review
  • [x ] leave review feedback as a comment in this issue and @-notify [github usernames]
@cynthia
Copy link
Member

cynthia commented Jul 25, 2018

@fergald The explainer Google Doc link seems to be locked, would it be possible to make it world open?

@travisleithead
Copy link
Contributor

@dbaron wonders if the name "microtask" is the best name to expose to developers--yes its the spec-term, but may not be the best name :-)

@domenic
Copy link
Member

domenic commented Jul 25, 2018

Hmm, any other suggestions?

@fergald
Copy link
Author

fergald commented Jul 26, 2018

@cynthia Sorry about that. Done.

@dbaron
Copy link
Member

dbaron commented Jul 26, 2018

So one thought is that setTimeout has clearTimeout, requestAnimationFrame has cancelAnimationFrame, and requestIdleCallback has cancelIdleCallback... but this has no cancellation method, which seems a bit odd. (It could be problematic if somebody is trying to convert code that uses one of the others to this one, which seems like a pretty reasonable scenario.)

@slightlyoff
Copy link
Member

Hey @fergald: thanks for filing a review request!

Some questions:

  • Is there a fuller code example somewhere? How would a developer use this API in context vs. the alternatives?
  • Is there an argument against Promise.resolve().then(....)? Implicit object allocation? Promise resolution order? Something else? The original discussion thread takes a lot of turns and I'm struggling to pull out the case being argued here.
  • What other timings should be exposed that aren't available in an API like this today?

@travisleithead travisleithead added Progress: pending external feedback The TAG is waiting on response to comments/questions asked by the TAG during the review and removed extra time labels Jul 26, 2018
@dbaron dbaron added this to the 2018-08-07-telcon milestone Jul 26, 2018
@domenic
Copy link
Member

domenic commented Jul 26, 2018

I'm working on adding some examples to the spec in my spare time; I have them about half-completed.

The argument against Promise.resolve().then() is severalfold:

  • Object allocation
  • Swallows exceptions and converts them into rejected promises
  • Bad layering: promises are layered on top of microtasks, but right now you need to use the high-level feature (promises) to get at the lower-level underlying capability (microtasks). We should just expose the lower-level feature.

Not aware of many other unavailable timings, personally... maybe the different task sources the browser uses for task-queuing?

@fergald
Copy link
Author

fergald commented Jul 27, 2018

@slightlyoff

Re: Arguments - I've updated the explainer to include Domenic's justifications.

Re: example for using this vs alternatives - I've rewritten the motivation section. There are 2 things I could be explaining:

  • when to use microtask vs task vs RAF
  • why we want to make queueMicrotask an API

Previously the doc was a mix of both. It's now focused on the latter (but includes a link to another article that explains more about the former). Viewed in that way, there are no reasonable alternatives, if you want to queue a microtask you should call queueMicrotask(), there is no scenario where you should still consider Promise.resolve().then() (assuming you're on a browser that supports it).

If you feel strongly that the doc needs to explain more about what a microtask is and when it's the right choice, I can add that but it seems secondary.

Re: other timings that should be exposed - I defer to others on that.

@dbaron Cancellation seems reasonable to me. @domenic any opinions on that?

@slightlyoff
Copy link
Member

slightlyoff commented Jul 27, 2018

The last argument about layering is pretty compelling to me. Do we know for a fact that implementations don't elide Promise.resolve() overhead?

@domenic
Copy link
Member

domenic commented Jul 27, 2018

I'm like 95% sure without testing/asking anyone. There are so many observable side effects to the promise existing (especially if the queued code returns anything or throws an exception) that it'd take some serious engine heroics to elide the result, and even if they did you could easily fall off of the garden path and accidentally force it to be allocated again.

@domenic
Copy link
Member

domenic commented Aug 1, 2018

@dbaron Cancellation seems reasonable to me. @domenic any opinions on that?

Fergal and I discussed this offline. We were initially agnostic, but came around eventually to thinking that cancelation isn't desirable, at least for now. The main reasons are:

  • It isn't part of the base primitive in existing spec infrastructure, and probably not in implementations either. Promise and mutation observer microtasks are never canceled. As this API is in the spirit of exposing primitives, we shouldn't add on top of it.
  • It isn't nearly as necessary for queueMicrotask as it is for the other scheduling APIs, because microtasks are basically-synchronous, so the intervening time you'd have to cancel is small. In other words, it's much harder to get new information that would cause you to cancel during the rest of your synchronous turn, than it is when multiple event loop turns could happen. For example it would be impossible for a user to click a "Cancel" button that triggers an event handler to cancel the microtask; the event handler would always fire after the microtask already completed.
  • If users really want this, they can build it themselves, by doing queueMicrotask(() => { if (canceled) { return; } ... }). Unlike similar code with setTimeout(), requestAnimationFrame(), et al., asking the browser to hold on to a now-redundant closure is not a big deal, because it's such a short time period.
  • There's no precedent in user-space libraries that attempt to give microtask semantics, even though such libraries could emulate cancelation by doing if (canceled) { ... } wrapping.

@domenic
Copy link
Member

domenic commented Aug 1, 2018

Pull request for examples up: whatwg/html#3873

@dbaron
Copy link
Member

dbaron commented Aug 10, 2018

Your conclusion about cancellation seems fine with me.

@travisleithead
Copy link
Contributor

I'm also satisfied with the conclusions. I only have a couple of minor bits of feedback:

  1. The explainer notes at the beginning, that one of the ways developers workaround the lack of queueMicrotask is by (3) triggering custom element reactions. However, according to the HTML spec, custom element reactions do not actually use microtasks (they use the special "almost synchronous" nano-task concept). So, I'd like to see that assertion dropped from the explainer. I don't believe that we should consider a queueNanotask because such an API would invoke its callback nearly synchronously, which is really pretty pointless--rather the API would need to bind to some other action to be more useful, and at that point it's starting to look a lot like Object.observe.

  2. It would be nice to move the explainer into GitHub (WICG) and reformat as an HTML or MD file for improved discoverability.

@plinss plinss closed this as completed Aug 21, 2018
@fergald
Copy link
Author

fergald commented Sep 4, 2018

@travisleithead Thanks for comments. Removed the custom elements line and migrated the doc to

https://fergald.github.io/docs/explainers/queueMicrotask.html

@travisleithead
Copy link
Contributor

@fergald Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Progress: pending external feedback The TAG is waiting on response to comments/questions asked by the TAG during the review
Projects
None yet
Development

No branches or pull requests

7 participants