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

A setTimeout that returns a promise that resolves after specified time. #617

Open
jisaacks opened this issue Feb 3, 2016 · 70 comments
Open
Labels
addition/proposal New features or enhancements needs implementer interest Moving the issue forward requires implementers to express interest

Comments

@jisaacks
Copy link

jisaacks commented Feb 3, 2016

I really love Promise.resolve("some value"), however if you want to delay this resolving you have to do something much uglier like

new Promise(resolve => setTimeout(() => resolve("some value"), 1000));

Would love a way to create a promise that waits and resolves. I made a proposal for Promise.after to ES and was informed it is a better fit to recommend here.

It seems a global function might make more sense in this context. Maybe something like resolveIn(1000) that would return a promise that resolves in 1000 milliseconds. This could even be implemented like so:

function resolveIn(milliseconds) {
    return new Promise(resolve => setTimeout(() => resolve(milliseconds), milliseconds));
}

And could be used like:

resolveIn(1000).then(/* do something */);

And could be used to resolve a specified value after a delay like so:

resolveIn(1000).then(() => "some value").then(/* do something with "some value" */);
@domenic
Copy link
Member

domenic commented Feb 3, 2016

I am in support of this idea (with a better name, but we can bikeshed that later). I would like to hear if any implementers would implement this assuming I specced it.

@AlicanC
Copy link

AlicanC commented Feb 3, 2016

How about cancelation? clearResolveIn(promise)? Cancelable promises?

@domenic
Copy link
Member

domenic commented Feb 3, 2016

I think we would have to wait for cancelable promises (in whatever form they show up in). That might be enough to sink this feature until that work is done, but I personally don't think it's necessary---adding it right now, and making it cancelable later, would be OK in my book.

@AlicanC
Copy link

AlicanC commented Feb 4, 2016

Are we absolutely sure that making this cancelable later won't be a breaking change or introduce nasty duplication like cancelableResolveIn() or CancelablePromise.after()?

Without cancelation, this proposal is just a shortcut to a one-liner which we can (and do) live without. Achieving clearTimeout functionality with promises is a lot harder and it would be better if this proposal could solve that issue.

@jisaacks
Copy link
Author

jisaacks commented Feb 4, 2016

Are we absolutely sure that making this cancelable later won't be a breaking change or introduce nasty duplication like cancelableResolveIn() or CancelablePromise.after()?

If promises become cancelable in the future, this would automatically become cancelable too.

it would be better if this proposal could solve that issue.

These are 2 separate issues.

@domenic
Copy link
Member

domenic commented Feb 4, 2016

Are we absolutely sure that making this cancelable later won't be a breaking change

Yes, sort of. From one perspective, it could be a breaking change, since code that previously assumed a promise would never be canceled would suddenly start to have to deal with that. This is a general problem with cancelable promises across the platform and is part of the reason why I am scared to start working on them because it will start causing lots of debates. (Which I really do not want to reiterate in this thread; so let's try to drop this line of discussion.)

or introduce nasty duplication like cancelableResolveIn() or CancelablePromise.after()?

We can guarantee this will not happen, yes.

Without cancelation, this proposal is just a shortcut to a one-liner which we can (and do) live without.

Even with cancelation, your statement applies. This proposal is entirely about convenience.

@domenic domenic added addition/proposal New features or enhancements needs implementer interest Moving the issue forward requires implementers to express interest labels Feb 4, 2016
@AlicanC
Copy link

AlicanC commented Feb 4, 2016

Are we absolutely sure that making this cancelable later won't be a breaking change

Yes, sort of.

Cool. 👍

@annevk
Copy link
Member

annevk commented Feb 4, 2016

Would TC39 not want this to be a static on Promise?

@domenic
Copy link
Member

domenic commented Feb 4, 2016

Would TC39 not want this to be a static on Promise?

I am pretty sure no. TC39 does not define an event loop or a notion of time; that is for the embedder. And using statics as a place to store utility methods is not a great pattern; it's forced by necessity in many cases, but in general statics should be reserved for things that vary among subclasses. You could argue this is one such thing, but I am not sure myself...

@jisaacks
Copy link
Author

jisaacks commented Feb 9, 2017

wait Doesn't look to be a reserved word and I think wait(500).then(x) reads rather well.

@benjamingr
Copy link
Member

Hey, copying my reply from the duplicate (thanks for finding this Domenic):

wanderview: I'm somewhat ambivalent, but any such API probably also needs something equivalent to clearTimeout(). Maybe using AbortSignal/AbortController.

Sure, what if we allow passing an AbortController.signal as an optional second parameter?

await delay(3000, { signal: controller.signal} );

In a way similar to fetch?

Another interesting idea is a variant of setInterval that returns an async iterator, though we might not want to tie the two together.

for await(const time of interval(1000)) {
 // do something, `break` here to cancel (or `.return` on the iterator)
}

@Fishrock123
Copy link

AbortController seems like a very odd way around the issue here. I've never heard of the api before but I am not fond of it so far.

This is because it tries to circle around access to the object itself, because Promises are supposed to be immutable and have no direct object access, because apparently people want to return just a Promise.

So here we have something persistent. Particularly in the case of intervals, but also in the case of a Timeout -- for example, in Node timeouts may be 'refed' and 'unrefed' from the event loop, refreshed (as the same object/id), and of course you may simply just want to cancel the timeout (which happens constantly if you are implementing any network protocol).

Another angle, brought up by @jasnell: For an interval, one might want to consume it as an async iterator, something like this:

async foo() {
  const interval = setInterval(fn, 1000)
  for await (const m of interval) {
    /* ... */
    clearInterval(interval) // would end the iterator...
  }
)

Promises do not modal persistently intractable interfaces. That is the problem. (Not just, not well... they just don't at all.)

Cancellable promises only help one very specific part of this issue, it should be noted.

This problem isn't just isolated to timeouts either, there a lot of APIs in Node where we'd like to "have a promise version" (really "an awaitable version") but we also need to return immediate access to something with... events and handlers and stuff. Like a child process or http server.

So, to model this "correctly" by allowing direct access the persistent objects in question, I have suggested that Node make timers "thenable". Which I am moving on recommending for other apis in node as well (as noted in this twitter thread about the subject (please don't reply to that though)). It's "ugly" but the other options are... well, there isn't much in terms of options given the constraints.

I don't have a good answer for web browsers which for some forlorn reason return a number from setTimeout(). (Node.js can't and won't stop returning an object, if you are wondering... as I am sure is the same for the dom and numbers)

@domenic
Copy link
Member

domenic commented Sep 17, 2019

AbortController is the web platform's primitive for cancelation, so that is what we will use for the promise-returning setTimeout analog. I can't speak to the larger issues in your message; I'm unsure how they're related to this issue (or to the web platform).

@Fishrock123
Copy link

I can't speak to the larger issues in your message; I'm unsure how they're related to this issue (or to the web platform).

They are related because (as sizeable group) folks want web compatibility and want to work with e.g. the whatwg to come up with things that work outside web platforms.

Node is also encountering this issue of "people want an awaitable timeout", but also that has the related issues noted above.

@bathos
Copy link

bathos commented Sep 18, 2019

That AbortController/AbortSignal unifies cancellation is its most important property. Right now people are mainly familiar with AC because of fetch, but the key to its effective use is to make all cancellable APIs respect it. This allows one signal to correspond to one cancellable ‘transaction’.

AC is kinda ugly to work with directly, but this is in part because cancellation is an ugly problem. I’d still rather see Node implement AbortController (as they’ve implemented URL, TextEncoder, etc) than see HTML not include it here.

@benjamingr
Copy link
Member

benjamingr commented Sep 18, 2019

@Fishrock123 you and @domenic are not actually disagreeing on this from what I understand. You can have setInterval as an async iterator and at the same time have a cancellable timeout with AbortController.

@devsnek
Copy link

devsnek commented Oct 5, 2019

I started to strawman this out a bit: https://gist.github.com/devsnek/9686da77db0421927c84e862bd70b214

one of my main goals with this approach is to sort of streamline everything into one neat api that's small so its easy to remember and use.

One of my assumptions for this is that https://github.com/tc39/proposal-cancellation moves forward, although it's not an explicit requirement.

@felixfbecker
Copy link

FYI the delay package on npm supports cancellation with AbortSignal: https://www.npmjs.com/package/delay#signal

@Jack-Works
Copy link

@domenic
Copy link
Member

domenic commented Oct 2, 2020

@Jack-Works as discussed previously in this thread, such a proposal is not appropriate for the JavaScript language, but instead needs to be part of the web platform.

@Jack-Works
Copy link

I didn't invent the notion of time, I leave it for the host to interpreter what does the time parameter means.

I also didn't add something like "event loop" to the langauge, I'm re-using the existing Job concept for scheduling task.

@domenic
Copy link
Member

domenic commented Oct 2, 2020

Job is not appropriate for this. Also the lack of integration with AbortSignal is a killer.

Do you have something in particular against doing this in the correct standards body?

@Jack-Works
Copy link

It seems like no one is working on this in whatwg for years.

And I don't like it appears on the AbortController. If so, how can I use it in Node? If it is not in the language, not every platform will implement it.

@domenic
Copy link
Member

domenic commented Oct 2, 2020

It seems like no one is working on this in whatwg for years.

Yes. So why don't you?

If so, how can I use it in Node?

Node.js has AbortController: nodejs/node#31971 (comment)

it is not in the language, not every platform will implement it.

That's not true. Many features standardized outside of TC39 are implemented on multiple platforms. For example, the Encoding Standard, performance.now(), the URL Standard, ...

@Jack-Works
Copy link

So Node need "require(...)" to use the promised timer. That also not good for cross-platform code.

@domenic 's argument only works if major platforms implement it in exact same API (a global object). If they need different way to access, "are implemented in multiple platforms" doesn't provide convenience for this simple function. Hand written one is much easy than environment detect and load different native apis.

@ljharb
Copy link
Contributor

ljharb commented Oct 4, 2020

@Jack-Works it can be imported as well in node.

@benjamingr
Copy link
Member

So Node need "require(...)" to use the promised timer. That also not good for cross-platform code.

What cross-platform code? Timers are not and never were cross-platform. Node does not implement the web-timers specification and while it follows some quirks it doesn't follow others. Additionally it has its own quirks that likely go against the spec.

Node does make timers interop nicely in universal code when possible - for example the new toPrimitive.

Node does have some spec compliant APIs (like URL) and there it follows the same spec as browsers.

@Jack-Works
Copy link

What cross-platform code? Timers are not and never were cross-platform.

That's why we need one.

@benjamingr
Copy link
Member

What about environments that run JavaScript but have no concept of timers like certain embedded environments?

@Jack-Works
Copy link

What about environments that run JavaScript but have no concept of timers like certain embedded environments?

They can throw (based on spec text of my proposal). But whatever, I'm not going to push this to stage 1 now.

@benjamingr
Copy link
Member

@Jack-Works

They can throw (based on spec text of my proposal). But whatever, I'm not going to push this to stage 1 now.

I am not trying to convince you to not pursue this. Please don't take my/our comments as attacks or discouraging.

You are looking to contribute in a (pretty large) area that some of us care about (Some examples: Domenic and Jordan from the specification and whatwg point of view, myself and Jordan from Node timers and I also help maintain sinon/jest's fake-timers).

People are just indicating that TC39 is not the correct avenue to push this because of the scope of what's part of ECMAScript and what's part of the platform.

Domenic's first ever comment to you here was to suggest you in fact work on this :]

Here are some constructive ways to contribute to this (feel free to do some, or none at all):

  • Play with Node's experimental timers/promises to find pros/cons for that sort of API to help bring it as prior art to whatwg for timers.
  • Add tests for timers/promises in Node.
  • Go over the timers specification and get familiar with the machinery used there (like the spec language, terms and bikeshed). Find bits that are not covered by the wpt and add tests for it.

@Jack-Works
Copy link

I am not trying to convince you to not pursue this. Please don't take my/our comments as attacks or discouraging.

Thanks I will ✨

@domenic says

For Chrome/V8, I can say that we're not interested in any version without AbortSignal integration, or any version that does not use the same specification infrastructure as setTimeout.

With objections from implementor, it's no meaning to push this forward because it will be blocked earlier or later. (And therefore it's meaningless to present to stage 1)
My main purpose is to suggest a unified API but it becomes impossible.

By accepting this irreversible conclusion, I'm OK to write platform-aware code.

Since I have to write a helper function whatever I'm not so motivated in investigating timer API in any major platform cause it already has the ability to implement what I want in the user land.

@benjamingr
Copy link
Member

I think the Chrome/V8 requests Domenic raised (AbortSignal integration since that's the built-in cancellation mechanism, and reusing the same timer infrastructure) are pretty reasonable and I would (unsafely) assume other implementors would have similar requirements in order to avoid further fragmentation.

That said such an API (With an optional AbortSignal) would address both your use case (of promise returning timers) and Chrome's (reusing existing machinery). In fact that's the approach Node took.

This is entirely possible and probably (hopefully) we'll eventually get this sort of API.

The thing is: doing that as a first contribution to whatwg/html sounds like a very tall order to me which is why I suggested API feedback / adding tests to the existing machinery.

@jarrodek
Copy link

I didn't know there's a discussion about this going on (I was looking for similar topics in ES proposals). I've prepared an explainer for the same idea. The solution is different though. Maybe it would be helpful in this discussion: https://github.com/jarrodek/delayed-promises

@saschanaz
Copy link
Member

saschanaz commented Dec 3, 2020

FYI, Node.js 15 shipped timers/promises module. nodejs/node#33950 (Edit: as #617 (comment) said)

Edit 2: I'm trying writing a PR.

@domenic
Copy link
Member

domenic commented Nov 16, 2021

Continuing from #7340 (comment) where @annevk said

See https://wicg.github.io/scheduling-apis/#sec-scheduler. The idea is that scheduler.postTask() is that API.

see also #6201

Unfortunately scheduler.postTask() as is today is a bit less ergonomic than what people are using in Node.js or other libraries. In particular if you want to go from

doStuff();
await delay(1000);
doOtherStuff();

to scheduler.postTask() you end up with something like

doStuff();
await scheduler.postTask(() => {}, { delay: 1000 });
doOtherStuff();

@shaseley and I have discussed before whether we could add something simple like scheduler.wait(1000). The worry is basically that we only get one shot at that and if we want to create something well-integrated and future-proof there are some open questions. https://github.com/WICG/scheduling-apis/blob/main/misc/userspace-task-models.md goes into it more, but I think the main example is the following:

await scheduler.postTask(async () => {
  // This code definitely runs at "background" priority
  doStuff();
  
  await scheduler.wait(1000);

  // But what about this code? It's not the same browser task...
  // but it is the same conceptual task for developers. How can we
  // make it run at "background" priority?
  doOtherStuff();
}, { priority: "background" });

@jasnell
Copy link

jasnell commented Nov 16, 2021

I have to agree that postTask() is a bit heavy for the minimal case. A scheduler.wait(n) would be ok as long as it supported an AbortSignal option (e.g. "wait n-milliseconds or until canceled with the AbortSignal").

@josephrocca
Copy link

josephrocca commented Feb 5, 2023

Relevant:

  • Bun just announced that they're adding Bun.sleep(milliseconds) - not blocking despite name. They're also adding a blocking version, Bun.sleepSync. Optional second argument for AbortSignal to potentially come later.

Worth mentioning in this context:

@jasnell
Copy link

jasnell commented Feb 5, 2023

Node.js also includes an implementation of scheduler.wait(n).

Cloudflare workers also has scheduler.wait(n)

@annevk
Copy link
Member

annevk commented Feb 6, 2023

@domenic couldn't schedular.wait() grab priority from the current task?

@Kaiido
Copy link
Member

Kaiido commented Feb 6, 2023

Defining what's the current task's priority isn't that easy I guess.
As exposed in #mixing-async-apis a "userspace task" as defined there, can already be spread across different task sources and thus have various levels of priority assigned to it, e.g await fetch(someURL) would switch the current execution to a low priority network response task.
However while I understand the will for having that new method fit well with this model, I don't really understand what alternatives are proposed? This sounds like an issue with the model itself rather than one with that method.
I feel most people would be just fine with a shorthand for scheduler.postTask(()=>{}, { delay }) and possibly an optional priority field in the option bag that's used for signal, though for the ones needing to set a custom priority, the longhand might do fine.

@annevk
Copy link
Member

annevk commented Feb 6, 2023

Well, we could inherit from postTask()-tasks and not inherit otherwise. Or we could always use the timer task source indeed, but that might not do what people want in @domenic's example.

@Kaiido
Copy link
Member

Kaiido commented Feb 7, 2023

You'd still face situations like

await scheduler.postTask(async () => {
  // This code definitely runs at "background" priority
  doFirstStuff();
  await scheduler.postTask(doSecondStuff, { priority: "user-blocking" });
  // should we inherit from the outer "background" task
  // or from the current "user-blocking" doSecondStuff one?
  await scheduler.wait(1000);
  doOtherStuff();
}, { priority: "background" });

But anyway, wouldn't people also want for fetch()'s task to keep a "user-blocking" priority in a similar example? And then what about callbacks that aren't even part of the priority system like requestAnimationFrame()?

I guess my point is that currently this idea of an "userspace task" that would somehow keep a given priority for all the inner browser tasks (including callbacks?) doesn't seem doable. And I don't see how scheduler.wait() could help with it. I do understand how @domenic and @shaseley came to this through .yield() and .wait() – not having to reiterate again and again which priority you want for all your tasks sounds like a good thing – but I don't see how these two methods can fix the big picture. And hence, I'm a bit doubtful on the need to block these methods over this consideration.
In my very biased opinion, people willing to mess with the task prioritization system would already have some basic understanding of what a "browser task" is, and will probably not be surprised that the prioritization they set for the "parent" task isn't maintained in the "child" ones.
Given both methods are basically shorthands, I'd say setting a good default for each one should probably be good enough. For instance wait() could default to "user-visible" like timer tasks, and maybe yield() default to "user-blocking" since people generally don't want more than the rendering update to slip in there.

@shaseley
Copy link

shaseley commented Feb 8, 2023

I recently published an explainer for scheduler.yield() which discusses inheritance design and proposes an API shape (I'm working on prototyping/drafting a spec now). My thinking is that scheduler.wait() would have a similar API shape in terms of options, and I'm planning to get an explainer/spec for that written once I get yield() a little further along. For inheritance, basically I'm proposing an option to inherit but not necessarily making it the default (still an open question). See the explainer for semantics.

Note: this doesn't fix/uncomplicate the big picture @Kaiido mentions (it's not clear exactly how/if that could be done), but I think it fits in to the current world reasonably well.

@benjamingr
Copy link
Member

@jasnell do you think it makes sense for Node to implement scheduler.yield as experimental?

@shaseley would that help for feedback/prior art if we did?

(In Node terms it'd likely be just setImmediate(resolve, 0) or some such most likely, for what it's worth I think scheduler.yield is a nicer API than what we currently have for it and that it's a pretty common Node use case)

@littledan
Copy link
Contributor

Is there interest from WebKit or Gecko in the Scheduler API?

@smaug----
Copy link

Gecko has had an implementation of the Scheduler API enabled on Nightly for quite awhile now, but there hasn't be any too convincing use cases for it, so it hasn't shipped. We've been pondering whether to remove it or ship it. (since when implementing the spec literally it doesn't really add anything to the platform). But recently in WICG/scheduling-apis#61 it has become clear that Chrome's implementation does some other scheduling too (details a bit unclear), which actually do add new capabilities. If the spec proposal gets clarified, it might convince Gecko to ship the API.

@josephrocca
Copy link

josephrocca commented Feb 16, 2023

Aside: Is there any slight possibility that this could be a global, rather than under scheduler? We're adding new globals all the time (some random ones: EyeDropper, IdleDetector, AbortController, Ink, VirtualKeyboard, etc), and most of them are used far less often than this will be. Seems a shame to make people type out so many characters for such a commonly used feature.

I understand there may be Very Good Reasons (consistency, etc.) for not doing this, but I'm making a case from an all-things-considered, ergonomics perspective here - still may not be enough to outweigh the consistency/etc reasons, of course - just looking for comments here.

(That said, we somehow settled on document.querySelector to replace jQuery's $, so maybe we should stick to the "webby" pattern of giving the longest names to the most commonly used features 🥲)

@Pauan
Copy link

Pauan commented Feb 16, 2023

That said, we somehow settled on document.querySelector to replace jQuery's $

querySelector is a method because it works on any element, not just the document.

@josephrocca
Copy link

josephrocca commented Feb 16, 2023

(sure, just comparing querySelector to $ - and in general the pattern of very wordy names in JS compared to many other languages, but don't want to derail/digress to far on this point. it's kind of nice to have very explicit names like addEventListener, but I do envy the brevity and ergonomics of some other languages in comparison to JS)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addition/proposal New features or enhancements needs implementer interest Moving the issue forward requires implementers to express interest
Development

Successfully merging a pull request may close this issue.