Add option to override whole provider/argprovider - #40
Conversation
WalkthroughDisco 3.0 introduces callable provider bindings, provider-based overrides, always-lazy value creation, recursive scope lookup, circular-dependency detection, reverse-order disposal, updated tests, documentation, examples, and benchmarks. ChangesProvider API and runtime
Documentation and examples
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #40 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 10 10
Lines 288 288
=========================================
Hits 288 288
... and 1 file with indirect coverage changes 🚀 New features to boost your workflow:
|
|
Technically, we could easily add two wrapper types |
|
@manuel-plavsic If you remove overrideWithValue it's going to be a breaking change and we have to bump a major. Additionally, what do you think about numberProvider.overrideWith(mockNumberProvider),
// instead of
numberProvider.overrideWithProvider(mockNumberProvider),? |
|
@nank1ro good idea, I'll rename the method to |
|
and yes, it makes sense to keep |
…when inserting them into the widget tree
Refresh override if provider is disposed and rebuilt.
Deploying flutter-disco with
|
| Latest commit: |
77f8ca8
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://e5b200f1.flutter-disco.pages.dev |
| Branch Preview URL: | https://fix-allow-whole-provider-ove.flutter-disco.pages.dev |
Provider Benchmark ResultsDate: 2026-08-09 16:31:28 UTC Results
|
|
@nank1ro I was partially assisted by Claude Opus for this PR. Multiple changes were made, not only what is in the title. The easiest way to understand all is to read the commit messages for every commit. Note that #32 introduced this issue #39. That part had to be rewritten differently. Good news: there is no global state anymore at all. Actually, what was very nice of Claude (Opus 5 High) is that it detected the global state and wanted to replace it with a better alternative, which solved the issue. All providers were made lazy because with eager evaluation there is the problem that if the provider is eager, it will be created even in presence of a mock. This really defeats the idea of providers. There is a workaround to have them eager, which is to inject the provider immediately (a bit like what riverpod does). Another change is now the values of the normal providers (i.e., without argument) are injected like this: Claude recommended using: instead of However, I am not fully sure about that. You are more the expert in this area. Do you think the suggestion is good, or should I revert this part? |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/disco/test/disco_test.dart (1)
729-784: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winTwo tests share the same name.
The test at Line 729 and the test at Line 757 are both named
ProviderScopeOverride should override providers. Duplicate names make a failure report ambiguous. The two bodies also cover the same scenario with different constants.Rename one test, or delete the redundant one.
♻️ Proposed rename
- testWidgets('''ProviderScopeOverride should override providers''', ( + testWidgets('''ProviderScopeOverride should override a provider declared in a nested scope''', ( tester, ) async { final numberProvider = Provider<int>((_) => 0); final number100Provider = Provider<int>((_) => 100);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/disco/test/disco_test.dart` around lines 729 - 784, Rename one of the duplicate testWidgets cases around the ProviderScopeOverride tests so each test name uniquely identifies its scenario, while preserving both test bodies and their existing assertions.packages/disco/lib/src/models/providers/arg_provider.dart (1)
79-84: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winForward
debugNameto the intermediate provider.
Provider._generateIntermediateProviderinpackages/disco/lib/src/models/providers/provider.dart(Line 134) forwardsdebugName. This method does not. The intermediate provider becomes the key ofProviderScopeState._createdValues, and disposal errors are reported withentry.key._debugNameinpackages/disco/lib/src/widgets/provider_scope.dart(Line 497). Without the forward, a disposal error for an argument provider loses the user-supplied debug name.🔧 Proposed fix
Provider<T> _generateIntermediateProvider(A arg) => Provider<T>( (context) => _createValue(context, arg), dispose: _disposeValue, + debugName: debugName, );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/disco/lib/src/models/providers/arg_provider.dart` around lines 79 - 84, Update _generateIntermediateProvider in the argument provider implementation to forward the provider’s existing debugName when constructing the intermediate Provider, matching Provider._generateIntermediateProvider and preserving the user-supplied name for disposal error reporting.
🧹 Nitpick comments (5)
examples/solidart/test/widget_test.dart (1)
24-26: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueConsider disposing the mocked controller.
The mock provider declares no
dispose. The mock'sdisposereplaces the original one, sotodosControlleris never disposed when the scope unmounts. Signal-based controllers usually need disposal at teardown.Add a
disposecallback ifTodosControllerowns resources.♻️ Proposed change
todosControllerProvider.overrideWith( - Provider((context) => todosController), + Provider( + (context) => todosController, + dispose: (controller) => controller.dispose(), + ), ),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/solidart/test/widget_test.dart` around lines 24 - 26, Update the todosControllerProvider override to declare a dispose callback that disposes todosController when the provider scope unmounts, preserving the controller’s teardown behavior.packages/disco/lib/src/widgets/provider_scope.dart (1)
229-307: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConfirm that the immutability check is debug-only.
didUpdateWidgetcalls_debugCheckScopeDidNotChangeinside anassert. In release builds the check never runs, so a changed provider list is silently ignored. The doc comment explains the reporting choice but not the debug-only scope. Add that fact to the doc comment so users do not expect the report in profile or release builds.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/disco/lib/src/widgets/provider_scope.dart` around lines 229 - 307, Update the documentation for _debugCheckScopeDidNotChange to explicitly state that the immutability check and its reported error run only in debug builds, not profile or release builds. Keep the existing explanation of reporting versus throwing unchanged.packages/disco/example/lib/main.dart (1)
115-120: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a
disposecallback tomodelProvider.
ModelImplementationextendsModel, which is aChangeNotifier.HomePageprovidesmodelProviderin a page-scopedProviderScopeat line 230, so the value is discarded when the page is removed, butdispose()is never called on it.loggerProviderandcartProviderboth pass adisposecallback. Making this provider consistent avoids teaching a leak in the canonical example.♻️ Proposed change
final modelProvider = Provider<Model>( (context) => ModelImplementation(), + dispose: (model) => model.dispose(), debugName: 'model', );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/disco/example/lib/main.dart` around lines 115 - 120, Add a dispose callback to modelProvider that invokes dispose on its ModelImplementation/Model value, matching the existing cleanup pattern used by loggerProvider and cartProvider while preserving lazy creation and the current provider API.packages/disco/benchmark/provider_benchmark.dart (2)
52-71: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove provider construction outside the stopwatch.
This benchmark measures registration. The stopwatch starts at line 53, before
List.generatebuilds the 100Providerinstances at lines 55-61, so the reported time also includes provider construction. Every other benchmark builds the providers before starting the stopwatch. Align this one so the numbers stay comparable.♻️ Proposed change
testWidgets('Benchmark: Register 100 providers', (tester) async { - final stopwatch = Stopwatch()..start(); - final providers = List.generate( 100, (i) => Provider( (_) => 'Value$i', debugName: 'provider$i', ), ); + + final instantiatedProviders = _instantiateAll(providers); + + final stopwatch = Stopwatch()..start(); await tester.pumpWidget( MaterialApp( home: ProviderScope( - providers: _instantiateAll(providers), + providers: instantiatedProviders, // No value is created, since nothing is injected. child: Container(),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/disco/benchmark/provider_benchmark.dart` around lines 52 - 71, Move the `providers` List.generate construction in the `Benchmark: Register 100 providers` test before `Stopwatch` is started, so the timed section measures only provider registration through `pumpWidget` and remains comparable with the other benchmarks.
152-177: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winThe "Retrieve" benchmark measures first creation, not retrieval.
Values are created lazily on first injection, as the file comment at lines 10-13 states. The builder at line 166 is the first injection point, so the loop at lines 170-172 creates the 100 values. The result duplicates "Create 100 provider values" and reports nothing about cached lookup cost. Inject the providers once before the timed loop, then measure a second pass.
♻️ Proposed change
child: Builder( builder: (context) { + // Create all the values first, so that the loop below + // measures the retrieval of already created values. + for (final provider in providers) { + provider.of(context); + } + final stopwatch = Stopwatch()..start(); - // Access all providers to trigger creation + // Access all providers again: no value is created here. for (final provider in providers) { provider.of(context); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/disco/benchmark/provider_benchmark.dart` around lines 152 - 177, Update the “Retrieve 100 provider values” benchmark so the providers are first accessed once before starting the Stopwatch, allowing lazy creation to complete outside the measurement. Keep the timed loop as a second pass over providers using provider.of(context), and continue recording the elapsed time under the existing benchmark result key.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/src/content/docs/core/immutability.mdx`:
- Line 92: Update the comment describing ProviderScope’s debug-mode behavior to
state that it reports an error and continues with the original providers, rather
than saying it throws when showDetails changes.
In `@docs/src/content/docs/core/provider-retrieval-process.mdx`:
- Line 21: Update the complexity aside in the provider retrieval process
documentation to distinguish the O(1) lookup within each ProviderScope from the
overall worst-case traversal cost of O(number of traversed ProviderScope
ancestors).
In `@docs/src/content/docs/core/scoped-di.mdx`:
- Around line 45-49: Update the provider invocation description in the Aside to
state that calling a provider returns the binding used to register it with
ProviderScope, rather than instantiating the provider or creating its value.
Clarify that the provider value is created lazily when injected, while
preserving the explanation about passing arguments and consistent syntax.
In `@docs/src/content/docs/core/testing.mdx`:
- Around line 8-12: Update the testing documentation around overrideWith to
state that the replacement must use the same provider kind as the provider being
overridden, including using an argument provider for Provider.withArgument;
preserve the existing explanation of override lifecycle and behavior.
In `@packages/disco/.metadata`:
- Line 10: Update the metadata configuration for the published packages/disco
package so it does not reference the example app’s lib/main.dart or
ios/Runner.xcodeproj/project.pbxproj paths. If the metadata is intended for the
Dart example app, move it to the example app’s metadata location instead.
In `@packages/disco/benchmark_results.md`:
- Around line 9-21: Regenerate packages/disco/benchmark_results.md from the
current benchmark output produced by _writeBenchmarkResults in
provider_benchmark.dart. Ensure the rows use the emitted keys, including
Register 100 providers, Create 100 provider values, Create 50 values with
dependencies, Retrieve 100 provider values, and Create 100 ArgProvider values,
and remove the obsolete Mixed lazy and eager (100 total) entry; commit the
updated generated results.
In `@packages/disco/CHANGELOG.md`:
- Line 4: Update the changelog entry to state that overrideWithValue is
deprecated rather than removed, and recommend migrating to overrideWith while
preserving the existing explanation of provider overriding behavior.
In `@packages/disco/example/lib/main.dart`:
- Around line 132-141: Update the documentation around analyticsProvider to
remove the ProviderForwardReferenceError reference. Either remove this
forward-reference example or describe only the requirement that loggerProvider
precede analyticsProvider in the providers list, without mentioning the removed
exception.
In `@packages/disco/lib/src/models/override.dart`:
- Around line 3-5: Update the dartdoc description in the override configuration
declaration to change “an mock provider” to “a mock provider,” without altering
the surrounding documentation.
In `@packages/disco/lib/src/models/providers/provider.dart`:
- Around line 72-81: The removed overrideWithValue API must remain available as
deprecated wrappers for gradual migration. In
packages/disco/lib/src/models/providers/provider.dart:72-81, add deprecated
overrideWithValue(T value) forwarding through overrideWith(Provider<T>((_) =>
value)); in packages/disco/lib/src/models/providers/arg_provider.dart:34-38, add
the equivalent deprecated wrapper forwarding through
overrideWith(Provider.withArgument<T, A>((_, _) => value)).
In `@packages/disco/pubspec.yaml`:
- Line 14: Update the Flutter SDK constraint in the pubspec configuration to the
lowest released version supported by the package, unless Disco depends on APIs
introduced in Flutter 3.38.0; if that dependency is intentional, document the
specific requirement instead. Preserve compatibility with the workspace
packages’ broader constraints.
In `@packages/disco/test/disco_test.dart`:
- Around line 2134-2138: Update the comment above the second buildTree call to
replace the stale “InstantiableProviders” type name with the current
“ValueBinding” name, without changing the test behavior.
---
Outside diff comments:
In `@packages/disco/lib/src/models/providers/arg_provider.dart`:
- Around line 79-84: Update _generateIntermediateProvider in the argument
provider implementation to forward the provider’s existing debugName when
constructing the intermediate Provider, matching
Provider._generateIntermediateProvider and preserving the user-supplied name for
disposal error reporting.
In `@packages/disco/test/disco_test.dart`:
- Around line 729-784: Rename one of the duplicate testWidgets cases around the
ProviderScopeOverride tests so each test name uniquely identifies its scenario,
while preserving both test bodies and their existing assertions.
---
Nitpick comments:
In `@examples/solidart/test/widget_test.dart`:
- Around line 24-26: Update the todosControllerProvider override to declare a
dispose callback that disposes todosController when the provider scope unmounts,
preserving the controller’s teardown behavior.
In `@packages/disco/benchmark/provider_benchmark.dart`:
- Around line 52-71: Move the `providers` List.generate construction in the
`Benchmark: Register 100 providers` test before `Stopwatch` is started, so the
timed section measures only provider registration through `pumpWidget` and
remains comparable with the other benchmarks.
- Around line 152-177: Update the “Retrieve 100 provider values” benchmark so
the providers are first accessed once before starting the Stopwatch, allowing
lazy creation to complete outside the measurement. Keep the timed loop as a
second pass over providers using provider.of(context), and continue recording
the elapsed time under the existing benchmark result key.
In `@packages/disco/example/lib/main.dart`:
- Around line 115-120: Add a dispose callback to modelProvider that invokes
dispose on its ModelImplementation/Model value, matching the existing cleanup
pattern used by loggerProvider and cartProvider while preserving lazy creation
and the current provider API.
In `@packages/disco/lib/src/widgets/provider_scope.dart`:
- Around line 229-307: Update the documentation for _debugCheckScopeDidNotChange
to explicitly state that the immutability check and its reported error run only
in debug builds, not profile or release builds. Keep the existing explanation of
reporting versus throwing unchanged.
🪄 Autofix
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 912adfe3-d119-4d43-809e-9275f6877ac8
📒 Files selected for processing (46)
docs/astro.config.mjsdocs/src/content/docs/core/configuration.mddocs/src/content/docs/core/immutability.mdxdocs/src/content/docs/core/modals.mdxdocs/src/content/docs/core/provider-retrieval-process.mdxdocs/src/content/docs/core/providers.mdxdocs/src/content/docs/core/scoped-di.mdxdocs/src/content/docs/core/testing.mdxdocs/src/content/docs/examples/auto-route.mdxdocs/src/content/docs/examples/basic.mdxdocs/src/content/docs/examples/bloc.mdxdocs/src/content/docs/examples/solidart.mdxdocs/src/content/docs/index.mdxdocs/src/content/docs/installing.mddocs/src/content/docs/miscellaneous/comparison-with-alternatives.mdxdocs/src/content/docs/miscellaneous/reactivity.mdexamples/auto_route/lib/pages/books.dartexamples/bloc/lib/main.dartexamples/solidart/lib/pages/todos.dartexamples/solidart/lib/widgets/todos_body.dartexamples/solidart/test/widget_test.dartpackages/disco/.gitignorepackages/disco/.metadatapackages/disco/CHANGELOG.mdpackages/disco/README.mdpackages/disco/benchmark/provider_benchmark.dartpackages/disco/benchmark_results.mdpackages/disco/example/README.mdpackages/disco/example/analysis_options.yamlpackages/disco/example/lib/main.dartpackages/disco/example/test/disco_test.dartpackages/disco/lib/src/disco_internal.dartpackages/disco/lib/src/models/override.dartpackages/disco/lib/src/models/overrides/override.dartpackages/disco/lib/src/models/overrides/provider_argument_override.dartpackages/disco/lib/src/models/overrides/provider_override.dartpackages/disco/lib/src/models/providers/arg_provider.dartpackages/disco/lib/src/models/providers/instantiable_provider.dartpackages/disco/lib/src/models/providers/provider.dartpackages/disco/lib/src/models/value_binding.dartpackages/disco/lib/src/utils/disco_config.dartpackages/disco/lib/src/utils/extensions.dartpackages/disco/lib/src/widgets/provider_scope.dartpackages/disco/lib/src/widgets/provider_scope_override.dartpackages/disco/pubspec.yamlpackages/disco/test/disco_test.dart
💤 Files with no reviewable changes (7)
- docs/src/content/docs/core/configuration.md
- packages/disco/lib/src/models/overrides/provider_argument_override.dart
- packages/disco/lib/src/utils/disco_config.dart
- packages/disco/lib/src/models/providers/instantiable_provider.dart
- docs/astro.config.mjs
- packages/disco/lib/src/models/overrides/override.dart
- packages/disco/lib/src/models/overrides/provider_override.dart
| Since this would silently surface much later as a `ProviderWithoutScopeError`, it is reported as an error in debug mode: | ||
|
|
||
| ```dart | ||
| // Throws in debug mode as soon as `showDetails` changes. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Correct the debug-mode behavior.
Line 92 says that this code throws. ProviderScope reports an error and continues with its original providers. Update the comment to say that it reports an error.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/src/content/docs/core/immutability.mdx` at line 92, Update the comment
describing ProviderScope’s debug-mode behavior to state that it reports an error
and continues with the original providers, rather than saying it throws when
showDetails changes.
| 3. If the provider is found, its value is returned. If the value has not been created yet (i.e. it is the first time the provider is injected), it gets created right before it is returned. | ||
|
|
||
| 4. Otherwise, the search continues for the first `ProviderScope` ancestor. | ||
| 4. If the provider is not found, the search proceeds to the next `ProviderScope` ancestor, continuing recursively up the widget tree until the root is reached. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Correct the lookup complexity claim.
This step can traverse multiple ProviderScope ancestors. The map lookup is O(1), but the complete lookup is O(number of traversed scopes) in the worst case. Update the following complexity aside.
🧰 Tools
🪛 LanguageTool
[style] ~21-~21: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...reated right before it is returned. 4. If the provider is not found, the search p...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/src/content/docs/core/provider-retrieval-process.mdx` at line 21, Update
the complexity aside in the provider retrieval process documentation to
distinguish the O(1) lookup within each ProviderScope from the overall
worst-case traversal cost of O(number of traversed ProviderScope ancestors).
| <Aside> | ||
| Note the parentheses: a provider is never inserted into a `ProviderScope` | ||
| directly. Calling it is what tells the scope to instantiate it, and it is | ||
| also what allows an argument to be passed. This way, the syntax is the same | ||
| for both kinds of providers. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Correct the provider invocation description.
Calling a provider returns the binding that registers it with ProviderScope. It does not create the provider value. The value is created lazily when it is injected.
Proposed fix
- directly. Calling it is what tells the scope to instantiate it, and it is
- also what allows an argument to be passed. This way, the syntax is the same
+ directly. Calling it creates the binding that registers the provider with
+ the scope. The provider value is created lazily when it is injected. Calling
+ it also allows an argument to be passed. This way, the syntax is the same📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Aside> | |
| Note the parentheses: a provider is never inserted into a `ProviderScope` | |
| directly. Calling it is what tells the scope to instantiate it, and it is | |
| also what allows an argument to be passed. This way, the syntax is the same | |
| for both kinds of providers. | |
| <Aside> | |
| Note the parentheses: a provider is never inserted into a `ProviderScope` | |
| directly. Calling it creates the binding that registers the provider with | |
| the scope. The provider value is created lazily when it is injected. Calling | |
| it also allows an argument to be passed. This way, the syntax is the same | |
| for both kinds of providers. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/src/content/docs/core/scoped-di.mdx` around lines 45 - 49, Update the
provider invocation description in the Aside to state that calling a provider
returns the binding used to register it with ProviderScope, rather than
instantiating the provider or creating its value. Clarify that the provider
value is created lazily when injected, while preserving the explanation about
passing arguments and consistent syntax.
| Testing is done with overrides. You need to place a `ProviderScopeOverride` and then specify the `overrides` argument with a list containing the providers followed by `.overrideWith(provider)`. | ||
|
|
||
| An override replaces a provider **entirely**, and not just the value it holds: the provider passed to `overrideWith` is a regular provider, with its own `create` and `dispose`. This means a mock can, for instance, inject other providers through its context, exactly like the provider it replaces. The value of the original provider is never created at all. | ||
|
|
||
| An override also takes the place of the provider it replaces in every `ProviderScope` below the `ProviderScopeOverride`. The mock therefore has the very same lifecycle as the original provider: its value is created lazily where the value of the original provider would have been created, it is disposed when that `ProviderScope` is disposed, and there is one value per `ProviderScope` providing it. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Describe the required matching provider kind.
Line 10 states that overrideWith always receives a regular provider. An override for Provider.withArgument must receive an argument provider, as the later example shows. State that the override must use the same provider kind as the provider it replaces.
Proposed fix
-An override replaces a provider **entirely**, and not just the value it holds: the provider passed to `overrideWith` is a regular provider, with its own `create` and `dispose`.
+An override replaces a provider **entirely**, and not just the value it holds: the provider passed to `overrideWith` has the same provider kind as the provider it replaces, with its own `create` and `dispose`.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Testing is done with overrides. You need to place a `ProviderScopeOverride` and then specify the `overrides` argument with a list containing the providers followed by `.overrideWith(provider)`. | |
| An override replaces a provider **entirely**, and not just the value it holds: the provider passed to `overrideWith` is a regular provider, with its own `create` and `dispose`. This means a mock can, for instance, inject other providers through its context, exactly like the provider it replaces. The value of the original provider is never created at all. | |
| An override also takes the place of the provider it replaces in every `ProviderScope` below the `ProviderScopeOverride`. The mock therefore has the very same lifecycle as the original provider: its value is created lazily where the value of the original provider would have been created, it is disposed when that `ProviderScope` is disposed, and there is one value per `ProviderScope` providing it. | |
| Testing is done with overrides. You need to place a `ProviderScopeOverride` and then specify the `overrides` argument with a list containing the providers followed by `.overrideWith(provider)`. | |
| An override replaces a provider **entirely**, and not just the value it holds: the provider passed to `overrideWith` has the same provider kind as the provider it replaces, with its own `create` and `dispose`. This means a mock can, for instance, inject other providers through its context, exactly like the provider it replaces. The value of the original provider is never created at all. | |
| An override also takes the place of the provider it replaces in every `ProviderScope` below the `ProviderScopeOverride`. The mock therefore has the very same lifecycle as the original provider: its value is created lazily where the value of the original provider would have been created, it is disposed when that `ProviderScope` is disposed, and there is one value per `ProviderScope` providing it. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/src/content/docs/core/testing.mdx` around lines 8 - 12, Update the
testing documentation around overrideWith to state that the replacement must use
the same provider kind as the provider being overridden, including using an
argument provider for Provider.withArgument; preserve the existing explanation
of override lifecycle and behavior.
| revision: "2c9eb20739dfec95e2c74bd3dfa4601b0a8a36aa" | ||
| channel: "stable" | ||
|
|
||
| project_type: app |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Locate .metadata and pubspec files to confirm package versus app layout.
fd -H '^\.metadata$' --type f
fd -H '^pubspec\.yaml$' packages | xargs -I{} sh -c 'echo "== {}"; rg -n "^name:|^publish_to:|flutter:" {} | head -20'
fd -H 'main.dart' packages/disco --type fRepository: our-creativity/disco
Length of output: 758
Fix packages/disco/.metadata to match the package layout.
packages/disco is the published package; its Dart app exists at packages/disco/example. This metadata directory should not declare lib/main.dart or ios/Runner.xcodeproj/project.pbxproj, or should be removed/moved to packages/disco/example/.metadata if it is meant to track the example app.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/disco/.metadata` at line 10, Update the metadata configuration for
the published packages/disco package so it does not reference the example app’s
lib/main.dart or ios/Runner.xcodeproj/project.pbxproj paths. If the metadata is
intended for the Dart example app, move it to the example app’s metadata
location instead.
| /// A provider depending on another provider of the **same** scope. This works | ||
| /// only if `loggerProvider` is declared before this provider in the | ||
| /// `providers` list, otherwise a `ProviderForwardReferenceError` is thrown. | ||
| final analyticsProvider = Provider( | ||
| (context) { | ||
| final logger = loggerProvider.of(context)..log('Analytics created lazily'); | ||
| return Analytics(logger); | ||
| }, | ||
| debugName: 'analytics', | ||
| ); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm the error type exists and is exported from the public API.
rg -n 'ForwardReference' packages/disco/lib
rg -n 'class .*Error' packages/disco/lib/src --glob '*.dart'
fd -H 'disco.dart' packages/disco/lib --type f --exec cat {}Repository: our-creativity/disco
Length of output: 979
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== candidate files mentioning ProviderForwardReferenceError =="
rg -n 'ProviderForwardReferenceError|ForwardReferenceError|ForwardReference' packages || true
echo
echo "== package/lib tree relevant files =="
git ls-files packages/disco/lib | sed -n '1,120p'
echo
echo "== exports with class/type declarations for ProviderForwardReferenceError =="
rg -n 'ProviderForwardReferenceError|class .*Error|type .*Error' packages/disco/lib --glob '*.dart' || true
echo
echo "== analyzer/API symbols availability? =="
if command -v dart >/dev/null 2>&1; then
dart pub get --offline -C packages/disco 2>/tmp/dart-get.log || true
grep -n 'ProviderForwardReferenceError' /tmp/dart-get.log || true
else
echo "dart command not available"
fiRepository: our-creativity/disco
Length of output: 1958
Replace the removed forward-reference error in the example.
Disco 3.0 removes ProviderForwardReferenceError; forward references are now ordered and cannot be relied upon. Remove this misleading example or document the providers order requirement without naming the removed exception.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/disco/example/lib/main.dart` around lines 132 - 141, Update the
documentation around analyticsProvider to remove the
ProviderForwardReferenceError reference. Either remove this forward-reference
example or describe only the requirement that loggerProvider precede
analyticsProvider in the providers list, without mentioning the removed
exception.
| /// A declarative configuration holding all the data needed to | ||
| /// construct and register an mock provider within a | ||
| /// [ProviderScope]. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the article in the dartdoc.
Line 4 reads "an mock provider". Use "a mock provider".
📝 Proposed fix
-/// A declarative configuration holding all the data needed to
-/// construct and register an mock provider within a
-/// [ProviderScope].
+/// A declarative configuration holding all the data needed to
+/// construct and register a mock provider within a
+/// [ProviderScope].📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /// A declarative configuration holding all the data needed to | |
| /// construct and register an mock provider within a | |
| /// [ProviderScope]. | |
| /// A declarative configuration holding all the data needed to | |
| /// construct and register a mock provider within a | |
| /// [ProviderScope]. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/disco/lib/src/models/override.dart` around lines 3 - 5, Update the
dartdoc description in the override configuration declaration to change “an mock
provider” to “a mock provider,” without altering the surrounding documentation.
| // Override ----------------------------------------------------------------- | ||
|
|
||
| /// {@template Provider.overrideWithValue} | ||
| /// {@template Provider.overrideWithProvider} | ||
| /// It creates an override of this provider to be passed to | ||
| /// [ProviderScopeOverride]. | ||
| /// {@endtemplate} | ||
| @visibleForTesting | ||
| ProviderOverride<T> overrideWithValue(T value) => | ||
| ProviderOverride._(this, value); | ||
| ProviderOverride<T> overrideWith( | ||
| Provider<T> override, | ||
| ) => ProviderOverride._withProvider(this, override); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
overrideWithValue was removed from both provider types. The PR objectives state that overrideWithValue must remain available and be marked @Deprecated, so that users can migrate gradually. Both provider classes replaced it with overrideWith and deleted the value-based API. Confirm the removal is intentional for the 3.0 major bump, or restore deprecated wrappers at both sites.
packages/disco/lib/src/models/providers/provider.dart#L72-L81: add a@DeprecatedoverrideWithValue(T value)that forwards tooverrideWith(Provider<T>((_) => value)).packages/disco/lib/src/models/providers/arg_provider.dart#L34-L38: add a@DeprecatedoverrideWithValue(T value)that forwards tooverrideWith(Provider.withArgument<T, A>((_, _) => value)).
📍 Affects 2 files
packages/disco/lib/src/models/providers/provider.dart#L72-L81(this comment)packages/disco/lib/src/models/providers/arg_provider.dart#L34-L38
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/disco/lib/src/models/providers/provider.dart` around lines 72 - 81,
The removed overrideWithValue API must remain available as deprecated wrappers
for gradual migration. In
packages/disco/lib/src/models/providers/provider.dart:72-81, add deprecated
overrideWithValue(T value) forwarding through overrideWith(Provider<T>((_) =>
value)); in packages/disco/lib/src/models/providers/arg_provider.dart:34-38, add
the equivalent deprecated wrapper forwarding through
overrideWith(Provider.withArgument<T, A>((_, _) => value)).
|
|
||
| environment: | ||
| sdk: ^3.10.0 | ||
| flutter: ">=3.38.0" |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Flutter 3.38.0 stable release date
💡 Result:
The Flutter 3.38.0 stable release was officially released on November 12, 2025 [1]. Flutter's official documentation and repository history confirm that this version was part of the stable release cycle [2][3][4]. As of August 9, 2026, this version is no longer the current stable release, having been succeeded by later versions such as Flutter 3.44 [3][5][6].
Citations:
- 1: https://blog.flutter.dev/whats-new-in-flutter-3-38-3f7b258f7228
- 2: https://flutter.googlesource.com/mirrors/flutter.git/+/refs/tags/3.38.0
- 3: https://docs.flutter.dev/release/release-notes
- 4: https://github.com/flutter/website/blob/main/src/content/release/release-notes/index.md
- 5: https://docs.flutter.dev/install/archive
- 6: https://flutter.dev/blog/whats-new-in-flutter-3-44
🏁 Script executed:
#!/bin/bash
# Description: Check the declared SDK constraints across the repo for consistency.
fd -t f 'pubspec.yaml' --exec sh -c 'echo "== $1"; sed -n "1,30p" "$1"' _ {}Repository: our-creativity/disco
Length of output: 4394
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Tracked files under packages/disco:"
git ls-files 'packages/disco/**' | sed -n '1,200p'
echo
echo "Environment constraints in package files:"
rg -n 'environment|flutter| sdk|version:' packages/disco packages/disco/example packages/disco_lint packages/disco_lint/example examples -g 'pubspec.yaml' | sed -n '1,240p'
echo
echo "Imports/dependencies that may indicate API requirements:"
rg -n 'api_key|requireRuntimePermission|permission|permission_handler|permission_handler_android|permission_handler|permission_handler_platform_interface' packages/disco packages/disco_lint examples -g '*.dart' -g 'pubspec.yaml' | sed -n '1,300p'Repository: our-creativity/disco
Length of output: 5427
Lower or document the Flutter constraint.
packages/disco/pubspec.yaml pins Flutter to ">=3.38.0" while other workspace packages do not declare a Flutter constraint. If this package does not use APIs added in Flutter 3.38.0, lower the bound to keep the package compatible with released Flutter versions.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/disco/pubspec.yaml` at line 14, Update the Flutter SDK constraint in
the pubspec configuration to the lowest released version supported by the
package, unless Disco depends on APIs introduced in Flutter 3.38.0; if that
dependency is intentional, document the specific requirement instead. Preserve
compatibility with the workspace packages’ broader constraints.
| await tester.pumpWidget(buildTree(reversed: false)); | ||
| // A brand new list, holding brand new InstantiableProviders, is built | ||
| // every time: only the providers they refer to matter. Since the order | ||
| // does not matter either, reordering them is not a change. | ||
| await tester.pumpWidget(buildTree(reversed: true)); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Update the stale type name in the comment.
The comment says "brand new InstantiableProviders". The commit messages state that InstantiableProvider was renamed to ValueBinding. Use the current name.
📝 Proposed fix
- // A brand new list, holding brand new InstantiableProviders, is built
+ // A brand new list, holding brand new ValueBindings, is built
// every time: only the providers they refer to matter. Since the order
// does not matter either, reordering them is not a change.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| await tester.pumpWidget(buildTree(reversed: false)); | |
| // A brand new list, holding brand new InstantiableProviders, is built | |
| // every time: only the providers they refer to matter. Since the order | |
| // does not matter either, reordering them is not a change. | |
| await tester.pumpWidget(buildTree(reversed: true)); | |
| await tester.pumpWidget(buildTree(reversed: false)); | |
| // A brand new list, holding brand new ValueBindings, is built | |
| // every time: only the providers they refer to matter. Since the order | |
| // does not matter either, reordering them is not a change. | |
| await tester.pumpWidget(buildTree(reversed: true)); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/disco/test/disco_test.dart` around lines 2134 - 2138, Update the
comment above the second buildTree call to replace the stale
“InstantiableProviders” type name with the current “ValueBinding” name, without
changing the test behavior.
Fixes Applied SuccessfullyFixed 13 file(s) based on 12 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 13 file(s) based on 12 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
There was a problem hiding this comment.
Pull request overview
Introduces Disco 3.0’s provider-binding API and full-provider overrides, including dynamic argument-provider mocks.
Changes:
- Adds
overrideWith, lazy-only creation, and explicit provider bindings. - Adds circular-dependency detection and revised lifecycle handling.
- Updates tests, examples, benchmarks, and documentation.
Reviewed changes
Copilot reviewed 46 out of 46 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
packages/disco/test/disco_test.dart |
Expands provider lifecycle and override tests. |
packages/disco/README.md |
Documents provider bindings. |
packages/disco/pubspec.yaml |
Bumps package to 3.0.0. |
packages/disco/lib/src/widgets/provider_scope.dart |
Reworks registration, lookup, creation, and disposal. |
packages/disco/lib/src/widgets/provider_scope_override.dart |
Privatizes override-scope state access. |
packages/disco/lib/src/utils/extensions.dart |
Makes debug names non-nullable. |
packages/disco/lib/src/utils/disco_config.dart |
Removes global lazy configuration. |
packages/disco/lib/src/models/value_binding.dart |
Adds provider binding models. |
packages/disco/lib/src/models/providers/provider.dart |
Adds bindings and provider overrides. |
packages/disco/lib/src/models/providers/instantiable_provider.dart |
Removes the previous binding abstraction. |
packages/disco/lib/src/models/providers/arg_provider.dart |
Adds argument-provider overrides and bindings. |
packages/disco/lib/src/models/overrides/provider_override.dart |
Removes the old value override model. |
packages/disco/lib/src/models/overrides/provider_argument_override.dart |
Removes the old argument override model. |
packages/disco/lib/src/models/overrides/override.dart |
Removes the old override base class. |
packages/disco/lib/src/models/override.dart |
Consolidates provider override models. |
packages/disco/lib/src/disco_internal.dart |
Updates library parts. |
packages/disco/example/test/disco_test.dart |
Migrates example override tests. |
packages/disco/example/README.md |
Documents example features. |
packages/disco/example/lib/main.dart |
Expands the lifecycle demonstration app. |
packages/disco/example/analysis_options.yaml |
Adjusts example analysis settings. |
packages/disco/CHANGELOG.md |
Records the 3.0.0 changes. |
packages/disco/benchmark/provider_benchmark.dart |
Updates benchmarks for lazy bindings. |
packages/disco/benchmark_results.md |
Adds current benchmark results. |
packages/disco/.metadata |
Adds Flutter project metadata. |
packages/disco/.gitignore |
Adds package-level exclusions. |
examples/solidart/test/widget_test.dart |
Migrates Solidart overrides. |
examples/solidart/lib/widgets/todos_body.dart |
Migrates provider registration. |
examples/solidart/lib/pages/todos.dart |
Migrates provider registration. |
examples/bloc/lib/main.dart |
Migrates provider registration. |
examples/auto_route/lib/pages/books.dart |
Migrates provider registration. |
docs/src/content/docs/miscellaneous/reactivity.md |
Updates registration syntax. |
docs/src/content/docs/miscellaneous/comparison-with-alternatives.mdx |
Updates registration syntax. |
docs/src/content/docs/installing.md |
Updates SDK and package versions. |
docs/src/content/docs/index.mdx |
Updates introductory syntax. |
docs/src/content/docs/examples/solidart.mdx |
Updates the Solidart guide. |
docs/src/content/docs/examples/bloc.mdx |
Updates the BLoC guide. |
docs/src/content/docs/examples/basic.mdx |
Updates the basic guide. |
docs/src/content/docs/examples/auto-route.mdx |
Updates the AutoRoute guide. |
docs/src/content/docs/core/testing.mdx |
Documents full-provider overrides. |
docs/src/content/docs/core/scoped-di.mdx |
Documents dependency and disposal behavior. |
docs/src/content/docs/core/providers.mdx |
Documents lazy creation and overrides. |
docs/src/content/docs/core/provider-retrieval-process.mdx |
Explains the revised lookup model. |
docs/src/content/docs/core/modals.mdx |
Updates registration syntax. |
docs/src/content/docs/core/immutability.mdx |
Documents fixed provider sets. |
docs/src/content/docs/core/configuration.md |
Removes obsolete lazy configuration. |
docs/astro.config.mjs |
Removes the configuration page from navigation. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
| final cycleStart = _idsBeingCreated.indexOf(id); | ||
| if (cycleStart >= 0) { | ||
| throw ProviderCircularDependencyError([ | ||
| ..._idsBeingCreated.skip(cycleStart), | ||
| id, |
| - **CHORE**: The bookkeeping of `ProviderScopeState` (its maps of providers and values, and its lookup and creation methods) and `ProviderScopeOverrideState.providerScopeState` are now library-private. | ||
| - **CHORE**: Rename `InstantiableProvider` to `ValueBinding`. |
| for (final item in scope._overrides!) { | ||
| if (item is ProviderOverride) { | ||
| ids.add(item._originalProvider); | ||
| } else if (item is ArgProviderOverride) { | ||
| ids.add(item._originalArgProvider); | ||
| } | ||
| } |
|
|
||
| ### Disposal order | ||
|
|
||
| Because the values are created lazily, a value is always created *after* the values it depends on. When a `ProviderScope` is disposed, its values are therefore disposed in the reverse order of creation, so that a value is always disposed *before* its own dependencies. This means that the `dispose` of a provider can safely use the values it injected in its `create`. |
This PR attempts to solve #35.
Tl;dr: I propose to remove
overrideWithValuein favour ofoverrideWithProvider. For example, instead of:one should write
Explanation: this second variant can do everything the first variant can do. It is just more powerful, as also
lazy, a propercreateanddisposecan be specified.This is even more useful when working with argument providers, whose mocking functionality is currently not ideal IMO. At the moment, one can only override the whole argument provider, but cannot make argument distinctions. This defeats the point of argument providers. Here is how it currently is on
main:disco/packages/disco/test/disco_test.dart
Lines 728 to 753 in 738b1ea
So, we definitely need arguments. In my current implementation, they would work like this:
However, I don't think this approach is ideal. People will have to write some custom logic somewhere every time they want to use the override (because they need to use specific arguments and provide specific values for that).
We should purse this approach instead:
This way, one can just write the custom logic in
mockNumberProvider'screateanddispose, without polluting the widget tree. This is also what was asked in the linked issue.The logic for this would still have to be adapted in
ProviderScope. Before I invest time there, is this okay for you @nank1ro?Right now, I added sealed classes and pattern matching to allow both
overrideWithValueandoverrideWithProvider, but if we get rid of the former, we don't need the sealed classes and pattern matching anymore.Summary by CodeRabbit