Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 2.0.0-nullsafety

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@amirh do you have any comment on this version bump?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that for the null safety migration as we expect all platform implementations to get the same bump in close proximity it is reasonable to do a major bump on the platform interface.


* Migrate to null safety.

## 1.0.8

* Added webOnlyWindowName parameter
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
import 'dart:async';

import 'package:flutter/services.dart';
import 'package:meta/meta.dart' show required;

import 'url_launcher_platform_interface.dart';

Expand All @@ -18,7 +17,7 @@ class MethodChannelUrlLauncher extends UrlLauncherPlatform {
return _channel.invokeMethod<bool>(
'canLaunch',
<String, Object>{'url': url},
);
).then((value) => value ?? false);
}

@override
Expand All @@ -29,13 +28,13 @@ class MethodChannelUrlLauncher extends UrlLauncherPlatform {
@override
Future<bool> launch(
String url, {
@required bool useSafariVC,
@required bool useWebView,
@required bool enableJavaScript,
@required bool enableDomStorage,
@required bool universalLinksOnly,
@required Map<String, String> headers,
String webOnlyWindowName,
required bool useSafariVC,
required bool useWebView,
required bool enableJavaScript,
required bool enableDomStorage,
required bool universalLinksOnly,
required Map<String, String> headers,
String? webOnlyWindowName,
}) {
return _channel.invokeMethod<bool>(
'launch',
Expand All @@ -48,6 +47,6 @@ class MethodChannelUrlLauncher extends UrlLauncherPlatform {
'universalLinksOnly': universalLinksOnly,
'headers': headers,
},
);
).then((value) => value ?? false);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

import 'dart:async';

import 'package:meta/meta.dart' show required;
import 'package:plugin_platform_interface/plugin_platform_interface.dart';

import 'method_channel_url_launcher.dart';
Expand Down Expand Up @@ -49,13 +48,13 @@ abstract class UrlLauncherPlatform extends PlatformInterface {
/// in `package:url_launcher/url_launcher.dart`.
Future<bool> launch(
String url, {
@required bool useSafariVC,
@required bool useWebView,
@required bool enableJavaScript,
@required bool enableDomStorage,
@required bool universalLinksOnly,
@required Map<String, String> headers,
String webOnlyWindowName,
required bool useSafariVC,
required bool useWebView,
required bool enableJavaScript,
required bool enableDomStorage,
required bool universalLinksOnly,
required Map<String, String> headers,
String? webOnlyWindowName,
}) {
throw UnimplementedError('launch() has not been implemented.');
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,21 @@ description: A common platform interface for the url_launcher plugin.
homepage: https://github.com/flutter/plugins/tree/master/packages/url_launcher/url_launcher_platform_interface
# NOTE: We strongly prefer non-breaking changes, even at the expense of a
# less-clean API. See https://flutter.dev/go/platform-interface-breaking-changes
version: 1.0.8
version: 2.0.0-nullsafety

dependencies:
flutter:
sdk: flutter
meta: ^1.0.5
plugin_platform_interface: ^1.0.1
# TODO (mvanbeusekom): use pub.dev once 1.10.0-nullsafety released.
plugin_platform_interface:
path: ../../plugin_platform_interface

dev_dependencies:
flutter_test:
sdk: flutter
mockito: ^4.1.1
pedantic: ^1.8.0
pedantic: ^1.10.0-nullsafety.1

environment:
sdk: ">=2.1.0 <3.0.0"
sdk: '>=2.10.0-56.0.dev <3.0.0'
flutter: ">=1.9.1+hotfix.4 <2.0.0"
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

// TODO(mvanbeusekom): Remove once Mockito is migrated to null safety.
// @dart = 2.9
import 'package:mockito/mockito.dart';
import 'package:flutter/services.dart';
import 'package:flutter_test/flutter_test.dart';
Expand Down Expand Up @@ -41,6 +43,10 @@ void main() {
final List<MethodCall> log = <MethodCall>[];
channel.setMockMethodCallHandler((MethodCall methodCall) async {
log.add(methodCall);

// Return null explicitly instead of relying on the implicit null
// returned by the method channel if no return statement is specified.
return null;
});
Comment on lines +46 to 50

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: what about moving this code to the specific test where it's being used, or the setUp function in the worst case? This way the test doesn't leak state, and it doesn't need log.clear(); in the tearDown.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is that setting the mock method channel (using setMockMethodCallHandler) has a global impact (this is how it is handled inside the MethodChannel class). So if I am to update in the specific test it will affect all tests that are run after (or during, since tests can run in parallel) the specific test. Or am I missing something here?

For now I removed the above logic, as it turns out the method channel will automatically return null if no return statement is specified, which is exactly the behavior needed by the tests. I did however added a return null; statement to make the logic explicit instead of implicitly relying on the behavior of the implementation of the MethodChannel class.


final MethodChannelUrlLauncher launcher = MethodChannelUrlLauncher();
Expand All @@ -61,6 +67,12 @@ void main() {
);
});

test('canLaunch should return false if platform returns null', () async {
final canLaunch = await launcher.canLaunch('http://example.com/');

expect(canLaunch, false);
});

test('launch', () async {
await launcher.launch(
'http://example.com/',
Expand Down Expand Up @@ -269,6 +281,20 @@ void main() {
);
});

test('launch should return false if platform returns null', () async {
final launched = await launcher.launch(
'http://example.com/',
useSafariVC: true,
useWebView: false,
enableJavaScript: false,
enableDomStorage: false,
universalLinksOnly: false,
headers: const <String, String>{},
);

expect(launched, false);
});

test('closeWebView default behavior', () async {
await launcher.closeWebView();
expect(
Expand Down
1 change: 1 addition & 0 deletions script/incremental_build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ CUSTOM_ANALYSIS_PLUGINS=(
"plugin_platform_interface"
"video_player/video_player_web"
"google_maps_flutter/google_maps_flutter_web"
"url_launcher/url_launcher_platform_interface"
)
# Comma-separated string of the list above
readonly CUSTOM_FLAG=$(IFS=, ; echo "${CUSTOM_ANALYSIS_PLUGINS[*]}")
Expand Down