Bump Dart SDK minimums to Dart 3.11.0.#9848
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the Dart SDK constraint to ^3.11.0 across multiple packages, updates various dependencies in pubspec.lock, and refactors map and list literals to use the new null-aware element syntax. Feedback is provided regarding an unnecessary null-aware operator (?) in fake_isolate_manager.dart because _selectedIsolate is statically inferred as non-nullable.
| final value = _selectedIsolate.value; | ||
| _isolates ??= ValueNotifier([if (value != null) value]); | ||
| return _isolates!; | ||
| return _isolates ??= ValueNotifier([?_selectedIsolate.value]); |
There was a problem hiding this comment.
[CONCERN] Statically, _selectedIsolate is inferred as ValueNotifier<IsolateRef> (non-nullable) because it is initialized with a non-null value and has no explicit type arguments. Therefore, _selectedIsolate.value is non-nullable, making the null-aware operator ? unnecessary here.
If _selectedIsolate is intended to be nullable (matching the ValueListenable<IsolateRef?> return type of selectedIsolate), you should explicitly type it as ValueNotifier<IsolateRef?> on line 23. Otherwise, you can remove the ? operator from _selectedIsolate.value in the list literal.
| return _isolates ??= ValueNotifier([?_selectedIsolate.value]); | |
| return _isolates ??= ValueNotifier([_selectedIsolate.value]); |
References
- Maintainability: Code should be easy to understand and modify. Avoiding unnecessary null-aware operators on non-nullable types prevents confusion and potential compiler/lint warnings. (link)
kenzieschmoll
left a comment
There was a problem hiding this comment.
we need to update package versions and add changelog entries for these changes since this will change the backwards compatibility of all the published DevTools packages
| description: Package of shared Dart structures between devtools_app, dds, and other tools. | ||
|
|
||
| version: 13.0.1 | ||
| version: 13.0.2-dev |
There was a problem hiding this comment.
this should be -wip to match the changelog
Bumping the minimum Dart SDK for each package exposes us to the use_null_aware_elements lint rule, allowing for little cleanups.