Add pause mechanism #3428
Add pause mechanism #3428
Conversation
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #3428 +/- ##
==========================================
+ Coverage 94.35% 99.15% +4.80%
==========================================
Files 43 5 -38
Lines 2339 238 -2101
==========================================
- Hits 2207 236 -1971
+ Misses 132 2 -130
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe changes in this pull request primarily enhance the Flutter Riverpod package by introducing mechanisms to pause provider subscriptions when widgets are not visible, thereby optimizing performance. Significant modifications include the removal of certain subclasses of Changes
Assessment against linked issues
Suggested reviewersPoem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@coderabbitai review |
Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (7)
packages/riverpod/lib/src/core/modifiers/select_async.dart (2)
35-41: Clarify the purpose and usage of theweakparameter.The
addListenermethod signature has been updated to include a new required parameterweak, but it is not being used within the method body. Please provide more context on the intended purpose and usage of this parameter.
150-151: Provide more context on the change in subscription type.The return statement has been modified to instantiate a
SelectorSubscriptioninstead of_SelectorSubscription. Please provide more information about the implications of this change and its impact on the broader codebase.packages/riverpod_generator/integration/build_yaml/lib/dependencies.g.dart (1)
184-184: Ignore comments are acceptable, but review their necessity periodically.The ignore comments are used to suppress specific linter warnings and static analysis errors, which may be necessary for the generated code to compile and function correctly. However, it's important to periodically review these ignore comments to ensure they are still relevant and necessary.
Consider reviewing the ignored warnings and errors to see if they can be resolved without the need for suppression. If the warnings or errors are no longer applicable or can be fixed, remove the corresponding ignore comments to maintain code quality and adherence to best practices.
packages/riverpod/lib/src/core/element.dart (2)
97-100: Improved testability, but use caution.Annotating the
subscriptionsanddependentsproperties with@visibleForTestingimproves testability by allowing direct interaction with these properties in test code. However, be cautious about over-exposing internal state, as it can lead to brittle tests that rely heavily on implementation details. Consider limiting the exposure to only what's necessary for effective testing.
107-108: Improved clarity with separate weak dependents list.Introducing the
weakDependentsproperty to track "listen(weak: true)" subscriptions separately enhances clarity and allows for targeted handling of these subscriptions. This separation of concerns is a positive change.However, as mentioned earlier, be mindful of over-exposing internal state through the
@visibleForTestingannotation. Ensure that the exposed properties are truly necessary for effective testing and consider alternative approaches if possible.packages/riverpod/CHANGELOG.md (2)
18-22: Clarify the description ofref.invalidate.The current description doesn't clearly convey how
ref.invalidatediffers fromref.refresh. Consider elaborating on the exact behavior ofref.invalidateand when it's preferable toref.refresh.
32-33: Mention the specific bugs fixed by the memory leak and provider disposal changes.To help users better understand the impact of these fixes, consider mentioning the symptoms of the bugs that were resolved. This can help users determine if they were affected by these issues.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (38)
- packages/flutter_riverpod/CHANGELOG.md (1 hunks)
- packages/flutter_riverpod/lib/src/core/consumer.dart (6 hunks)
- packages/flutter_riverpod/test/consumer_listen_test.dart (3 hunks)
- packages/flutter_riverpod/test/src/core/consumer_test.dart (7 hunks)
- packages/riverpod/CHANGELOG.md (2 hunks)
- packages/riverpod/lib/riverpod.dart (1 hunks)
- packages/riverpod/lib/src/core/element.dart (14 hunks)
- packages/riverpod/lib/src/core/foundation.dart (1 hunks)
- packages/riverpod/lib/src/core/modifiers/future.dart (8 hunks)
- packages/riverpod/lib/src/core/modifiers/select.dart (5 hunks)
- packages/riverpod/lib/src/core/modifiers/select_async.dart (3 hunks)
- packages/riverpod/lib/src/core/provider/provider.dart (2 hunks)
- packages/riverpod/lib/src/core/provider_container.dart (3 hunks)
- packages/riverpod/lib/src/core/provider_subscription.dart (1 hunks)
- packages/riverpod/lib/src/core/proxy_provider_listenable.dart (6 hunks)
- packages/riverpod/lib/src/core/ref.dart (5 hunks)
- packages/riverpod/lib/src/core/scheduler.dart (1 hunks)
- packages/riverpod/test/old/framework/auto_dispose_test.dart (1 hunks)
- packages/riverpod/test/old/framework/ref_watch_test.dart (5 hunks)
- packages/riverpod/test/old/legacy/framework2/framework_test.dart (6 hunks)
- packages/riverpod/test/old/legacy/framework_test.dart (1 hunks)
- packages/riverpod/test/old/legacy_providers/stream_provider/auto_dispose_family_stream_provider_test.dart (2 hunks)
- packages/riverpod/test/old/legacy_providers/stream_provider/auto_dispose_stream_provider_test.dart (3 hunks)
- packages/riverpod/test/old/legacy_providers/stream_provider/stream_provider_family_test.dart (4 hunks)
- packages/riverpod/test/old/legacy_providers/stream_provider/stream_provider_test.dart (26 hunks)
- packages/riverpod/test/old/utils.dart (1 hunks)
- packages/riverpod/test/src/core/modifiers/future_test.dart (2 hunks)
- packages/riverpod/test/src/core/modifiers/select_async_test.dart (7 hunks)
- packages/riverpod/test/src/core/modifiers/select_test.dart (2 hunks)
- packages/riverpod/test/src/core/provider_container_test.dart (3 hunks)
- packages/riverpod/test/src/core/provider_element_test.dart (7 hunks)
- packages/riverpod/test/src/core/provider_subscription_test.dart (1 hunks)
- packages/riverpod/test/src/core/proxy_provider_listenable_test.dart (1 hunks)
- packages/riverpod/test/src/core/ref_test.dart (23 hunks)
- packages/riverpod/test/src/providers/stream_notifier_test.dart (3 hunks)
- packages/riverpod/test/src/utils.dart (2 hunks)
- packages/riverpod_generator/integration/build_yaml/lib/dependencies.g.dart (1 hunks)
- packages/riverpod_lint_flutter_test/test/lints/avoid_build_context_in_providers.g.dart (0 hunks)
Files not reviewed due to no reviewable changes (1)
- packages/riverpod_lint_flutter_test/test/lints/avoid_build_context_in_providers.g.dart
Files skipped from review due to trivial changes (1)
- packages/riverpod/test/old/framework/auto_dispose_test.dart
Additional comments not posted (144)
packages/riverpod/test/src/core/proxy_provider_listenable_test.dart (1)
6-26: Test case aligns with PR objectives and correctly verifies the pause/resume functionality.The test case
handles pause/resumeeffectively tests the desired functionality of pausing and resuming subscriptions to a provider. It ensures that the provider element's active status is correctly updated based on the subscription state.The test case follows a clear and logical structure:
- Initialize the necessary components (container, provider, element, subscription).
- Check the initial active status of the element (expected to be true).
- Pause the subscription and verify that the element's active status changes to false.
- Resume the subscription and verify that the element's active status is restored to true.
This test case provides confidence that the implemented pause mechanism works as intended and aligns with the PR objectives of optimizing performance by preventing unnecessary rebuilds of widgets when the provider is not actively being listened to.
packages/riverpod/lib/riverpod.dart (2)
36-36: Approve the addition ofAsyncSubscriptionto the exports.The addition of
AsyncSubscriptionaligns with the objective of enhancing subscription management capabilities in the provider framework. It is likely to provide more control over asynchronous subscriptions and improve performance when used in conjunction with other newly added subscription-related types.
40-44: Approve the addition of subscription-related types to the exports.The introduction of
ProviderStateSubscription,ProviderSubscriptionImpl,ProviderSubscriptionWithOrigin,SelectorSubscription, andDelegatingProviderSubscriptionaligns with the objective of refining the subscription management architecture in the provider framework. These types are likely to provide more nuanced control over provider state and lifecycle events, allowing developers to fine-tune subscription behavior based on their specific requirements.The changes may contribute to improved performance by optimizing subscription handling and reducing unnecessary rebuilds. They enhance the flexibility and granularity of subscription management, making the provider framework more versatile and efficient.
packages/riverpod/test/src/core/modifiers/future_test.dart (1)
Line range hint
10-30: LGTM!The change in the test case from checking
hasListenerstoweakDependentsis a good refinement. It shifts the emphasis from general listener management to a more specific aspect of dependency tracking, which improves the accuracy of the test in relation to the provider's behavior when weak references are involved.packages/riverpod/test/old/legacy_providers/stream_provider/auto_dispose_family_stream_provider_test.dart (2)
30-30: LGTM!Adding a listener to the provider allows the test to be notified when the provider's state changes. The empty listener callback doesn't affect the behavior of the test.
105-105: LGTM!Adding a listener to the provider allows the test to be notified when the provider's state changes. The empty listener callback doesn't affect the behavior of the test.
packages/riverpod/test/old/legacy_providers/stream_provider/stream_provider_family_test.dart (1)
27-27: LGTM!The addition of
container.listenenhances the test's ability to monitor and react to changes in the provider's state, improving the robustness of the test case.packages/riverpod/lib/src/core/provider/provider.dart (3)
64-64: LGTM!The assertion update aligns with the parameter changes and ensures the correct behavior when using
fireImmediatelywith weak listeners.
70-72: Clarify the purpose and impact of the changes.The modifications to the
readProviderElementmethod call and the introduction of the conditional flush operation raise a few questions:
- What is the significance of passing the
sourceparameter toreadProviderElement? How does it affect the way the provider element is accessed and managed?- What is the rationale behind flushing the element for non-weak listeners? What are the performance and consistency implications of this change?
Providing more context about these changes would help in understanding their purpose and assessing their potential impact on the system. Could you please elaborate on the motivations behind these modifications and how they align with the overall goals of the refactoring effort?
82-89: Verify the subscription usage and lifecycle management.The modifications to the subscription creation, including the switch to
ProviderStateSubscriptionand the inclusion of thesourceandweakparameters, seem to be part of the broader refactoring effort to improve subscription management.To ensure the correctness and efficiency of these changes, please verify the following:
- The usage of the
ProviderStateSubscriptionis consistent throughout the codebase and aligns with the intended subscription management strategy.- The
sourceandweakparameters are properly utilized within the subscription lifecycle, including creation, updates, and disposal.- The performance and memory impact of these changes are carefully assessed, especially in scenarios with a large number of subscriptions.
You can use the following script to locate the usage of the
ProviderStateSubscription:Verification successful
To gather more comprehensive information about the ProviderStateSubscription and related classes, we need to run additional scripts. Let's continue our verification process with the following scripts:
These scripts will help us gather more detailed information about the ProviderStateSubscription implementation and its related classes, allowing us to make a more informed decision about the consistency and impact of the changes.
It seems our previous commands encountered some issues. Let's modify our approach and try again with simplified commands to gather the necessary information. We'll split the commands and use simpler patterns to ensure we get the required output.
These modified commands should help us gather the information we need about ProviderStateSubscription and its related classes without encountering the previous errors. Once we have this information, we'll be able to make a more informed decision about the consistency and impact of the changes.
It seems we've encountered some limitations with the 'ast-grep' command in our environment. However, we've gathered some useful information about ProviderStateSubscription. Let's continue our investigation using 'rg' to find the remaining details we need. We'll create new scripts to search for ProviderSubscriptionImpl, _OnPauseMixin, and try to locate the usage of 'source' and 'weak' parameters.
These new commands should help us gather the remaining information we need about ProviderStateSubscription, its related classes, and the usage of 'source' and 'weak' parameters. Once we have this information, we'll be able to make a more informed decision about the consistency and impact of the changes.
Verification complete: ProviderStateSubscription implementation is consistent
The changes to ProviderStateSubscription, including the use of 'source' and 'weak' parameters, are consistent with the broader refactoring effort. The class hierarchy and mixin usage align with the modifications observed in the review comment.
Key findings:
- ProviderStateSubscription extends ProviderSubscriptionImpl and mixes in _OnPauseMixin, maintaining the expected structure.
- The constructor includes 'source' and 'weak' as required parameters, as seen in the review comment.
While we couldn't find specific usage of 'source' and 'weak' within the ProviderStateSubscription class, they are properly included in the constructor. Their usage might be in the parent class or passed to other methods.
Recommendations:
- Manually verify the usage of 'source' and 'weak' parameters in the parent class (ProviderSubscriptionImpl) or any methods called within the constructor.
- Ensure that the performance impact of these changes has been considered, especially in scenarios with many subscriptions.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the `ProviderStateSubscription`. # Test: Search for the class usage. Expect: Consistent usage throughout the codebase. rg --type dart -A 5 $'ProviderStateSubscription'Length of output: 1786
Script:
#!/bin/bash # Description: Further investigate ProviderStateSubscription and related classes # Search for any other usages of ProviderStateSubscription rg --type dart -A 3 'ProviderStateSubscription(?!<)' # Investigate ProviderSubscriptionImpl class ast-grep --lang dart --pattern 'class ProviderSubscriptionImpl<$_, $_> { $$$ }' # Look into _OnPauseMixin ast-grep --lang dart --pattern 'mixin _OnPauseMixin { $$$ }' # Check usage of 'source' and 'weak' parameters in ProviderStateSubscription ast-grep --lang dart --pattern 'class ProviderStateSubscription<$_> { $$$ final $_ source; final bool weak; $$$ }'Length of output: 587
Script:
#!/bin/bash # Description: Retry investigation of ProviderStateSubscription and related classes # Search for usages of ProviderStateSubscription rg --type dart -A 3 'ProviderStateSubscription' # Investigate ProviderSubscriptionImpl class ast-grep --lang dart --pattern 'class ProviderSubscriptionImpl' # Look into _OnPauseMixin ast-grep --lang dart --pattern 'mixin _OnPauseMixin' # Check ProviderStateSubscription class definition ast-grep --lang dart --pattern 'class ProviderStateSubscription' # Search for 'source' usage in ProviderStateSubscription rg --type dart -A 3 'ProviderStateSubscription.*source' # Search for 'weak' usage in ProviderStateSubscription rg --type dart -A 3 'ProviderStateSubscription.*weak'Length of output: 2609
Script:
#!/bin/bash # Description: Further investigation of ProviderStateSubscription and related classes # Search for ProviderSubscriptionImpl class definition rg --type dart -A 5 'class ProviderSubscriptionImpl' # Search for _OnPauseMixin definition rg --type dart -A 5 'mixin _OnPauseMixin' # Search for 'source' usage in ProviderStateSubscription (case-insensitive) rg --type dart -i -A 3 'ProviderStateSubscription.*source' # Search for 'weak' usage in ProviderStateSubscription (case-insensitive) rg --type dart -i -A 3 'ProviderStateSubscription.*weak' # Search for methods or properties in ProviderStateSubscription rg --type dart -A 10 'class ProviderStateSubscription'Length of output: 2321
packages/riverpod/lib/src/core/proxy_provider_listenable.dart (5)
3-4: LGTM!The changes to the
_ProxySubscriptionclass introduce more flexibility in type handling by accepting two type parameters,OutTandStateT. ExtendingDelegatingProviderSubscriptionwith these type parameters ensures proper delegation and type safety.
6-12: LGTM!The changes to the
innerSubscriptionproperty and thereadmethod align with the new type parameters introduced in the class. UsingProviderSubscriptionWithOriginforinnerSubscriptionenhances its functionality and type safety. Updating thereadmethod to returnOutTprovides more flexibility in the returned value and ensures consistency with the class's type parameters.
Line range hint
15-31: LGTM!The changes to the
_readproperty and theclosemethod are consistent with the new type parameters and improve the class's functionality. Updating_readto returnOutTaligns with thereadmethod and provides consistency. The added check in theclosemethod prevents unnecessary work by ensuring that the subscription is not already closed before removing listeners and callingsuper.close().
Line range hint
52-140: LGTM!The changes to the
ProviderElementProxyclass align with the updates made to the_ProxySubscriptionclass, ensuring consistency in type handling. Updating the class to useStateTandOutTtype parameters instead ofInputTandOutputTimproves clarity and consistency.The modifications to the
addListenermethod, including returningProviderSubscriptionWithOrigin<OutT, StateT>and accepting aNode sourceparameter, enhance its functionality and type safety. The refined internal logic for reading values, including the updated_readmethod, leverages the new type parameters effectively and improves the overall functionality of the class.These changes contribute to better type safety, consistency, and functionality within the provider subscription mechanism.
Line range hint
1-144: Overall assessment: The changes are well-structured, consistent, and improve the provider subscription mechanism.The modifications made to the
_ProxySubscriptionandProviderElementProxyclasses are consistent and aligned with each other. The introduction of the new type parametersOutTandStateTenhances type safety and flexibility throughout the file.The changes to the
addListenermethod, including returningProviderSubscriptionWithOrigin<OutT, StateT>and accepting aNode sourceparameter, improve its functionality and type safety. The refined internal logic for reading values, including the updated_readmethod, leverages the new type parameters effectively and contributes to a more robust and efficient provider subscription mechanism.Overall, the changes are well-structured, maintain consistency between the two classes, and utilize the new type parameters appropriately. These modifications enhance type safety, flexibility, and functionality, resulting in a more reliable and efficient provider subscription mechanism.
packages/riverpod/test/src/core/provider_subscription_test.dart (6)
10-27: LGTM!The test correctly verifies the pause and resume functionality of a subscription. It checks the active state of the provider element before and after pausing/resuming the subscription, ensuring the expected behavior.
29-47: LGTM!The test correctly verifies that closing a paused subscription unpauses the provider element. It checks the active state of the provider element before and after pausing the subscription, closes the subscription, listens to the provider again, and ensures that the provider element is active.
52-65: LGTM!The test correctly verifies that resuming a paused subscription with no missed data events does not call listeners. It creates a container, a provider, and a listener, pauses and resumes the subscription, and ensures that the listener is not called using
verifyZeroInteractions.
67-91: LGTM!The test correctly verifies that resuming a paused subscription with missed data emits the last event. It creates a container, a provider, and a listener, pauses the subscription, updates the notifier state twice, resumes the subscription, and ensures that the listener is called with the last emitted value using
verifyOnly. It also resumes the subscription again and verifies that the listener is not called anymore usingverifyNoMoreInteractions.
93-132: LGTM!The test correctly verifies that resuming a paused subscription with a missed error emits the last error. It creates a container, a provider that throws an error, a listener, and an error listener, pauses the subscription, updates the error to be thrown, resumes the subscription, and ensures that the error listener is called with the last error using
verifyOnly. It also resumes the subscription again and verifies that the error listener and the listener are not called anymore usingverifyNoMoreInteractionsandverifyZeroInteractions.
134-150: LGTM!The test correctly verifies that the
resumemethod needs to be called as many times as thepausemethod was called to fully unpause the subscription. It creates a container and a provider, listens to the provider, pauses the subscription three times, resumes it twice, checking theisPausedstate after each resume, and finally resumes it one more time to ensure thatisPausedis false.packages/riverpod/lib/src/core/scheduler.dart (2)
136-138: Approve the change in the dispose loop condition.The modification to the dispose loop condition, which now checks for the presence of non-weak listeners (
element.hasNonWeakListeners) instead of the general active state (element.isActive), appears to be a reasonable change.This shift in focus suggests a more targeted approach to determining when provider states are eligible for disposal. By specifically checking for non-weak listeners, the code can potentially optimize resource management by disposing of provider states that only have weak listeners, while preserving those with active non-weak listeners.
This change aligns with the overall objective of refining the provider lifecycle management logic.
140-145: Approve the refined logic for disposing of provider states.The changes made to the logic for disposing of provider states demonstrate a more nuanced approach to resource management based on the presence of weak dependents.
The code now checks specifically if the
weakDependentslist is empty. If it is, the provider is disposed usingelement.container._disposeProvider(element.origin), indicating that there are no weak listeners and the resources can be safely released.However, if the
weakDependentslist is not empty, instead of disposing the element, the state is cleared usingelement.clearState(). This suggests that weak listeners are still present, and clearing the state allows for potential reuse of the provider if it becomes active again.This refined approach helps optimize resource management while considering the presence of weak dependents, aligning with the goal of improving provider lifecycle management.
packages/riverpod/lib/src/core/modifiers/select.dart (4)
Line range hint
92-105: LGTM!The changes in this code segment align with the refactoring of the listener management process. Passing the
weakandonDependencyMayHaveChangedparameters to theprovider.addListenermethod ensures consistency and proper handling of dependency changes.
109-109: Conditional assignment oflastSelectedValuelooks good!The assignment of
lastSelectedValuebased on the value of theweakparameter and the result ofsub.readsuggests a change in the initialization logic. This change may affect the behavior of code that relies onlastSelectedValue, so it is important to ensure that the_selectmethod andResult.guardare implemented correctly to avoid any unintended consequences.To verify the correctness of the
_selectmethod andResult.guard, please review their implementations and test scenarios whereweakis both true and false. Ensure that the expected behavior is observed in each case.
Line range hint
119-172: Introduction ofSelectorSubscriptionclass looks good!The
SelectorSubscriptionclass appears to be a well-designed replacement for the previous subscription handling mechanism. By extendingDelegatingProviderSubscriptionand encapsulating the_readfunction,onClosecallback, andinnerSubscription, the class promotes maintainability and reduces coupling.The updated
readmethod, which flushes theinnerSubscriptionbefore returning the result of the_readfunction, ensures that the latest state is always returned. However, this could have performance implications if thereadmethod is called frequently.To ensure a smooth transition to the new
SelectorSubscriptionclass, please verify the following:
- Update any code that interacts with subscriptions to accommodate the new
SelectorSubscriptionclass and its constructor parameters.- Review the usage of the
readmethod and assess the performance impact of flushing theinnerSubscriptionon each call. Consider optimizing the implementation if necessary.- Test the
SelectorSubscriptionclass thoroughly to ensure that it behaves as expected and handles edge cases correctly.
81-87: Refactoring of theaddListenermethod looks good!The changes to the
addListenermethod signature, including the addition of theweakparameter and the updated return type, suggest a refactoring of the listener management process. This refactoring may have implications for code that calls this method, requiring updates to accommodate the new parameter and return type.To ensure a smooth transition, please verify that all calls to the
addListenermethod have been updated to provide theweakparameter and handle the newSelectorSubscriptionreturn type. You can use the following script to search for potential usage sites:Verification successful
To gather more information about the impact of the
addListenermethod changes, we need to examine compilation errors, the implementation of new return types, and any migration documentation. Let's run the following scripts:
To gather more comprehensive information about the
addListenerchanges and their impact, let's run the following scripts:
Changes to
addListenermethod have been successfully implementedThe refactoring of the
addListenermethod, including the addition of theweakparameter and the updated return types, has been thoroughly implemented across the codebase. Here are the key findings:
- The
weakparameter is now widely used inaddListenercalls, particularly in test files.- The
SelectorSubscriptionandProviderSubscriptionWithOriginclasses have been properly implemented to support the new functionality.- Recent commits show intentional changes related to
addListenerand theweakparameter.- The changes have been extensively tested, as evidenced by updates in test files.
These observations indicate that the refactoring has been carried out consistently and comprehensively throughout the project.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for usage of the `addListener` method. # Test: Search for calls to the `addListener` method. Expect: Occurrences with the updated signature. rg --type dart -A 5 $'addListener\\('Length of output: 12629
Script:
#!/bin/bash # Check for compilation errors or warnings related to addListener rg --type dart -e "addListener.*weak" -e "error.*addListener" -e "warning.*addListener" # Examine the implementation of SelectorSubscription and ProviderSubscriptionWithOrigin ast-grep --lang dart --pattern 'class SelectorSubscription { $$$ }' ast-grep --lang dart --pattern 'class ProviderSubscriptionWithOrigin { $$$ }' # Look for migration guides or documentation updates rg --type md -e "addListener.*weak" -e "migration.*addListener"Length of output: 305
Script:
#!/bin/bash # Search for any usage of addListener rg --type dart -e "addListener" -C 3 # Look for recent changes related to addListener or weak parameter git log -n 5 --pretty=format:"%h - %s" --grep="addListener\|weak" # Search for SelectorSubscription and ProviderSubscriptionWithOrigin classes rg --type dart -e "class SelectorSubscription" -e "class ProviderSubscriptionWithOrigin" -C 3 # Search for weak parameter usage rg --type dart -e "weak:\s*\w+" -C 3Length of output: 40464
packages/riverpod/lib/src/core/modifiers/select_async.dart (3)
135-139: LGTM!The change from
node.listentoprovider.addListenerenhances the clarity and maintainability of the code by refactoring how the listener is registered. Theweakparameter is now being passed to theprovider.addListenermethod, suggesting that it is intended to be used for listener registration.
156-163: LGTM!The addition of a new listener registration for the
futurewithin theonCloseclosure of theSelectorSubscriptionsuggests an improvement in the handling of asynchronous operations and error management. Theweakparameter is being passed to thefuture.addListenermethod, indicating that it is intended to be used for listener registration.
165-169: LGTM!The chaining of asynchronous operations using the
futureobtained from the new listener registration suggests an improvement in the handling of asynchronous operations and error management. The use of the_selectmethod and thecompleter.completemethod indicates that the code segment is responsible for processing the value obtained from thefutureand completing the associated completer. The use of thecompleter.completeErrormethod suggests that any errors encountered during the asynchronous operations are being properly handled and propagated.packages/riverpod/test/src/core/modifiers/select_test.dart (3)
56-59: LGTM!The assertion correctly checks if the
weakDependentscollection is not empty after subscribing to the provider withweak: true.
63-66: LGTM!The assertion correctly checks if the
weakDependentscollection is empty after closing the subscription.
163-185: Test case for pause and resume functionality looks good!The test case correctly verifies the behavior of pausing and resuming a subscription. It ensures that:
- Pausing the subscription makes the provider element inactive.
- Resuming the subscription makes the provider element active again.
The assertions accurately check the state of the provider element after each action.
packages/riverpod/test/old/utils.dart (1)
39-48: LGTM!The changes to the
OnDisposeMockclass are well-implemented and improve its usability:
- The optional
labelparameter in the constructor allows for descriptive labeling of mock instances.- Storing the
labelas a final field ensures immutability.- Overriding
toStringto include thelabelenhances debugging by providing informative string representations.The changes maintain backward compatibility and align with the existing code style and conventions.
packages/riverpod/test/old/legacy/framework_test.dart (1)
77-77: LGTM!The change in the expected error type from
StateErrortoCircularDependencyErrorimproves the specificity of the error thrown in case of circular dependencies. This helps in better debugging and understanding the root cause of the error.packages/riverpod_generator/integration/build_yaml/lib/dependencies.g.dart (2)
9-9: LGTM!The
Calc2Reftype alias is a clean and concise way to refer toRef<int>in the code.
12-181: Excellent refactoring and improvements!The changes made to the
Calc2ProviderandCalc2Familyclasses significantly enhance the clarity, flexibility, and maintainability of the provider's implementation. Key improvements include:
- The new constructor in
Calc2Provideraccepts a callback function for creating instances, allowing for greater flexibility in instantiation.- The
createmethod inCalc2Providerhas been updated to utilize the callback, decoupling the creation logic from the provider's core functionality.- The
Calc2Familyclass has been restructured to initialize dependencies and transitive dependencies directly, improving readability and maintainability.- The
callmethod inCalc2Familyhas been simplified to create aCalc2Providerinstance with the provided arguments.- The
overrideWithmethod inCalc2Familyhas been updated to handle the new provider creation approach.Overall, these changes streamline the provider's implementation while preserving its core behavior. Great work!
packages/riverpod/lib/src/core/provider_subscription.dart (7)
3-32: Excellent improvements to theProviderSubscriptionclass!The changes enhance the design and functionality of the subscription system:
- Changing
ProviderSubscriptionto a sealed class improves type safety and encapsulation.- The new
pauseandresumemethods introduce a mechanism to control the subscription state, potentially optimizing performance.- The
closemethod ensures proper cleanup of the subscription, preventing resource leaks.- The change in the
readmethod's return type allows for greater flexibility.Overall, these modifications contribute to a more robust and efficient subscription management system.
34-43: Great addition of theProviderSubscriptionWithOriginclass!The new sealed class extends the functionality of
ProviderSubscriptionby including information about the provider's origin:
- The
originproperty provides access to the associatedProviderBaseinstance, enabling more granular control over the subscription behavior.- The
_listenedElementproperty represents theProviderElementthat listens to the provider, allowing for better tracking and management of the subscription's state.- The
_notifyand_notifyErrormethods handle state updates and errors, respectively, ensuring proper communication between the provider and its subscribers.This class enhances the subscription system's flexibility and maintainability.
45-68: Nice updates to theProviderSubscriptionImplclass!The changes consolidate the subscription implementation and improve resource management:
- Extending
ProviderSubscriptionWithOriginallows the class to inherit its properties and methods, promoting code reuse and consistency.- Mixing in
_OnPauseMixinprovides the pause and resume functionality, enabling better control over the subscription state.- The updated
closemethod ensures that the subscription is properly removed from the listened element's list of dependents when closed, preventing potential issues related to stale subscriptions and improving resource management.These modifications enhance the overall structure and reliability of the subscription system.
70-93: Excellent addition of the_OnPauseMixinmixin!This mixin encapsulates the pause and resume functionality for subscriptions, providing a reusable and consistent implementation:
- The
_isPausedproperty indicates the current pause state of the subscription.- The
_pauseCountproperty enables nested pausing, ensuring that the subscription is only resumed when all pause requests have been fulfilled.- The
pauseandresumemethods handle the pause state transitions and call the respectiveonCancelandonResumemethods for custom behavior.- The
onResumeandonCancelplaceholder methods allow for implementing custom logic when the subscription state changes.The
_OnPauseMixinmixin promotes code reuse and maintainability across different subscription classes.
95-158: Great addition of theDelegatingProviderSubscriptionclass!This abstract base class serves as a foundation for subscriptions that delegate their functionality to an inner subscription:
- It extends
ProviderSubscriptionImpl, inheriting its properties and methods, and overrides relevant properties and methods fromProviderSubscriptionWithOriginandProviderSubscriptionImplto delegate their implementation to the inner subscription.- The
onCancelandonResumemethods are overridden to propagate the pause and resume events to the inner subscription, ensuring consistent behavior.- The
pause,resume, andclosemethods are overridden to delegate the corresponding actions to the inner subscription and perform additional cleanup if necessary, maintaining proper management of the subscription lifecycle.The
DelegatingProviderSubscriptionclass provides a convenient and maintainable way to create subscriptions that wrap and delegate to an inner subscription, promoting code reuse and consistency.
162-245: Excellent improvements to theProviderStateSubscriptionclass!The changes enhance the handling of pause and resume events and ensure that no updates are lost:
- Extending
ProviderSubscriptionImpland mixing in_OnPauseMixinallows the class to inherit their properties and methods, promoting code reuse and consistency.- The introduction of the
_missedCalledproperty enables storing and processing missed events that occur while the subscription is paused, preventing loss of important updates.- The updated
onResumemethod ensures that any missed events are properly handled when the subscription is resumed, maintaining consistency and correctness.- The modifications to the
_notifyand_notifyErrormethods enable the subscription to capture and store missed events during a pause, guaranteeing that no updates are lost.These improvements make the
ProviderStateSubscriptionclass more robust and reliable in handling pause and resume scenarios.
Line range hint
247-262: Nice addition of the_handleFireImmediatelyfunction!This utility function handles the synchronous calling of listeners when using the
fireImmediatelyoption:
- It takes a
Result<StateT>parameter representing the currentpackages/riverpod/test/src/utils.dart (4)
23-51: LGTM!The
StreamSubscriptionViewclass provides a clean and extensible way to wrap an existingStreamSubscriptionand delegate method calls to it. This allows for additional functionality to be implemented in subclasses without modifying the original subscription directly.The implementation is straightforward and follows the
StreamSubscriptioninterface correctly.
53-82: LGTM!The
_DelegatingStreamSubscriptionclass extendsStreamSubscriptionViewand introduces useful hooks for subscription lifecycle events. The optional callback functions (onSubscriptionPause,onSubscriptionResume,onSubscriptionCancel) allow for custom actions to be executed when a subscription is paused, resumed, or canceled.The overridden methods correctly invoke the corresponding callback functions before calling the super method, ensuring that the additional logic is executed at the appropriate times.
The implementation is clean and follows the expected behavior of the
StreamSubscriptioninterface.
84-115: LGTM!The
DelegatingStreamclass extendsStreamViewand integrates the new subscription management features into the stream listening process. By overriding thelistenmethod to return an instance of_DelegatingStreamSubscription, it allows for custom actions to be executed during subscription lifecycle events when listening to a stream.The implementation is straightforward and correctly passes the original stream and the optional callback functions to the
_DelegatingStreamSubscriptionconstructor.The class provides a seamless way to incorporate the enhanced subscription functionality into the existing stream handling mechanism.
162-164: LGTM!The
OnPauseclass is a simple mock class that can be used in testing scenarios to simulate and verify the pause behavior of streams. By extending theMockclass from themockitopackage and defining acall()method without any implementation, it allows for asserting that the pause event is triggered at the expected times during the execution of tests.The class is straightforward and serves its purpose as a mock for testing the pause functionality of streams.
packages/riverpod/test/old/legacy_providers/stream_provider/auto_dispose_stream_provider_test.dart (3)
69-70: LGTM!The addition of the listener to the
providerwithin the test case is a valid change. It ensures that the provider is actively listened to during the test execution, which aligns with the purpose of verifying the auto-scoping behavior.
205-206: LGTM!The addition of the listener to the
providerwithin the test case is a valid change. It ensures that the provider is actively listened to during the test execution, which aligns with the purpose of verifying the behavior of scoping an override by passing the provider itself.
255-256: LGTM!The addition of the listener to the
providerwithin the test case is a valid change. It ensures that the provider is actively listened to during the test execution, which aligns with the purpose of verifying the behavior of scoping an override usingprovider.overrideWith.packages/flutter_riverpod/test/consumer_listen_test.dart (4)
160-160: LGTM!The change from
hasListenerstohasNonWeakListenersaligns with the PR objective and ensures a more specific check for active listeners.
164-167: LGTM!The assertion correctly verifies that the subscription is closed and the provider does not have non-weak listeners after the widget is disposed. This change maintains consistency with the PR objective.
186-193: LGTM!The assertions correctly verify that the subscription is specific to the provider instance being listened to. The change in method name maintains consistency with the PR objective.
207-214: LGTM!The assertions correctly verify that the subscription is updated when the provider instance changes. The previous subscription is closed, and a new subscription is established for the updated provider instance. The change in method name maintains consistency with the PR objective.
packages/riverpod/lib/src/core/foundation.dart (2)
188-194: LGTM!The changes to the
addListenermethod signature are well-structured and introduce meaningful enhancements:
The return type change from
ProviderSubscription<StateT>toProviderSubscriptionWithOrigin<StateT, Object?>suggests that the subscription now includes additional context or origin information. This could be beneficial for debugging or tracking the source of subscriptions.The renaming of the
nodeparameter tosourceprovides a more descriptive and accurate representation of its purpose.Note that these modifications may require updates to the code that invokes the
addListenermethod to handle the new return type and parameter name.
188-194: Verify the impact of removing thereadmethod.The removal of the
readmethod from theProviderListenablemixin suggests a change in how state can be accessed without adding a listener. This could potentially break existing code that relies on this functionality.Please ensure that:
- There are alternative methods available for consumers of this mixin to obtain state without subscribing to changes.
- The removal of the
readmethod does not introduce any unexpected behavior or limitations.- The documentation and usage guidelines are updated to reflect this change and provide guidance on accessing state without the
readmethod.To verify the impact of removing the
readmethod, we can search for its usage in the codebase:This script will help identify any code that directly invokes the
readmethod on instances ofProviderListenable. If there are any matches, it indicates that the removal of the method could break existing functionality, and alternative approaches may need to be implemented.Verification successful
These scripts will help us gather more information about the usage of
ProviderListenableand potential alternatives to thereadmethod. This will allow us to make a more informed decision about the impact of removing thereadmethod.
The removal of the
readmethod appears to have minimal impact on the codebase.Based on the analysis of the codebase, we couldn't find any direct usages or mentions of the
readmethod in relation toProviderListenable. This suggests that its removal is unlikely to cause widespread issues.However, to ensure a smooth transition:
- Review the
ProviderOrFamilyandRefreshable<StateT>classes, which implementProviderListenable, to confirm they don't rely on the removedreadmethod.- Update any relevant documentation to reflect the removal of the
readmethod and provide guidance on alternative ways to access state without subscribing to changes.- Consider documenting the
get providerListenablemethod found in theProviderListenableExpressionclass, as it might be a potential alternative for accessing state.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for usages of the `read` method rg --type dart $'ProviderListenable<.*>\.read\('Length of output: 49
Script:
#!/bin/bash # 1. Verify rg is working correctly echo "Verifying rg command:" rg --version # 2. Search for mentions of 'read' in relation to ProviderListenable in comments echo "Searching for 'read' mentions in comments:" rg --type dart -i 'providerlistenable.*read' -g '!*.g.dart' # 3. Look for potential alternative methods echo "Searching for potential alternative methods:" rg --type dart 'ProviderListenable.*\b(get|value|state)\b' -g '!*.g.dart' # 4. Search for ProviderListenable usage echo "Searching for ProviderListenable usage:" rg --type dart 'class.*implements.*ProviderListenable' -g '!*.g.dart'Length of output: 988
packages/riverpod/test/src/core/modifiers/select_async_test.dart (5)
19-19: LGTM!Adding the
container.listencall allows the test to observe changes in thedepprovider, enhancing the test's ability to verify the behavior of the asynchronous selection made throughselectAsync.
36-36: LGTM!Adding the
container.listencall allows the test to observe changes in thedepprovider, enhancing the test's ability to verify the behavior of the asynchronous selection made throughselectAsyncwhen errors occur.
238-240: LGTM!Adding the
container.listencalls for theaandbproviders allows the test to observe changes in these providers, enhancing the test's ability to verify the behavior of watching async selectors.
296-298: LGTM!Adding the
container.listencalls for theaandbproviders allows the test to observe changes in these auto-disposed providers, enhancing the test's ability to verify the behavior of watching async selectors with auto-dispose.
349-350: LGTM!Adding the
container.listencalls for theproviderin various test cases allows the tests to observe changes in the provider, enhancing the tests' ability to verify the behavior ofProviderContainer.readwith async selectors.Also applies to: 362-363, 374-375
packages/riverpod/test/src/core/provider_element_test.dart (5)
12-36: LGTM!The test is correctly verifying that only direct subscriptions are included in the subscription lists of provider elements. The setup and assertions look good.
358-377: LGTM!The test is correctly verifying that a provider is paused if all its watchers are paused. The setup and assertions look good.
379-397: LGTM!The test is correctly verifying that a provider is paused if all its subscriptions are paused. The setup and assertions look good.
453-463: LGTM!The test is correctly verifying that weak listeners are excluded from the count of active listeners. The setup and assertions look good.
524-538: LGTM!The test is correctly verifying that listeners are not notified multiple times when using the
fireImmediatelyoption. The setup and assertions look good.packages/riverpod/test/old/legacy/framework2/framework_test.dart (5)
136-137: LGTM!The change enhances the clarity of the test by explicitly using a subscription to read the provider value, which aligns with the intended usage of Riverpod's listening capabilities.
144-144: LGTM!The change enhances the clarity of the test by explicitly using a subscription to read the provider value, which aligns with the intended usage of Riverpod's listening capabilities.
273-275: LGTM!The change from
hasListenerstohasNonWeakListenerssuggests a refinement in how listener states are evaluated, potentially improving the accuracy of the tests regarding listener management.Also applies to: 291-293
332-332: LGTM!The change from
hasListenerstohasNonWeakListenerssuggests a refinement in how listener states are evaluated, potentially improving the accuracy of the tests regarding listener management.Also applies to: 343-343
433-433: LGTM!The change from
hasListenerstohasNonWeakListenerssuggests a refinement in how listener states are evaluated, potentially improving the accuracy of the tests regarding listener management.Also applies to: 437-437, 442-442
packages/flutter_riverpod/lib/src/core/consumer.dart (5)
72-80: Documentation looks good!The added section clearly explains the performance optimization behavior of
Consumerby pausing subscriptions when the widget is not visible. This can help avoid unnecessary network requests and computations.
371-379: Good addition of_visibleproperty and_allSubscriptionsgetter!The
_visibleproperty is necessary to track the visibility state of the widget. The_allSubscriptionsgetter provides a convenient way to access all subscriptions in one place, which will be useful for applying visibility-based actions.
381-383:_applyVisibilitymethod looks good!The
_applyVisibilitymethod encapsulates the logic for pausing subscriptions based on visibility. It checks if_visibleisfalseand callspause()on the provided subscription. This will be useful for consistently applying visibility-based actions to subscriptions.
400-411: Visibility handling inbuildmethod looks good!The added code in the
buildmethod properly handles visibility changes by:
- Retrieving the current visibility using
Visibility.of(context).- Updating the
_visibleproperty if the visibility has changed.- Iterating over all subscriptions using
_allSubscriptionsand callingresume()orpause()based on the visibility state.This ensures that visibility actions are applied consistently to all subscriptions, optimizing performance by avoiding unnecessary work when the widget is not visible.
444-449: Subscription handling changes and_ListenManualclass look good!The changes made to the
watch,listen, andlistenManualmethods ensure that visibility actions are applied to subscriptions created through these methods by calling_applyVisibilityat appropriate places.The introduction of the
_ListenManualclass is a good way to wrap theProviderSubscriptionWithOriginand handle thecloselogic specific to manual listeners. Theclosemethod correctly removes the subscription from_manualListenersbefore callingsuper.close(), and thereadmethod delegates the call to the inner subscription.These changes work well with the visibility handling logic and contribute to the overall performance optimization of the
Consumerwidget.Also applies to: 490-490, 536-544, 551-569
packages/riverpod/lib/src/core/modifiers/future.dart (6)
5-9: LGTM!The new
AsyncSubscriptiontypedef provides a structured way to manage the lifecycle of an asynchronous subscription, including cancellation, pausing, and resuming. This change aligns with the objective of introducing a pause mechanism for subscriptions.
239-239: LGTM!The change in property type from
CancelAsyncSubscriptiontoAsyncSubscriptionreflects the transition to a more complex structure for managing asynchronous subscriptions, which is consistent with the introduction of theAsyncSubscriptiontypedef.
351-361: LGTM!The change in the
handleStreammethod to use the newlistenAndTrackLastextension method onStreamis a good improvement. It encapsulates the logic for tracking the last emitted value and handling the completion of the stream, while returning anAsyncSubscriptioninstance for more granular control over the subscription lifecycle.
363-374: LGTM!The updates to the
onCancelandonResumemethods to invoke thepauseandresumefunctions of the_cancelSubscriptionproperty are a good addition. This allows for pausing and resuming the asynchronous subscription when the associated provider is cancelled or resumed, leveraging the new functionality provided by theAsyncSubscriptiontuple.
426-430: LGTM!The modification to the
handleFuturemethod to return anAsyncSubscriptioninstance with acancelfunction andnullvalues forpauseandresumeis a good change. It ensures consistency with the newAsyncSubscriptiontypedef while acknowledging the limitations of futures in terms of pausing and resuming.
Line range hint
536-591: LGTM!The new
listenAndTrackLastextension method on theStreamclass is a valuable addition. It encapsulates the logic for listening to the stream, tracking the last emitted value, and handling the completion of the stream. The method returns anAsyncSubscriptioninstance with functions for cancelling, pausing, and resuming the subscription, providing more control over the subscription lifecycle. The use of aCompleterto capture the last emitted value and propagate it to theprovider.futurewhen the stream completes is a good approach. The error handling implementation covers cases where the stream completes with an error or without emitting any value.packages/riverpod/test/old/framework/ref_watch_test.dart (6)
320-320: LGTM!The instantiation of
notifier0looks good.
323-323: LGTM!The instantiation of
notifier1with an initial value of42looks good.
366-367: Verify the usage ofhasNonWeakListeners.Similar to the previous comment, ensure that the
hasNonWeakListenersproperty correctly reflects the presence of non-weak listeners on the provider elements after the state change.Run the following script to verify the usage:
Verification successful
Usage of
hasNonWeakListenersis correct and consistent.The
hasNonWeakListenersproperty is used appropriately in the test file to verify the presence of non-weak listeners on provider elements after state changes. This usage is consistent with its application throughout the codebase and correctly checks the listener status of provider elements.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `hasNonWeakListeners` in the test case after the state change. # Test: Search for the usage of `hasNonWeakListeners` after the state change. # Expect: The property is used to check the presence of non-weak listeners on the provider elements. rg --type dart $'hasNonWeakListeners' -A 5Length of output: 19853
345-346: Verify the usage ofhasNonWeakListeners.Ensure that the
hasNonWeakListenersproperty correctly reflects the presence of non-weak listeners on the provider elements.Run the following script to verify the usage:
Verification successful
The usage of
hasNonWeakListenersis correct and consistent with the rest of the codebase.The
hasNonWeakListenersproperty is being used appropriately in the test file to verify the presence or absence of non-weak listeners on provider elements. This usage aligns with its implementation and usage patterns throughout the Riverpod package's test suite.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `hasNonWeakListeners` in the test case. # Test: Search for the usage of `hasNonWeakListeners`. # Expect: The property is used to check the presence of non-weak listeners on the provider elements. rg --type dart $'hasNonWeakListeners' -A 5Length of output: 19853
413-414: Verify the usage ofhasNonWeakListeners.Ensure that the
hasNonWeakListenersproperty correctly reflects the presence of non-weak listeners on the provider elements.Run the following script to verify the usage:
Verification successful
Usage of
hasNonWeakListenersis correct and consistentThe
hasNonWeakListenersproperty is used correctly in the test case to verify the presence of non-weak listeners on provider elements. This usage is consistent with its implementation and usage throughout the Riverpod codebase. The test accurately checks whether providers are being listened to or not as the state changes, which is crucial for verifying the correct behavior of the ref.watch mechanism.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `hasNonWeakListeners` in the test case. # Test: Search for the usage of `hasNonWeakListeners`. # Expect: The property is used to check the presence of non-weak listeners on the provider elements. rg --type dart $'hasNonWeakListeners' -A 5Length of output: 19853
434-435: Verify the usage ofhasNonWeakListeners.Similar to the previous comment, ensure that the
hasNonWeakListenersproperty correctly reflects the presence of non-weak listeners on the provider elements after the state change.Run the following script to verify the usage:
Verification successful
The usage of
hasNonWeakListenersis correct and consistent with the rest of the codebase.The
hasNonWeakListenersproperty is used consistently throughout the test files to verify the presence of non-weak listeners on provider elements after state changes. This usage aligns with its implementation and intended purpose, as evidenced by numerous test cases across the codebase that specifically check its behavior in various scenarios.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `hasNonWeakListeners` in the test case after the state change. # Test: Search for the usage of `hasNonWeakListeners` after the state change. # Expect: The property is used to check the presence of non-weak listeners on the provider elements. rg --type dart $'hasNonWeakListeners' -A 5Length of output: 19853
packages/riverpod/lib/src/core/ref.dart (5)
14-16: Capturing the origin in UnmountedRefException is a valuable addition.The inclusion of the
originproperty in theUnmountedRefExceptionconstructor allows for more informative error messages by indicating which reference was disposed of when the exception is thrown. This change enhances the debugging experience and aligns with the goal of providing better context in error scenarios.
150-150: Improved error reporting by including the origin in UnmountedRefException.The modification to throw
UnmountedRefExceptionwith_element.originas a parameter enhances error reporting by providing context about the source of the error. This change leverages the newly addedoriginproperty in the exception constructor and aligns with the goal of improving error messages for better debugging.
169-169: Correctly handles the scenario when all KeepAliveLinks are closed.The code segment correctly checks if the
linkslist becomes empty after removing aKeepAliveLink. If there are no more activeKeepAliveLinks, it appropriately calls_element.mayNeedDispose()to allow the provider to potentially dispose itself if there are no listeners. This ensures proper resource management and aligns with the purpose of thekeepAlivemethod.
233-233: Debug assertion helps catch potential dependency issues.The addition of the debug assertion in the
invalidatemethod is a valuable safeguard. It ensures that the provider can properly depend on the givenproviderOrFamilyand helps catch potential issues during development. This assertion maintains the integrity of the dependency graph and aids in identifying incorrect dependencies early in the development process.
546-551: Simplified dependency management logic improves readability and maintainability.The refactored code segment in the
watchmethod simplifies the dependency management logic compared to the previous implementation. By directly utilizing the_element.listenmethod, the code becomes more readable and maintainable. TheonErrorcallback ensures that the provider is invalidated when an error occurs, triggering a refresh. TheonDependencyMayHaveChangedcallback properly marks the dependency as potentially changed, allowing for necessary updates. These changes streamline the code and enhance its overall quality.packages/riverpod/lib/src/core/element.dart (10)
45-45: Clarify the reason for changing the implemented interface.The class now implements
Nodeinstead ofWrappedNode. Could you provide more context about the differences between these interfaces and the motivation behind this change? Understanding the implications of this shift will help ensure a smooth transition.
64-64: Improved type safety!Updating the return type of the
origingetter toProviderBase<StateT>enhances type safety by providing a more specific type. This change aligns with best practices and can help catch type-related issues early. Nice work!
83-89: Simplified activity determination logic.The updated logic for determining if a provider is active based on the listener count and paused active subscription count simplifies the code and improves clarity. Introducing dedicated properties for tracking these values enhances maintainability. Great job streamlining this functionality!
95-95: Improved naming clarity.Renaming the
hasListenersproperty tohasNonWeakListenersclarifies that it specifically checks for the presence of non-weak listeners. This change enhances code readability and reduces the potential for confusion. Well done!
235-240: Enhanced notification control.The additional arguments
isMountandcheckUpdateShouldNotifypassed to the_notifyListenersmethod provide more granular control over the notification behavior. TheisMountargument distinguishes between initial mount and subsequent notifications, whilecheckUpdateShouldNotifyallows for conditional notification based on update significance. These enhancements improve the flexibility and efficiency of the notification process. Good job!
401-401: Consistent invalidation and disposal flow.Adding the
mayNeedDisposemethod call after invalidating the provider ensures that the disposal logic is triggered when necessary. This change enhances the consistency of the invalidation and disposal flow and aligns with the expected behavior. Nice catch!
419-421: Improved notification safety.The inclusion of the
isMountparameter in the_notifyListenersmethod allows for distinguishing between initial mount and subsequent notifications. The associated assertion in debug mode adds a safety check to prevent unexpected notifications during non-mount scenarios. This change enhances the robustness of the notification mechanism and helps catch potential issues early. Well done!
462-484: Consolidated and optimized notification logic.The consolidation of the listener notification logic into a single loop improves code readability and maintainability. By including both weak dependents and regular dependents in the notification process, the code ensures comprehensive coverage.
Skipping closed listeners during notification is a nice optimization that avoids unnecessary work. The consolidated logic also handles both
ResultDataandResultErrorstates effectively, providing a unified notification mechanism.Overall, these changes enhance the clarity, efficiency, and robustness of the notification process. Great job simplifying and optimizing this critical functionality!
490-505: Targeted observer notifications.Differentiating between mount and non-mount scenarios when notifying observers allows for more targeted and specific notifications. Calling
didAddProviderduring mount ensures that observers are properly informed when a new provider is added, while callingdidUpdateProviderduring non-mount scenarios keeps observers up to date with provider changes.This improvement enhances the granularity and precision of observer notifications, enabling observers to react appropriately based on the scenario. It's a thoughtful addition that promotes better communication and coordination between providers and observers. Nice work!
550-567: Flexible subscription handling and dependency management.The updates to the
listenmethod introduce support for handling different types of subscriptions, providing flexibility and extensibility. The specific handling ofProviderSubscriptionImplsubscriptions, including the call toaddDependentSubscription, ensures proper tracking and management of dependencies between the provider and the listened element.This change demonstrates a well-structured approach to managing subscriptions and their associated dependencies. It allows for targeted behavior based on the subscription type while maintaining a clear and organized code structure.
The improved subscription handling and dependency management contribute to the overall robustness and maintainability of the provider system. Great job implementing this flexible and extensible solution!
packages/flutter_riverpod/test/src/core/consumer_test.dart (3)
10-31: LGTM!The
SimpleVisibilitywidget is implemented correctly. It wraps its child with aVisibilitywidget and passes thevisibleparameter to control the visibility. Themaintain*properties are set totrueto preserve the child's state when it's not visible.
34-68: Test case looks good!The test case
handles pause/resumecorrectly verifies the pause and resume functionality ofProviderSubscription. It checks the active state of theProviderElementbefore and after pausing/resuming the subscription.
70-225: Test cases for handling visibility look good!The test cases in the
Handles Visibilitygroup cover various scenarios related to visibility and listener behavior:
when adding a listener, initializes pause state based on visibility: Verifies that listeners are initialized with the correct pause state based on the visibility of the associated widget.
listenManual inside life-cycles before didChangeDependencies on non-visible widgets does not paused twice: Verifies that callinglistenManualinside life-cycle methods beforedidChangeDependencieson non-visible widgets does not pause the subscription twice.
when visibility changes, pause/resume listeners: Verifies that listeners are paused when the associated widget becomes non-visible and resumed when it becomes visible again.
when a dependency changes but visibility does not, do not pause/resume listeners: Verifies that listeners are not unnecessarily paused or resumed when a dependency changes but the visibility remains unchanged.The test cases ensure the correct pausing and resuming of listeners based on visibility changes.
packages/riverpod/test/old/legacy_providers/stream_provider/stream_provider_test.dart (12)
84-84: LGTM!Changing from
readtolistenis the correct approach to actively monitor the provider's state changes in the test.
105-106: LGTM!Changing from
readtolistenis the correct approach to actively monitor the provider's state changes triggered byrefreshandinvalidatein the test.
134-135: LGTM!Changing from
readtolistenis the correct approach to actively monitor the provider's state changes triggered by dependency changes in the test.
157-158: LGTM!Changing from
readtolistenis the correct approach to actively monitor the provider's state changes triggered by both dependency changes andrefreshin the test.
228-228: LGTM!Changing from
readtolistenis the correct approach to actively monitor the scoped and overridden provider's state in the test.
245-245: LGTM!Changing from
readtolistenis the correct approach to actively monitor the provider's state and verify that listeners are not notified when transitioning fromAsyncLoadingtoAsyncLoadingin the test.
286-286: LGTM!Changing from
readtolistenis the correct approach to actively monitor the provider's state and verify that it can be refreshed and emits the updated value in the test.
312-312: LGTM!Changing from
readtolistenis the correct approach to actively monitor the provider's state and verify that overriding the provider itself scopes all the associated sub-providers in the test.
337-337: LGTM!Changing from
readtolistenis the correct approach to actively monitor the provider's state and verify that usingprovider.overrideWithValuescopes all the associated sub-providers in the test.
362-362: LGTM!Changing from
readtolistenis the correct approach to actively monitor the provider's state and verify that usingprovider.overrideWithscopes all the associated sub-providers in the test.
381-382: LGTM!Changing from
readtolistenis the correct approach to actively monitor the provider's state and verify the transition fromAsyncLoadingtoAsyncDatawhen a value is added to the stream in the test.
396-397: LGTM!Changing from
readtolistenis the correct approach to actively monitor the provider's state and verify the transition fromAsyncLoadingtoAsyncErrorwhen an error is added to the stream in the test.packages/riverpod/lib/src/core/provider_container.dart (3)
570-570: LGTM!The change from implementing
WrappedNodetoNodeinterface seems reasonable. It likely provides a more suitable abstraction for theProviderContainerfunctionality.
738-744: Excellent improvement to thereadmethod!The utilization of a subscription mechanism and the introduction of the try-finally block enhances the resource management and reliability of the
readmethod. It ensures that subscriptions are properly closed after reading, preventing potential resource leaks.This change improves the overall robustness and efficiency of the
readoperation without negatively impacting existing usage.
773-787: Nice enhancement to thelistenmethod!The changes made to the
listenmethod improve the subscription management and dependency tracking within the provider system. By creating a subscription usingaddListenerand adding it to a list of dependent subscriptions based on the subscription type, it allows for better management of the relationships between providers and their listeners.This enhancement could potentially lead to better performance and more accurate state management by efficiently handling the subscription dependencies. It does not negatively impact the existing usage of the
listenmethod.packages/riverpod/test/src/providers/stream_notifier_test.dart (4)
33-64: LGTM!The test correctly verifies that the
StreamSubscriptionis properly closed when the provider is disposed. It checks that an error is thrown when the stream is closed and confirms that the cancellation callback is invoked.
66-99: LGTM!The test correctly verifies the behavior of the stream when the provider is paused and resumed. It checks that the appropriate callbacks for pausing and resuming the subscription are triggered.
135-161: LGTM!The test correctly verifies that manually setting the state to an error does not trigger a retry mechanism. It checks that the retry count remains at 0 after manually setting the state to an error.
717-737: LGTM!The test correctly verifies that transitioning back to a loading state after a future has resolved throws a
StateError. It sets up a provider with a future that resolves, then manually sets the state back to loading and expects aStateErrorto be thrown.packages/flutter_riverpod/CHANGELOG.md (4)
3-3: Looks good!The changelog entry clearly describes the breaking change in behavior and its potential impact on existing code. This information is valuable for users to understand and adapt to the change.
4-6: Changelog entry is informative and clear.The entry effectively communicates the breaking change of removing
Refsubclasses and provides actionable migration steps for users. The additional guidance to useAsyncNotifierfor handling futures is a helpful suggestion for adapting existing code.
Line range hint
7-10: Organizational change is well-documented.The changelog entry clearly outlines the breaking change of moving specific providers to a
legacy.dartfile. This information is important for users to update their import statements accordingly. Listing the affected providers helps in identifying the impact of this change.
Line range hint
11-16: Concise and informative changelog entries.The changelog entries in this section provide a good summary of the various changes, additions, and fixes included in this version. The descriptions are brief but clear, making it easy for users to understand the scope and impact of each change. No additional information seems necessary.
packages/riverpod/CHANGELOG.md (7)
Line range hint
1-47: The "Unreleased build" section looks good overall!The changes are well-documented with clear explanations and code samples. The breaking changes, new features, and bug fixes are all significant and will have a notable impact on users.
Line range hint
49-58: The "3.0.0-dev.3" section looks good!The bug fixes are well-described and address important issues. No changes needed.
Line range hint
60-62: The "3.0.0-dev.2" section looks good!The bug fix is clearly described. No changes needed.
Line range hint
64-70: The "3.0.0-dev.1" section looks good!The bug fixes and improvements are well-described and address important issues. No changes needed.
Line range hint
72-76: The "3.0.0-dev.0" section looks good!The breaking changes and SDK version bump are clearly documented. No changes needed.
Line range hint
78-87: The "2.5.1" section looks good!The documentation improvements and bug fixes are well-described. No changes needed.
Line range hint
89-91: The "2.5.0" section looks good!The enhancement to
AsyncValue.guardis clearly described. No changes needed.packages/riverpod/test/src/core/ref_test.dart (5)
826-843: LGTM!The new test verifies that value listeners are not invoked when the subscription is paused, ensuring the correct behavior.
845-872: LGTM!The new test verifies that error listeners are not invoked when the subscription is paused, ensuring the correct behavior.
966-981: LGTM!The changes to the subscription closing test clarify that closing a subscription removes the listener, aligning with the actual behavior.
Line range hint
2108-2117: LGTM!The updates to the
.onRemoveListenertest confirm that the listener is invoked on read, as expected.
Line range hint
2286-2295: LGTM!The updates to the
.onAddListenertest confirm that the listener is invoked on read, as expected.packages/riverpod/test/src/core/provider_container_test.dart (2)
Line range hint
2463-2488: Approved: The changes correctly update the test case to align with the new listener management behavior.The updated test case now verifies that when a listener removes another listener, the removed listener is not called for subsequent notifications. This change in behavior is consistent with the updated expectations around listener management.
Note that this change could impact how listener removal is handled in the broader application context, so developers should be aware of this when implementing listener removal logic.
2880-2900: Approved: The updated assertions improve the specificity and clarity of the test case.The changes to the assertions, now using the
hasNonWeakListenersproperty, provide a more precise verification of the presence of non-weak listeners. This aligns with the focus of the test case and improves the clarity of the expected behavior.While this change does not significantly impact the overall functionality, it enhances the test's ability to accurately verify the desired listener management behavior.
| ProviderSubscriptionWithOrigin<StateT, StateT> addListener( | ||
| Node source, | ||
| void Function(StateT? previous, StateT next) listener, { | ||
| required void Function(Object error, StackTrace stackTrace)? onError, | ||
| required void Function()? onDependencyMayHaveChanged, | ||
| required bool fireImmediately, | ||
| required bool weak, |
There was a problem hiding this comment.
Tip
Codebase Verification
Update addListener usage across the codebase
Our verification process has revealed inconsistencies in the usage of the addListener method throughout the codebase. While some files have adopted the new signature with the weak parameter and the ProviderSubscriptionWithOrigin return type, others still use the old signature. This inconsistency could lead to potential issues and should be addressed.
Please review and update the following areas:
- In
packages/riverpod/lib/src/common/listenable.dart, update theaddListenermethod to include theweakparameter and the new return type. - Check and update the usage in
website/docs/essentials/websockets_sync/shared_pipe_change_notifier.dartandwebsite/docs/essentials/websockets_sync/pipe_change_notifier.dart. - Review all other occurrences of
addListenerin the codebase to ensure they are using the updated signature consistently.
Note: The usage in website/docs/migration/from_state_notifier/add_listener_old.dart may be intentionally using the old signature for documentation purposes. Please verify if this is the case and update or add a comment explaining the use of the old signature if necessary.
Analysis chain
Verify the impact of the return type change and new parameter.
The changes to the addListener method signature, including the return type modification and the addition of the weak parameter, seem to be part of a larger refactoring effort to improve subscription management. These changes are likely to have implications on the code that consumes this method.
Please ensure that:
- The code that relies on the
addListenermethod is updated to handle the newProviderSubscriptionWithOriginreturn type. - The usage of the new
weakparameter is consistent throughout the codebase and aligns with the intended subscription management strategy.
You can use the following script to locate the usage of the addListener method:
To gather more information about potential wrapper functions or compatibility layers that might be hiding the updated addListener signature, let's run an additional script:
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the usage of the `addListener` method.
# Test: Search for the method usage. Expect: Only occurrences with the updated signature.
rg --type dart -A 5 $'addListener'
Length of output: 16667
Script:
#!/bin/bash
# Description: Search for potential wrapper functions or compatibility layers for addListener
# Search for functions that might be wrapping addListener
rg --type dart -A 5 'function.*addListener'
# Search for potential compatibility layer classes or functions
rg --type dart -A 5 '(class|function).*Compat.*addListener'
Length of output: 107
Related Issues
fixes #1344
fixes #1207
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]).I have updated the
CHANGELOG.mdof the relevant packages.Changelog files must be edited under the form:
If this contains new features or behavior changes,
I have updated the documentation to match those changes.
Summary by CodeRabbit
Release Notes
New Features
ConsumerWidgetsby pausing provider subscriptions when widgets are not visible.ProviderSubscription.pause()andProviderSubscription.resume()for better subscription management.AsyncNotifierfor handling asynchronous state more effectively.Breaking Changes
Ref, requiring direct use ofRef.hasNonWeakListenersfor listener checks.Bug Fixes
Tests
Documentation