-
Notifications
You must be signed in to change notification settings - Fork 277
add ValueStream.isReplayValueStream
#784
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
base: master
Are you sure you want to change the base?
Conversation
…eplayValueStream # Conflicts: # examples/flutter/github_search/lib/search_screen.dart # examples/flutter/github_search/pubspec.lock
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.
Pull Request Overview
This pull request introduces the isReplayValueStream
property to the ValueStream
interface to indicate whether a stream replays its last emitted event to new subscribers. This enables more flexible stream handling and allows Flutter widgets to dynamically determine stream behavior instead of relying on hardcoded defaults.
- Adds
isReplayValueStream
property toValueStream
interface and implementations - Updates Flutter widgets to use the new property instead of hardcoded
true
defaults - Refactors example app to use updated RxDart API
Reviewed Changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
packages/rxdart/lib/src/streams/value_stream.dart | Adds isReplayValueStream property to ValueStream interface |
packages/rxdart/lib/src/subjects/behavior_subject.dart | Implements isReplayValueStream in BehaviorSubject classes |
packages/rxdart/lib/src/streams/connectable_stream.dart | Implements isReplayValueStream in ValueConnectableStream |
packages/rxdart_flutter/lib/src/value_stream_*.dart | Updates Flutter widgets to use dynamic stream property |
packages/rxdart_flutter/test/src/rxdart_ext/value_subject.dart | Implements property in test helper classes |
examples/flutter/github_search/lib/bloc/search_bloc.dart | Updates example to use new RxDart API |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
_ambiguate(WidgetsBinding.instance)!.addPostFrameCallback((_) { | ||
if (widget.stream != stream) { | ||
return; | ||
} | ||
_notifyListener(stream.value); | ||
_subscribeIfNeeded(stream); |
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.
This condition uses widget.stream
instead of _currentStream
which could cause incorrect behavior. The condition should check if the current stream has changed since the callback was scheduled.
_ambiguate(WidgetsBinding.instance)!.addPostFrameCallback((_) { | |
if (widget.stream != stream) { | |
return; | |
} | |
_notifyListener(stream.value); | |
_subscribeIfNeeded(stream); | |
final scheduledStream = stream; | |
_ambiguate(WidgetsBinding.instance)!.addPostFrameCallback((_) { | |
if (widget.stream != scheduledStream) { | |
return; | |
} | |
_notifyListener(scheduledStream.value); | |
_subscribeIfNeeded(scheduledStream); |
Copilot uses AI. Check for mistakes.
void _subscribeIfNeeded(Stream<T> streamToListen) { | ||
if (_subscription != null) { | ||
return; | ||
} |
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.
The method name _subscribeIfNeeded
is misleading because it only checks if _subscription
is null, but doesn't verify if the subscription is for the correct stream. This could lead to using an outdated subscription when the stream changes.
Copilot uses AI. Check for mistakes.
This pull request includes significant updates to the
examples/flutter/github_search
andpackages/rxdart
directories, focusing on stream handling and improving theValueStream
functionality. The most important changes include updating dependencies, modifying stream classes and widgets, and adding new properties to enhance stream behavior.Updates to
examples/flutter/github_search
:examples/flutter/github_search/lib/bloc/search_bloc.dart
: ReplacedStateStream
withValueStream
and updated thepublishState
method topublishValueSeeded
. [1] [2]examples/flutter/github_search/pubspec.yaml
: Removedrxdart_ext
dependency.Enhancements to
packages/rxdart
:packages/rxdart/lib/src/streams/connectable_stream.dart
: AddedisReplayValueStream
property toValueConnectableStream
.packages/rxdart/lib/src/streams/replay_stream.dart
: UpdatedReplayStream
documentation to reflect replay behavior.packages/rxdart/lib/src/streams/value_stream.dart
: IntroducedisReplayValueStream
property toValueStream
interface.packages/rxdart/lib/src/subjects/behavior_subject.dart
: ImplementedisReplayValueStream
inBehaviorSubject
and_BehaviorSubjectStream
. [1] [2]Improvements to
packages/rxdart_flutter
:packages/rxdart_flutter/lib/src/value_stream_builder.dart
: UpdatedValueStreamBuilder
to useisReplayValueStream
property and modified its default behavior. [1] [2] [3]packages/rxdart_flutter/lib/src/value_stream_consumer.dart
: Similar updates toValueStreamConsumer
to handleisReplayValueStream
. [1] [2] [3]packages/rxdart_flutter/lib/src/value_stream_listener.dart
: EnhancedValueStreamListener
withisReplayValueStream
property and refactored subscription logic. [1] [2] [3] [4] [5]Additional changes:
packages/rxdart/test/subject/behavior_subject_test.dart
: Added tests forisReplayValueStream
property inBehaviorSubject
.packages/rxdart_flutter/test/src/rxdart_ext/value_subject.dart
: ImplementedisReplayValueStream
inValueSubject
and_ValueSubjectStream
. [1] [2]