-
Notifications
You must be signed in to change notification settings - Fork 9.8k
[connectivity_for_web] Migration to null-safety. #3652
[connectivity_for_web] Migration to null-safety. #3652
Conversation
… instead of an internal subscription to another stream (which was hard to cleanup)
…ream, to prevent multiple attachments on hot reload.
@@ -18,7 +18,7 @@ class NetworkInformationApiConnectivityPlugin extends ConnectivityPlugin { | |||
|
|||
/// The constructor of the plugin. | |||
NetworkInformationApiConnectivityPlugin() | |||
: this.withConnection(html.window.navigator.connection); | |||
: this.withConnection(html.window.navigator.connection!); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is ensured by the isSupported
method that is used before instantiating this plugin class.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems worth documenting that requirement in the constructor comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with nits
? ConnectivityResult.wifi | ||
: ConnectivityResult.none; | ||
} | ||
|
||
StreamController<ConnectivityResult> _connectivityResult; | ||
late StreamController<ConnectivityResult> _connectivityResult; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Huh, I thought you couldn't null-check a late
value (and that lazy initialization like the below needed ?
as a result).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, let me give this a test!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was indeed crashy. Fixed. This file needs a test (mock window + navigator), I'll add one after this lands.
@@ -18,7 +18,7 @@ class NetworkInformationApiConnectivityPlugin extends ConnectivityPlugin { | |||
|
|||
/// The constructor of the plugin. | |||
NetworkInformationApiConnectivityPlugin() | |||
: this.withConnection(html.window.navigator.connection); | |||
: this.withConnection(html.window.navigator.connection!); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems worth documenting that requirement in the constructor comment.
The requirement of the 'connection' API is documented in the README: https://github.com/flutter/plugins/tree/master/packages/connectivity/connectivity_for_web#limitations-on-the-web-platform |
* master: Adopt Xcode 12 for podspec lints (flutter#3653) Run static analyzer during xctest (flutter#3667) [google_maps_flutter_web] update min flutter sdk version to 1.20.0 (flutter#3662) [image_picker] Run CocoaPods iOS tests in RunnerUITests target (flutter#3663) [webview_flutter] Run CocoaPods iOS tests in RunnerUITests target (flutter#3664) [device_info] Enable NNBD for unit test (flutter#3658) remove unused plugin (flutter#3661) [android_intent] move unit test to nullsafety (flutter#3659) [url_launcher] Migrate unit tests to NNBD (flutter#3657) [share] Migrate unit tests to null-safety. (flutter#3660) [connectivity_for_web] Migration to null-safety. (flutter#3652) [camera] Stable release for null safety. (flutter#3641) [in_app_purchase] fix plugin version (flutter#3654) Move plugin tool tests over (flutter#3606) [in_app_purchase] migrate playing billing library to v3 (flutter#3636) Update plugin_platform_interface min version (flutter#3650) # Conflicts: # packages/webview_flutter/CHANGELOG.md # packages/webview_flutter/example/ios/Runner.xcodeproj/project.pbxproj
This change migrates the package to null safety, and moves its (very obsolete) tests to the new integration_test style.
The tests used to use mockito, but those mocks have been replaced by Fake implementations.
Tests
Pre-launch Checklist
[shared_preferences]
///
).If you need help, consider asking for advice on the #hackers-new channel on Discord.