-
Notifications
You must be signed in to change notification settings - Fork 205
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
Adapt timers to async iterables by cooling #3949
Conversation
@@ -1,49 +1,75 @@ | |||
// @ts-check |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note to reviewers:
Aside from this @ts-check
, all the rest of the changes to this file are only whitespace. (Too bad GitHub's "Hide whitespace changes" doesn't really.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aside from this
@ts-check
, all the rest of the changes to this file are only whitespace. (Too bad GitHub's "Hide whitespace changes" doesn't really.)
Ah, I see why. The whitespace changes also inserted *
in various places, which defeats Github's algorithm.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow, this is really beautiful!
I love how you've factored things, and it's a great example of what I need in order to cool the vat-bank.js
balance notifiers.
And I tested that it even works, too!
@@ -1,49 +1,75 @@ | |||
// @ts-check |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aside from this
@ts-check
, all the rest of the changes to this file are only whitespace. (Too bad GitHub's "Hide whitespace changes" doesn't really.)
Ah, I see why. The whitespace changes also inserted *
in various places, which defeats Github's algorithm.
7d10721
to
e5f675b
Compare
Stacked on #3945
Proposed as an alternative to #3854
Hi @michaelfig it turns out there already was a makeNotifierFromAsyncIterable adaptor, which sounds like it was the remaining piece we needed. I believe I wrote it long ago. However, armed with your distinction, I saw that it was hot. I rewrote it to be a coldness-preserving adaptor and then used it to make the timer repair we talked about.