[url_launcher] Migrates Link to null safety#3196
Conversation
| return pushRouteNameToFramework(context, routeName); | ||
| await pushRouteNameToFramework(context, routeName); | ||
| return; |
There was a problem hiding this comment.
Is this nnbd or just better practice? If pushRouteNameToFramework returns Future, why not return that from here? Is it because it's in another package and it's some Future<void>??
There was a problem hiding this comment.
I'm wondering the same thing.
There was a problem hiding this comment.
Otherwise, the analyzer complains:
A value of type 'ByteData' can't be returned from method '_followLink' because it has a return type of 'Future<void>'.dartreturn_of_invalid_type
Also, it doesn't seem like the intention is to return Future<ByteData>.
There was a problem hiding this comment.
Ooooh good catch by the analyzer, I wonder how come that wasn't an error before nnbd!
There was a problem hiding this comment.
Casting anything to void used to be fine. I'm not sure why it changed with nnbd.
mdebbar
left a comment
There was a problem hiding this comment.
Thanks for migrating this! I only have a few suggestions and questions, but nothing blocking.
| }) : target = target ?? LinkTarget.defaultTarget, | ||
| Key? key, | ||
| required this.uri, | ||
| LinkTarget? target, |
There was a problem hiding this comment.
What do you think about changing it to this:
LinkTarget target = LinkTarget.defaultTarget,|
|
||
| Future<void> _followLink(BuildContext context) async { | ||
| if (!link.uri.hasScheme) { | ||
| if (link.uri == null || !link.uri!.hasScheme) { |
There was a problem hiding this comment.
This function is not supposed to be called when uri is null. I think it's more appropriate to force it to be non-null:
if (!link.uri!.hasScheme) {| return pushRouteNameToFramework(context, routeName); | ||
| await pushRouteNameToFramework(context, routeName); | ||
| return; |
There was a problem hiding this comment.
I'm wondering the same thing.
| if (debugForceRouter || _hasRouter(context)) { | ||
| SystemNavigator.routeInformationUpdated(location: routeName); | ||
| window.onPlatformMessage( | ||
| window.onPlatformMessage!( |
There was a problem hiding this comment.
To avoid repeatedly adding ! you can do:
final PlatformMessageCallback? onPlatformMessage = window.onPlatformMessage;
if (onPlatformMessage == null) {
return Future<ByteData>.value(null);
}
// After this point, the analyzer knows that the local variable `onPlatformMessage` is
// not null, so there's no need to add ! anymore.
Null safety migration for #3177