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

push*(repeater) type API #57

Open
jedwards1211 opened this issue Mar 30, 2020 · 3 comments
Open

push*(repeater) type API #57

jedwards1211 opened this issue Mar 30, 2020 · 3 comments

Comments

@jedwards1211
Copy link
Contributor

jedwards1211 commented Mar 30, 2020

One of the kind of cool things about zen-observable is you can

new Observable(observer => {
  checkSomethingSynchronously()
  return someOtherObservable.subscribe(observer)
})

I'm looking for an elegant way to do this with repeater. Seems like right now I'd have to use for..await:

new Repeater(async (push, stop) => {
  await checkSomethingAsynchronously()
  for await (const x of someOtherRepeater) push(x)
})

The syntax isn't bad really, but didn't we come to the conclusion awhile back that using for..await like this is dangerous? I can never remember the caveats exactly.

@brainkim
Copy link
Member

brainkim commented Mar 30, 2020

Hmmm. So first things first: your use-case is probably best served by an async generator and not a repeater:

async function *gen() {
  await checkSomethingAsynchronously();
  yield *someOtherRepeater;
} 

Think of it like the Promise constructor: if you have a promise already then using the Promise constructor is probably a mistake. Same goes for repeaters: if you have an async iterator, you should probably be using an async generator and not a repeater. Choosing between the two can be tricky and I’ve thought about making a flow-chart for the different use-cases. It really depends on the sources that you’re pulling from. If they’re callback-based, use repeaters obviously. If they’re promises or async iterators, then it depends on how complicated the pulling algorithm is. If you’re simply pulling in series, then async generators and yield/yield* work. If you’re pulling from them concurrently, then repeaters can help.

didn't we come to the conclusion awhile back that using for..await like this is dangerous

The problem was with using async generators and for...await. Calling return on an async generator object paused at the top of a for...await loop wouldn’t work until the loop resumed, potentially causing deadlocks. The same would be true with a Repeater, but you could mitigate this issue by racing iteration with the stop promise.

@jedwards1211
Copy link
Contributor Author

jedwards1211 commented Mar 30, 2020

Oh yeah, I didn't think of just using an async generator.

Calling return on an async generator object paused at the top of a for...await loop wouldn’t work until the loop resumed, potentially causing deadlocks.

I wonder if this is the case for yield * also though?

@brainkim
Copy link
Member

I think I checked and found that yield * propagates return into the inner iterator immediately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants