Make future and stream required arguments in their respective builder widgets
#125838
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
flutter-dashboard
bot
added
the
framework
caseycrogers
changed the title
future and stream required arguments in their respective builder widgets
domesticmouse
approved these changes
May 1, 2023
Contributor
domesticmouse
left a comment
domesticmouse
left a comment
There was a problem hiding this comment.
Does this classify as a breaking change?
Contributor
|
PTAL @goderbauer |
Contributor
Author
I figured so as it will throw a compiler error for anyone who has been using the builders with omitted |
Member
All checks are passing (including goole and customer tests), so according to our breaking change policy, this is not considered breaking and we can merge this as is. |
goderbauer
added
the
autosubmit
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
May 2, 2023
…tive builder widgets (flutter/flutter#125838)
auto-submit bot
pushed a commit
to flutter/packages
that referenced
this pull request
May 2, 2023
flutter/flutter@828a040...db6074a 2023-05-02 [email protected] Roll Flutter Engine from 46d5ce4784b8 to 44376e8cd3de (1 revision) (flutter/flutter#125876) 2023-05-02 [email protected] Roll Flutter Engine from a687d62ff631 to 46d5ce4784b8 (1 revision) (flutter/flutter#125874) 2023-05-02 [email protected] Roll Flutter Engine from d0ed35b7bf67 to a687d62ff631 (1 revision) (flutter/flutter#125871) 2023-05-02 [email protected] Roll Flutter Engine from c0d68c36c376 to d0ed35b7bf67 (1 revision) (flutter/flutter#125870) 2023-05-02 [email protected] Roll Flutter Engine from a9e38dd0510d to c0d68c36c376 (1 revision) (flutter/flutter#125868) 2023-05-02 [email protected] Roll Flutter Engine from 024bf946232d to a9e38dd0510d (2 revisions) (flutter/flutter#125866) 2023-05-02 [email protected] Make `future` and `stream` required arguments in their respective builder widgets (flutter/flutter#125838) 2023-05-02 [email protected] Roll Flutter Engine from 7d87410a51d5 to 024bf946232d (6 revisions) (flutter/flutter#125858) 2023-05-02 [email protected] Roll Flutter Engine from 3fa6084dfb36 to 7d87410a51d5 (4 revisions) (flutter/flutter#125845) 2023-05-02 [email protected] Migrate Xcode projects last version checks to Xcode 14.3 (flutter/flutter#125827) 2023-05-01 [email protected] Roll Flutter Engine from 58cc54197a54 to 3fa6084dfb36 (1 revision) (flutter/flutter#125839) 2023-05-01 [email protected] Roll Flutter Engine from 687f4c761db1 to 58cc54197a54 (6 revisions) (flutter/flutter#125833) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
nploi
pushed a commit
to nploi/packages
that referenced
this pull request
Jul 16, 2023
flutter/flutter@828a040...db6074a 2023-05-02 [email protected] Roll Flutter Engine from 46d5ce4784b8 to 44376e8cd3de (1 revision) (flutter/flutter#125876) 2023-05-02 [email protected] Roll Flutter Engine from a687d62ff631 to 46d5ce4784b8 (1 revision) (flutter/flutter#125874) 2023-05-02 [email protected] Roll Flutter Engine from d0ed35b7bf67 to a687d62ff631 (1 revision) (flutter/flutter#125871) 2023-05-02 [email protected] Roll Flutter Engine from c0d68c36c376 to d0ed35b7bf67 (1 revision) (flutter/flutter#125870) 2023-05-02 [email protected] Roll Flutter Engine from a9e38dd0510d to c0d68c36c376 (1 revision) (flutter/flutter#125868) 2023-05-02 [email protected] Roll Flutter Engine from 024bf946232d to a9e38dd0510d (2 revisions) (flutter/flutter#125866) 2023-05-02 [email protected] Make `future` and `stream` required arguments in their respective builder widgets (flutter/flutter#125838) 2023-05-02 [email protected] Roll Flutter Engine from 7d87410a51d5 to 024bf946232d (6 revisions) (flutter/flutter#125858) 2023-05-02 [email protected] Roll Flutter Engine from 3fa6084dfb36 to 7d87410a51d5 (4 revisions) (flutter/flutter#125845) 2023-05-02 [email protected] Migrate Xcode projects last version checks to Xcode 14.3 (flutter/flutter#125827) 2023-05-01 [email protected] Roll Flutter Engine from 58cc54197a54 to 3fa6084dfb36 (1 revision) (flutter/flutter#125839) 2023-05-01 [email protected] Roll Flutter Engine from 687f4c761db1 to 58cc54197a54 (6 revisions) (flutter/flutter#125833) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Aug 16, 2023
…tive builder widgets (flutter/flutter#125838)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Aug 17, 2023
…tive builder widgets (flutter/flutter#125838)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Aug 17, 2023
…tive builder widgets (flutter/flutter#125838)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Aug 17, 2023
…tive builder widgets (flutter/flutter#125838)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
cc'ing existing conversation participants: @domesticmouse @srawlins
cc'ing to request review: @goderbauer
This PR makes the following constructor arguments required:
FutureBuilder.futureStreamBuilderBase.streamStreamBuilder.streamThis fixes:
#83081
#125188 (dupe of 83081)
This obviates:
dart-lang/sdk#59118
(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)
#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
futureorstreamwhen using the respective*Builderwidgets. 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
futureorstreamseems highly niche.The only place I've found non-accidental examples of this is in widget tests where you're calling
pumpWidgetwith and without these arguments to test*Builder.didUpdateWidget's behavior. In this and similar cases, it is a trivial fix to addfuture: 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.
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.