Unverified Commit 4d86f5d0 authored by Casey Rogers's avatar Casey Rogers Committed by GitHub

Make `future` and `stream` required arguments in their respective builder widgets (#125838)

cc'ing existing conversation participants: @domesticmouse @srawlins
cc'ing to request review: @goderbauer 

This PR makes the following constructor arguments required:
1. `FutureBuilder.future`
2. `StreamBuilderBase.stream`
3. `StreamBuilder.stream`

This fixes:
https://github.com/flutter/flutter/issues/83081
https://github.com/flutter/flutter/issues/125188 (dupe of 83081)

This obviates:
https://github.com/dart-lang/linter/issues/4309
(I suggest we skip straight to merging this PR as this should be a low impact breaking change-assuming few to no devs are intentionally using the builders without their relevant arguments, however we could always merge 4309 first and then this)
https://github.com/flutter/flutter/pull/83101 
(The above PR required that at least one of future and initial data be non-null, this is undesirable as there are plenty of valid reasons to have both arguments be null)

See above issues for a deeper dive, but here is a summary:
It is very easy for a developer to forget to specify `future` or `stream` when using the respective `*Builder` widgets. This produces a non-obvious failure where the UI sits in a "no data yet received" state. It is easy for a dev to misinterpret this as the async work backing the future/stream hanging and they thus waste a lot of time trying to debug the async work.
As such, we should require these two constructor arguments to make it impossible/much harder for devs to make this time-wasting mistake.

This is a breaking change. However, it should break only a small number of active projects given that using a builder without specifying `future` or `stream` seems highly niche.
The only place I've found non-accidental examples of this is in widget tests where you're calling `pumpWidget` with and without these arguments to test `*Builder.didUpdateWidget`'s behavior. In this and similar cases, it is a trivial fix to add `future: null`/`stream: null`.

*If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].*
parent 7907f931
......@@ -40,7 +40,7 @@ import 'framework.dart';
/// recent interaction is needed for widget building.
abstract class StreamBuilderBase<T, S> extends StatefulWidget {
/// Creates a [StreamBuilderBase] connected to the specified [stream].
const StreamBuilderBase({ super.key, this.stream });
const StreamBuilderBase({ super.key, required this.stream });
/// The asynchronous computation to which this builder is currently connected,
/// possibly null. When changed, the current summary is updated using
......@@ -391,7 +391,7 @@ class StreamBuilder<T> extends StreamBuilderBase<T, AsyncSnapshot<T>> {
const StreamBuilder({
super.key,
this.initialData,
super.stream,
required super.stream,
required this.builder,
});
......@@ -521,7 +521,7 @@ class FutureBuilder<T> extends StatefulWidget {
/// The [builder] must not be null.
const FutureBuilder({
super.key,
this.future,
required this.future,
this.initialData,
required this.builder,
});
......
......@@ -105,7 +105,7 @@ void main() {
testWidgets('gracefully handles transition from null future', (WidgetTester tester) async {
final GlobalKey key = GlobalKey();
await tester.pumpWidget(FutureBuilder<String>(
key: key, builder: snapshotText,
key: key, builder: snapshotText, future: null,
));
expect(find.text('AsyncSnapshot<String>(ConnectionState.none, null, null, null)'), findsOneWidget);
final Completer<String> completer = Completer<String>();
......@@ -122,7 +122,7 @@ void main() {
));
expect(find.text('AsyncSnapshot<String>(ConnectionState.waiting, null, null, null)'), findsOneWidget);
await tester.pumpWidget(FutureBuilder<String>(
key: key, builder: snapshotText,
key: key, builder: snapshotText, future: null,
));
expect(find.text('AsyncSnapshot<String>(ConnectionState.none, null, null, null)'), findsOneWidget);
completer.complete('hello');
......@@ -170,6 +170,7 @@ void main() {
final GlobalKey key = GlobalKey();
await tester.pumpWidget(FutureBuilder<String>(
key: key,
future: null,
builder: snapshotText,
initialData: 'I',
));
......@@ -179,6 +180,7 @@ void main() {
final GlobalKey key = GlobalKey();
await tester.pumpWidget(FutureBuilder<String>(
key: key,
future: null,
builder: snapshotText,
initialData: 'I',
));
......@@ -215,7 +217,7 @@ void main() {
testWidgets('gracefully handles transition from null stream', (WidgetTester tester) async {
final GlobalKey key = GlobalKey();
await tester.pumpWidget(StreamBuilder<String>(
key: key, builder: snapshotText,
key: key, builder: snapshotText, stream: null,
));
expect(find.text('AsyncSnapshot<String>(ConnectionState.none, null, null, null)'), findsOneWidget);
final StreamController<String> controller = StreamController<String>();
......@@ -232,7 +234,7 @@ void main() {
));
expect(find.text('AsyncSnapshot<String>(ConnectionState.waiting, null, null, null)'), findsOneWidget);
await tester.pumpWidget(StreamBuilder<String>(
key: key, builder: snapshotText,
key: key, builder: snapshotText, stream: null,
));
expect(find.text('AsyncSnapshot<String>(ConnectionState.none, null, null, null)'), findsOneWidget);
});
......@@ -285,6 +287,7 @@ void main() {
final GlobalKey key = GlobalKey();
await tester.pumpWidget(StreamBuilder<String>(
key: key,
stream: null,
builder: snapshotText,
initialData: 'I',
));
......@@ -335,15 +338,15 @@ void main() {
});
testWidgets('when Future is null', (WidgetTester tester) async {
await tester.pumpWidget(Column(children: <Widget>[
FutureBuilder<String>(builder: snapshotText),
StreamBuilder<String>(builder: snapshotText),
FutureBuilder<String>(builder: snapshotText, future: null),
StreamBuilder<String>(builder: snapshotText, stream: null,),
]));
expect(find.text('AsyncSnapshot<String>(ConnectionState.none, null, null, null)'), findsNWidgets(2));
});
testWidgets('when initialData is used with null Future and Stream', (WidgetTester tester) async {
await tester.pumpWidget(Column(children: <Widget>[
FutureBuilder<String>(builder: snapshotText, initialData: 'I'),
StreamBuilder<String>(builder: snapshotText, initialData: 'I'),
FutureBuilder<String>(builder: snapshotText, initialData: 'I', future: null),
StreamBuilder<String>(builder: snapshotText, initialData: 'I', stream: null),
]));
expect(find.text('AsyncSnapshot<String>(ConnectionState.none, I, null, null)'), findsNWidgets(2));
});
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment