Skip to content
This repository has been archived by the owner on Oct 17, 2024. It is now read-only.

Make StreamQueue start listening immediately to broadcast streams. #119

Merged
merged 2 commits into from
Jun 30, 2020

Conversation

lrhn
Copy link
Contributor

@lrhn lrhn commented Jun 29, 2020

When a StreamQueue is created for a broadcast stream, it didn't use to listen to the stream immediately, which means that any events sent before the first call to next (or any other queue request) would be silently lost. That makes an initial peek operation change the behavior of the queue, which is confusing to users.

lrhn added 2 commits June 29, 2020 11:31
When a `StreamQueue` is created for a broadcast stream, it didn't use to listen to the stream immediately, which means that any events sent before the first call to `next` (or any other queue request) would be silently lost. That makes an initial `peek` operation change the behavior of the queue, which is confusing to users.
@lrhn lrhn requested a review from natebosch June 29, 2020 12:03
@natebosch
Copy link
Contributor

So far I've only found one use case that this breaks (a test was mocking a Stream and didn't implement isBroadcast).

I think this is the right thing to do. I do wonder whether we should roll this out before or after NNBD and whether we should model it as a breaking change.

My gut reaction is to treat this as non-breaking and try to get it out ASAP. @jakemac53 - do you have any thoughts?

I'll try to run more tests and see if anything more scare comes up.

@jakemac53
Copy link
Contributor

Ya, I would say run it through an internal presubmit and as long as that seems reasonable to land then :shipit:

@natebosch natebosch merged commit e0c41e0 into master Jun 30, 2020
@natebosch natebosch deleted the lrhn-streamqueue-startimmediately branch June 30, 2020 19:31
mosuem pushed a commit to dart-lang/core that referenced this pull request Oct 14, 2024
…art-archive/async#119)

When a `StreamQueue` is created for a broadcast stream, it didn't use to listen to the stream immediately, which means that any events sent before the first call to `next` (or any other queue request) would be silently lost. That makes an initial `peek` operation change the behavior of the queue, which is confusing to users.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

Successfully merging this pull request may close these issues.

4 participants