fix: correct use.stream/future cancellation code (#196)

Fixes #195
GregoryConrad · Jun 5, 2024 · c2e48c0 · c2e48c0
1 parent c35e599
commit c2e48c0
Show file tree

Hide file tree

Showing 2 changed files with 88 additions and 15 deletions.
diff --git a/packages/flutter_rearch/test/side_effects_test.dart b/packages/flutter_rearch/test/side_effects_test.dart
@@ -1,3 +1,5 @@
+import 'dart:async';
+
 import 'package:flutter/gestures.dart';
 import 'package:flutter/material.dart';
 import 'package:flutter_rearch/flutter_rearch.dart';
@@ -98,6 +100,82 @@ void main() {
     expect(find.text('2'), findsOneWidget);
   });
 
+  testWidgets("future won't rebuild after build", (tester) async {
+    final outerCompleter = Completer<void>();
+    final innerCompleter = Completer<void>();
+    await tester.pumpWidget(
+      MaterialApp(
+        home: RearchBootstrapper(
+          child: Scaffold(
+            body: RearchBuilder(
+              builder: (context, use) {
+                final val = use.future(outerCompleter.future);
+                if (val is AsyncLoading) {
+                  return RearchBuilder(
+                    builder: (context, use) {
+                      use.future(innerCompleter.future);
+                      return const CircularProgressIndicator();
+                    },
+                  );
+                } else {
+                  return const Text('switched');
+                }
+              },
+            ),
+          ),
+        ),
+      ),
+    );
+    expect(find.text('switched'), findsNothing);
+
+    outerCompleter.complete(null);
+    await tester.pump();
+    expect(find.text('switched'), findsOneWidget);
+
+    innerCompleter.complete(null);
+    await tester.pump();
+    expect(find.text('switched'), findsOneWidget);
+  });
+
+  testWidgets("stream won't rebuild after build", (tester) async {
+    final outerCompleter = Completer<void>();
+    final innerCompleter = Completer<void>();
+    final outerStream = outerCompleter.future.asStream();
+    final innerStream = innerCompleter.future.asStream();
+    await tester.pumpWidget(
+      MaterialApp(
+        home: RearchBootstrapper(
+          child: Scaffold(
+            body: RearchBuilder(
+              builder: (context, use) {
+                final val = use.stream(outerStream);
+                if (val is AsyncLoading) {
+                  return RearchBuilder(
+                    builder: (context, use) {
+                      use.stream(innerStream);
+                      return const CircularProgressIndicator();
+                    },
+                  );
+                } else {
+                  return const Text('switched');
+                }
+              },
+            ),
+          ),
+        ),
+      ),
+    );
+    expect(find.text('switched'), findsNothing);
+
+    outerCompleter.complete(null);
+    await tester.pump();
+    expect(find.text('switched'), findsOneWidget);
+
+    innerCompleter.complete(null);
+    await tester.pump();
+    expect(find.text('switched'), findsOneWidget);
+  });
+
   testWidgets('PageView control test (default args)', (tester) async {
     final container = useContainer();
 

diff --git a/packages/rearch/lib/src/side_effects.dart b/packages/rearch/lib/src/side_effects.dart
@@ -291,8 +291,7 @@ extension BuiltinSideEffects on SideEffectRegistrar {
   /// To remove this cached data from the returned [AsyncValue],
   /// you may call [AsyncValueConvenience.withoutPreviousData].
   AsyncValue<T>? nullableFuture<T>(Future<T>? future) {
-    // NOTE: we convert to a stream here because we can cancel
-    // a stream subscription; there is no builtin way to cancel a future.
+    // NOTE: we convert to a stream here to reuse our cancellation code.
     final asNullableStream = use.memo(() => future?.asStream(), [future]);
     return use.nullableStream(asNullableStream);
   }
@@ -317,24 +316,20 @@ extension BuiltinSideEffects on SideEffectRegistrar {
       AsyncLoading<T>(None<T>()),
     );
 
-    final (getSubscription, setSubscription) =
-        use.data<StreamSubscription<T>?>(null);
-    use.effect(() => getSubscription()?.cancel, [getSubscription()]);
-
-    final oldStream = use.previous(stream);
-    final needToInitializeState = stream != oldStream;
+    use.effect(
+      () {
+        setValue(AsyncLoading(getValue().data));
 
-    if (needToInitializeState) {
-      setValue(AsyncLoading(getValue().data));
-      setSubscription(
-        stream?.listen(
+        final subscription = stream?.listen(
           (data) => setValue(AsyncData(data)),
           onError: (Object error, StackTrace trace) =>
               setValue(AsyncError(error, trace, getValue().data)),
           cancelOnError: false,
-        ),
-      );
-    }
+        );
+        return () => subscription?.cancel();
+      },
+      [stream],
+    );
 
     return stream == null ? null : getValue();
   }