Fix FilePickerWebOptions.readSequential having no effect - #2207
Merged
Merged
Conversation
Fixes #2206. _processSelectedFiles processed the picked FileList with a single for loop that awaited each file in turn, with no branch on readSequential and no parallel code path at all, so the flag never did anything regardless of its value. Confirmed with a reproduction of the exact loop structure: max observed concurrency was 1 in both cases. Extracted the per-file work into an indexed task and added runIndexedTasks, a small internal helper that runs those tasks either one at a time or concurrently via Future.wait, writing results into index-based slots so the output always matches selection order regardless of completion order or concurrency mode. Added indexed_task_runner_test.dart, a plain Dart test (no browser needed) that exercises both modes directly: sequential caps concurrency at 1, concurrent allows more than one in flight, and both preserve order even when completion order is reversed.
navaronbracke
requested changes
Sep 11, 2026
Future.wait() already returns results in the order the futures were passed in, not completion order, so the manual index bookkeeping via a pre-filled nullable list and then() side effects was unnecessary. Return its result directly in the concurrent branch instead, which also removes the need for the trailing cast. Restructured the sequential branch as an early return instead of if/else, and added an explicit early return for length == 0.
Owner
Author
|
Addressed all four comments in 12a8dbf, replied inline on each. Future.wait() already preserves input order, so the manual index writes and trailing cast were unnecessary, simplified to return its result directly. All green (analyze/test/format). |
4 tasks
navaronbracke
approved these changes
Sep 13, 2026
vicajilau
enabled auto-merge
September 13, 2026 09:39
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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.
Fixes #2206.
The bug
FilePickerWeb._processSelectedFilesprocessed the pickedFileListwith a singleforloop that awaited each file in turn, with no branch onreadSequentialanywhere and no parallel code path at all. The flag never did anything regardless of its value. Confirmed by reproducing the exact loop structure against fake delayed reads: max observed concurrency was 1 whetherreadSequentialwastrueorfalse.withData/withReadStreamon the sameFilePickerWebOptionsclass were already correctly wired, this was specific toreadSequential.The fix
Extracted the per-file work into an indexed task and added
runIndexedTasks(indexed_task_runner.dart), a small internal helper that runs those tasks either one at a time or concurrently viaFuture.wait, writing results into index-based slots so the output always matches selection order regardless of completion order or concurrency mode.readSequential: truenow genuinely reads one file at a time.readSequential: false(the default) reads them concurrently. Selection order is preserved in the result either way.Test plan
indexed_task_runner_test.dart, a plain Dart test (no browser needed) exercising both modes directly: sequential caps concurrency at 1, concurrent allows more than one in flight, both preserve order even when completion order is reversedmelos exec -- flutter analyze .clean across all 8 packagesmelos exec -- flutter testall green across all 8 packagesdart format --output=none --set-exit-if-changed .clean