isAutoDispose #4080
isAutoDispose #4080
Conversation
WalkthroughThis update introduces an optional Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ProviderFamilyBuilder
participant ProviderFamily
User->>ProviderFamilyBuilder: call(create, isAutoDispose: true/false)
ProviderFamilyBuilder->>ProviderFamily: new(create, isAutoDispose: true/false)
ProviderFamily-->>ProviderFamilyBuilder: ProviderFamily instance
ProviderFamilyBuilder-->>User: ProviderFamily instance
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (8)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
packages/riverpod/CHANGELOG.md (1)
3-3: Clarify the new syntax with an explicit “since” version markerThe bullet nicely explains the new
isAutoDisposeparameter, but it is easy to miss in a sea of other changes. Consider explicitly prefixing the sentence with the version tag (e.g. “Since 3.0.0-dev.x: …”) so that readers can immediately see when the change became available.tools/generate_providers/bin/generate_providers.dart (2)
260-266: Parameter ordering: keep the often-usedisAutoDisposeclose tonameIn the generated
callmethods,isAutoDisposeis placed after rarely-used parameters (dependencies,retry). To improve discoverability, consider moving it right aftername.- String? name, - Iterable<ProviderOrFamily>? dependencies, - Retry? retry, - bool isAutoDispose = false, + String? name, + bool isAutoDispose = false, + Iterable<ProviderOrFamily>? dependencies, + Retry? retry,This mirrors the mental model: most users toggle auto-dispose and give the provider a name before tweaking advanced knobs.
Also applies to: 298-304, 417-418
369-380: Trailing comma handled byDartFormatter, but an empty string could still trip string-based tooling
_isAutoDisposeArgreturns an empty string for the auto-dispose variant.
If another tool (e.g. a doc generator) parses parameters via regex rather than the formatted output, the missing comma could produce false positives. Returningnulland letting the caller decide whether to add a comma avoids that subtle edge case:-String _isAutoDisposeArg(_DisposeType disposeType) { - if (disposeType == _DisposeType.autoDispose) return ''; - return 'bool isAutoDispose = false,'; -} +String? _isAutoDisposeArg(_DisposeType disposeType) { + if (disposeType == _DisposeType.autoDispose) return null; + return 'bool isAutoDispose = false,'; +}Call-sites would wrap the interpolation in a null-aware operator.
Only a minor robustness tweak.packages/flutter_riverpod/lib/src/builders.dart (1)
232-244: Add a short doc comment for the newisAutoDisposeparameterThe method signature now exposes
isAutoDispose, but users scanning the huge existing docstring won’t see what the flag does or why they might need it.- bool isAutoDispose = false, + /// Whether the created provider should be automatically disposed when it + /// no longer has listeners. Defaults to `false`. + bool isAutoDispose = false,A one-liner keeps the public API self-documenting, especially when IDEs show parameter hints.
packages/riverpod/lib/src/builder.dart (1)
1-883: Overall code changes look good, consider adding documentation for the new parameter.The implementation of the
isAutoDisposeparameter across all provider family builders is consistent and thorough. All auto-dispose variants correctly setisAutoDispose: trueinternally, while the regular variants default tofalse.While the codebase has extensive documentation for other features, consider adding specific documentation for the new
isAutoDisposeparameter in the method signatures to help users understand its purpose.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/flutter_riverpod/lib/src/builders.dart(2 hunks)packages/riverpod/CHANGELOG.md(1 hunks)packages/riverpod/lib/src/builder.dart(16 hunks)packages/riverpod/test/feature/auto_dispose_test.dart(1 hunks)tools/generate_providers/bin/generate_providers.dart(7 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (15)
- GitHub Check: riverpod_lint (master, packages/riverpod_lint_flutter_test)
- GitHub Check: riverpod_lint (stable, packages/riverpod_lint_flutter_test)
- GitHub Check: riverpod_lint (master, packages/riverpod_lint)
- GitHub Check: riverpod_lint (stable, packages/riverpod_analyzer_utils_tests)
- GitHub Check: riverpod_lint (master, packages/riverpod_analyzer_utils)
- GitHub Check: build (master, packages/hooks_riverpod/example)
- GitHub Check: build (master, packages/flutter_riverpod/example)
- GitHub Check: riverpod_lint (master, packages/riverpod_analyzer_utils_tests)
- GitHub Check: build (master, examples/random_number)
- GitHub Check: build (master, packages/riverpod)
- GitHub Check: build (master, examples/stackoverflow)
- GitHub Check: build (master, examples/pub)
- GitHub Check: build (stable, packages/riverpod/example)
- GitHub Check: build (stable, examples/random_number)
- GitHub Check: check_generation
🔇 Additional comments (13)
tools/generate_providers/bin/generate_providers.dart (1)
248-254:❓ Verification inconclusive
@publicInMiscmay require an explicit importYou added
@publicInMiscon every generated builder, but this file does not import the library that defines that annotation.
Ifinternals.dartdoes not export it, generated code will not compile.-import 'internals.dart'; +import 'internals.dart'; +import 'package:riverpod/src/public_in_misc.dart'; // or wherever the annotation livesPlease double-check the annotation’s location (or re-export it from
internals.dart) so the generated source stays self-contained.Also applies to: 286-294
🏁 Script executed:
#!/bin/bash # Locate internals.dart file(s) in the repo internals_paths=$(fd -HI internals.dart) echo "Found internals.dart at:" echo "$internals_paths" # Show export statements in each internals.dart echo -e "\nExport statements in internals.dart:" grep -R "export" -n $internals_paths || true # Search the entire repo for the definition of PublicInMisc or its usage echo -e "\nDefinition or declaration of PublicInMisc:" rg "class PublicInMisc" -n . echo -e "\nOccurrences of publicInMisc in code:" rg "publicInMisc" -n .Length of output: 6541
🏁 Script executed:
#!/bin/bash # Locate generate_providers.dart file file=$(fd -HI generate_providers.dart | grep "tools/generate_providers/bin") echo "Found generate_providers.dart at: $file" # Show the imports and context around internals.dart echo -e "\nImports in generate_providers.dart:" sed -n '1,100p' "$file" | grep -n "^import" # Show the relative path for internals.dart import echo -e "\nLines importing internals.dart:" grep -n "internals.dart" -n "$file"Length of output: 803
🏁 Script executed:
# After inspecting the imports, show if common/internal_lints.dart is referenced grep -n "internal_lints.dart" -n "$file" || echo "No direct import of internal_lints.dart"Length of output: 158
🏁 Script executed:
# Check if the local internals.dart near generate_providers.dart exists and what's inside dir=$(dirname "$file") if [ -f "$dir/internals.dart" ]; then echo -e "\nContents of $dir/internals.dart:" sed -n '1,50p' "$dir/internals.dart" else echo "No local internals.dart next to generate_providers.dart" fiLength of output: 172
Add explicit import for
publicInMiscin generated providersThe generation script emits
@publicInMiscbut only writesimport 'internals.dart';If your local internals.dart stub doesn’t re-export
common/internal_lints.dart, the generated files won’t compile.
- In tools/generate_providers/bin/generate_providers.dart (around lines 172 & 194), update the import block to:
buffer.writeln("import 'internals.dart';"); +buffer.writeln("import 'package:riverpod/src/common/internal_lints.dart';"); // for @publicInMiscThis ensures
@publicInMiscis in scope.
Alternatively, re-exportpublicInMiscfrom your local internals.dart so the generated code stays self-contained.packages/riverpod/test/feature/auto_dispose_test.dart (4)
7-24: Well-designedBuilderclass for flexible provider testing.The
Builderclass provides an elegant way to wrap various provider constructors for testing with theisAutoDisposeparameter. It handles different parameter cases well and throws appropriate errors for invalid input.
26-38: Good use of extensions for handling different function signatures.The extensions on the
Builderclass effectively handle different provider constructor signatures, providing a consistent interface through theunimplementedgetter.
40-60: Comprehensive test matrix with good organization.The matrix effectively organizes all provider types into functional, notifier, and legacy categories, ensuring thorough test coverage across the Riverpod ecosystem.
62-82: Thorough test cases for both default and explicitisAutoDisposevalues.These tests effectively verify that:
- The default
isAutoDisposevalue isfalsefor all providers- Setting
isAutoDispose: trueis correctly reflected on the provider instancesThis ensures consistency across all provider types with the new API.
packages/riverpod/lib/src/builder.dart (8)
231-245: Consistent implementation ofisAutoDisposeparameter forStateProviderFamilyBuilder.The
isAutoDisposeparameter has been correctly added with a default value offalseand is properly forwarded to the constructor ofStateProviderFamily.
353-369: Consistent implementation ofisAutoDisposeparameter forStateNotifierProviderFamilyBuilder.The
isAutoDisposeparameter has been correctly added with a default value offalseand is properly forwarded to the constructor ofStateNotifierProviderFamily.
426-446: Consistent implementation ofisAutoDisposeparameter forProviderFamilyBuilder.The
isAutoDisposeparameter has been correctly added with a default value offalseand is properly forwarded to the constructor ofProviderFamily.
501-521: Consistent implementation ofisAutoDisposeparameter forFutureProviderFamilyBuilder.The
isAutoDisposeparameter has been correctly added with a default value offalseand is properly forwarded to the constructor ofFutureProviderFamily.
576-596: Consistent implementation ofisAutoDisposeparameter forStreamProviderFamilyBuilder.The
isAutoDisposeparameter has been correctly added with a default value offalseand is properly forwarded to the constructor ofStreamProviderFamily.
678-699: Consistent implementation ofisAutoDisposeparameter forNotifierProviderFamilyBuilder.The
isAutoDisposeparameter has been correctly added with a default value offalseand is properly forwarded to the constructor ofNotifierProviderFamily.
756-777: Consistent implementation ofisAutoDisposeparameter forStreamNotifierProviderFamilyBuilder.The
isAutoDisposeparameter has been correctly added with a default value offalseand is properly forwarded to the constructor ofStreamNotifierProviderFamily.
834-855: Consistent implementation ofisAutoDisposeparameter forAsyncNotifierProviderFamilyBuilder.The
isAutoDisposeparameter has been correctly added with a default value offalseand is properly forwarded to the constructor ofAsyncNotifierProviderFamily.
Summary by CodeRabbit