Tiny cleanup for Navigator code #125099
Tiny cleanup for Navigator code #125099
Conversation
|
can you rebase off latest tip of tree? it should fix the ci |
| import 'dart:developer' as developer; | ||
| import 'dart:ui' as ui; | ||
|
|
||
| import 'package:collection/collection.dart'; |
There was a problem hiding this comment.
we try to avoid depending on other packages in this repo. ideally we should remove all the dependencies on package:collection rather than adding more.
There was a problem hiding this comment.
sounds good to me. In that case, I don't think this change worth making
There was a problem hiding this comment.
Hmm, then what about adding the firstWhereOrNull as a (private) extension method?
extension<T> on List<T> {
firstWhereOrNull() => ...;
}
By doing so, IMHO the codebase is still a bit neater, since (1) collection package is well-known so people feel familiar when reading firstWhereOrNull and knows what it is (2) the firstWhereOrNull name looks a bit cleaner than .cast().where(... orElse: null) and introduce less cognitive overhead.
It is just like extracting the common parts (repeated 10 times in this file) into an (extension) function.
| import 'dart:developer' as developer; | ||
| import 'dart:ui' as ui; | ||
|
|
||
| import 'package:collection/collection.dart'; |
There was a problem hiding this comment.
sounds good to me. In that case, I don't think this change worth making
|
closing this pr per comment above |
|
(replied to above) |
…5628) Close #125724 > The `navigator.dart` has ~10 repeats of things like: > > ```dart > final _RouteEntry? currentRouteEntry = _navigator!._history.cast<_RouteEntry?>().lastWhere( > (_RouteEntry? e) => e != null && _RouteEntry.isPresentPredicate(e), > orElse: () => null, > ); > ``` > > while it can be greatly simplified as: > > ```dart > final _RouteEntry? currentRouteEntry = _navigator!._history.lastWhereOrNull(_RouteEntry.isPresentPredicate); > ``` > > Thus, it seems that we can beautify the code a little bit. Same as #125099, but without external dependency For more detailed explanation why it does not introduce external dependency: #125099 (comment)
The code is self-explainable :) Just use
firstWhereOrNullto cleanup the code.List which issues are fixed by this PR. You must list at least one issue.
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.