Fix SplitPane RangeError when child count changes between rebuilds#9822
Open
ishaquehassan wants to merge 2 commits intoflutter:masterfrom
Open
Conversation
Fixes flutter#9648. SplitPane cached its fractions list in initState only. When the parent rebuilt the widget with a different number of children (for example, toggling a panel via a collection-if), fractions.length stayed at the old value while widget.minSizes and widget.children shrank, causing minSizeForIndex to read past the end of widget.minSizes and throw 'RangeError (index): Index out of range: index should be less than 2: 2' from the layout pass. This adds didUpdateWidget to _SplitPaneState. When the child count changes, fractions is reset to List.of(widget.initialFractions) so it stays in sync with the new children and minSizes. The existing constructor assertion already guarantees children.length matches initialFractions.length. Bumps devtools_app_shared to 0.5.2 with a CHANGELOG entry, and adds regression tests that pump a 3-child SplitPane and then a 2-child SplitPane (and vice versa) and assert no exception is thrown.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request addresses a RangeError in the SplitPane widget triggered by changes in the number of children during rebuilds. The fix modifies _SplitPaneState to reset the fractions list within didUpdateWidget whenever the child count changes. The devtools_app_shared package version is updated to 0.5.2, and new tests are included to ensure stability when the number of children grows or shrinks. I have no feedback to provide.
Member
kenzieschmoll
left a comment
kenzieschmoll
left a comment
There was a problem hiding this comment.
please add a release note entry for this change.
Author
|
@kenzieschmoll yeah calling a RangeError 'not user facing' was a stretch on my part 😅. Added a General updates entry and corrected the PR body. Pushed. |
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 #9648.
SplitPanecached itsfractionslist ininitStateonly. When the parent rebuilt the widget with a different number of children (for example, toggling a panel in or out with a collection-if),fractions.lengthstayed at the old value whilewidget.minSizesandwidget.childrenshrank. The next layout pass calledminSizeForIndexwith an index past the end ofwidget.minSizesand threwRangeError (index): Index out of range: index should be less than 2: 2fromsplit_pane.dart:126.This adds
didUpdateWidgetto_SplitPaneState. When the child count changes,fractionsis reset toList.of(widget.initialFractions)so it stays in sync with the newchildrenandminSizes. The existing constructor assertion (children.length == initialFractions.length) already guarantees the new list has the right length, so no extra defensive logic is needed.fractionslost itsfinalmodifier to allow the reassignment.devtools_app_sharedis bumped to0.5.2and a CHANGELOG entry is added.Tests
Added two regression tests in
split_pane_test.dartunder a newrebuilds with a different number of childrengroup:SplitPane, then pumps a 2-childSplitPanewith the same key path, and asserts no exception is thrownBoth tests reproduce the original
RangeErroron master and pass with the fix.Note: running the local
flutter testagainst the bundled Flutter SDK currently fails to compile because of an unrelated pre-existing error influtter/lib/src/widgets/_window_linux.dart(isSizedToContentgetter is missing onWindowingOwnerLinux). That failure reproduces on master without this change and is independent ofSplitPane. CI should compile against a matching SDK.Pre-launch Checklist
General checklist
///).Issues checklist
contributions-welcomeorgood-first-issuelabel.contributions-welcomeorgood-first-issuelabel. I understand this means my PR might take longer to be reviewed.Tests checklist
AI-tooling checklist
Feature-change checklist
release-notes-not-requiredlabel or left a comment requesting the label be added.packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md.If you need help, consider asking for help on Discord.