Remove unused code under screens/#9911
Conversation
There was a problem hiding this comment.
Code Review
This pull request performs a large-scale cleanup of unused code, imports, and variables across multiple DevTools screens and packages, replacing some assert-based debug initializations with if (kDebugMode) and adding @visibleForTesting annotations. A critical issue was identified in app_size_screen.dart, where removing the value parameter from the stateless DiffTreeTypeDropdown widget introduces a logical bug because it can no longer receive and display the currently selected value from its parent.
| class DiffTreeTypeDropdown extends StatelessWidget { | ||
| const DiffTreeTypeDropdown({ | ||
| super.key, | ||
| required this.value, | ||
| required this.onChanged, | ||
| }); | ||
|
|
||
| final DiffTreeType value; | ||
| final ValueChanged<DiffTreeType?>? onChanged; |
There was a problem hiding this comment.
[MUST-FIX] Removing the value parameter from DiffTreeTypeDropdown is likely a logical bug.
Since DiffTreeTypeDropdown is a StatelessWidget, it has no internal state and must receive the currently selected value from its parent to display the correct selection. If the value field was flagged as unused by the lint, it is highly probable that the build method of DiffTreeTypeDropdown was missing the binding of value to the underlying dropdown widget (e.g., DropdownButton or DevToolsDropdown).
Instead of deleting the value field and parameter, please restore them and ensure that value is properly bound to the dropdown widget inside the build method:
@override
Widget build(BuildContext context) {
return DevToolsDropdown<DiffTreeType>(
value: value, // <-- Ensure this is bound!
onChanged: onChanged,
...
);
}References
- Categorize Severity: Prefix every comment with a severity: [MUST-FIX] for logical bugs. (link)
Work towards #9906