Skip to content

Add dispose to FutureProvider #912

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add dispose to FutureProvider #912

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions packages/provider/lib/src/async_provider.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,10 +181,15 @@ class FutureProvider<T> extends DeferredInheritedProvider<Future<T>?, T> {
/// Creates a [Future] from `create` and subscribes to it.
///
/// `create` must not be `null`.
///
/// The optional [dispose] callback is invoked with the last exposed value
/// when the provider is removed from the widget tree, allowing cleanup of
/// resources (closing streams, cancelling timers, etc.).
FutureProvider({
Key? key,
required Create<Future<T>?> create,
required T initialData,
Dispose<T>? dispose,
Copy link
Owner

@rrousselGit rrousselGit Jun 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is impractical because more often than not, it's not T that needs disposing when using futures/streams. It's something else, like a Completer/StreamController/...

Copy link
Author

@RossComputerGuy RossComputerGuy Jun 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, the problem I was running into was that if it were a Future then I didn't have access to the data since it was already completed. How would I get around this to work for Completer or a StreamController?

Copy link
Owner

@rrousselGit rrousselGit Jun 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no easy solution with the API that Provider offers. We'd have to make a breaking change and do:

StreamProvider(create: (context, ref) {
  final controller = StreamController()
  ref.onDispose(() => controller.close());
  return controller.stream;
})

That's the API Riverpod uses.

ErrorBuilder<T>? catchError,
UpdateShouldNotify<T>? updateShouldNotify,
bool? lazy,
Expand All @@ -195,6 +200,7 @@ class FutureProvider<T> extends DeferredInheritedProvider<Future<T>?, T> {
lazy: lazy,
builder: builder,
create: create,
dispose: dispose,
updateShouldNotify: updateShouldNotify,
startListening: _futureStartListening(
catchError: catchError,
Expand Down
12 changes: 9 additions & 3 deletions packages/provider/lib/src/deferred_inherited_provider.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class DeferredInheritedProvider<T, R> extends InheritedProvider<R> {
DeferredInheritedProvider({
Key? key,
required Create<T> create,
Dispose<T>? dispose,
Dispose<R>? dispose,
required DeferredStartListening<T, R> startListening,
UpdateShouldNotify<R>? updateShouldNotify,
bool? lazy,
Expand Down Expand Up @@ -165,7 +165,7 @@ class _CreateDeferredInheritedProvider<T, R> extends _DeferredDelegate<T, R> {
}) : super(updateShouldNotify, startListening);

final Create<T> create;
final Dispose<T>? dispose;
final Dispose<R>? dispose;

@override
_CreateDeferredInheritedProviderElement<T, R> createState() {
Expand Down Expand Up @@ -220,7 +220,13 @@ class _CreateDeferredInheritedProviderElement<T, R>
void dispose() {
super.dispose();
if (_didBuild) {
delegate.dispose?.call(element!, _controller as T);
if (T is Future<R>) {
(_controller as Future<R>).then((value) => delegate.dispose?.call(element!, value));
} else if (isLoaded) {
delegate.dispose?.call(element!, value);
} else {
delegate.dispose?.call(element!, _controller as R);
}
Comment on lines 221 to +229
Copy link

@coderabbitai coderabbitai bot Jun 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Compile-time error & wrong cast inside _CreateDeferredInheritedProviderElement.dispose

  1. if (T is Future<R>) is invalid Dart – the left-hand side of is must be an expression, not a type parameter.
  2. delegate.dispose?.call(element!, _controller as R); will throw at runtime when T ≠ R (e.g. StreamProvider<Stream<int>, int>), because _controller is a Stream<int> while R is int.

Suggested fix:

-      if (T is Future<R>) {
-        (_controller as Future<R>).then((value) => delegate.dispose?.call(element!, value));
+      if (_controller is Future<R>) {
+        // FutureProvider – dispose after the Future completes.
+        (_controller as Future<R>).then(
+          (resolved) => delegate.dispose?.call(element!, resolved),
+          onError: (_) {}, // ignore errors, we cannot dispose a failed value
+        );
       } else if (isLoaded) {
         delegate.dispose?.call(element!, value);
       } else {
-        delegate.dispose?.call(element!, _controller as R);
+        // No value produced; fall back to disposing the controller itself.
+        // Cast to `dynamic` to avoid a bad runtime cast when T ≠ R.
+        delegate.dispose?.call(element!, _controller as dynamic);
       }

This eliminates the compile error and prevents a potential TypeError at runtime.

📝 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.

Suggested change
super.dispose();
if (_didBuild) {
delegate.dispose?.call(element!, _controller as T);
if (T is Future<R>) {
(_controller as Future<R>).then((value) => delegate.dispose?.call(element!, value));
} else if (isLoaded) {
delegate.dispose?.call(element!, value);
} else {
delegate.dispose?.call(element!, _controller as R);
}
super.dispose();
if (_didBuild) {
if (_controller is Future<R>) {
// FutureProvider – dispose after the Future completes.
(_controller as Future<R>).then(
(resolved) => delegate.dispose?.call(element!, resolved),
onError: (_) {}, // ignore errors, we cannot dispose a failed value
);
} else if (isLoaded) {
delegate.dispose?.call(element!, value);
} else {
// No value produced; fall back to disposing the controller itself.
// Cast to `dynamic` to avoid a bad runtime cast when T ≠ R.
delegate.dispose?.call(element!, _controller as dynamic);
}
🤖 Prompt for AI Agents 
In packages/provider/lib/src/deferred_inherited_provider.dart around lines 221
to 229, the code incorrectly uses a type check with a type parameter in `if (T
is Future<R>)`, which is invalid in Dart, and performs an unsafe cast
`_controller as R` that can cause runtime errors when T and R differ. To fix
this, replace the invalid type check with a runtime type check on the actual
instance held by _controller, such as using `_controller is Future<R>`, and
avoid casting _controller directly to R; instead, ensure the correct value is
passed to delegate.dispose by handling each case based on the runtime type of
_controller or by restructuring the logic to safely extract the value without
unsafe casts.

}
}

Expand Down