Skip to content
This repository has been archived by the owner on Feb 22, 2023. It is now read-only.

[image_picker_platform_interface] migrate to nnbd #3492

Merged
merged 21 commits into from
Feb 4, 2021
Merged

[image_picker_platform_interface] migrate to nnbd #3492

Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
edf0a36
deps
Jan 29, 2021
41212fc
upstream dep for now
Jan 29, 2021
fb198c9
draft
Feb 1, 2021
21dd55f
platform_interface migrate to nnbd
Feb 1, 2021
29d8a8a
line eof
Feb 1, 2021
906de01
changelog
Feb 1, 2021
8f37451
move image_picker to nnbd ignore list
Feb 1, 2021
1161e1f
revert changes in image_picker
Feb 1, 2021
a9dbc00
skip platform_interface
Feb 1, 2021
66bde58
review
Feb 1, 2021
aa828fb
remove deprecated methods
Feb 1, 2021
fcf55a1
revert test name for easier review
Feb 1, 2021
be2a278
review
Feb 2, 2021
b548792
add breaking changes in changelogs
Feb 2, 2021
3aa1d4b
Merge branch 'master' into nnbd_image_picker
Feb 3, 2021
cde75ee
Merge branch 'master' into nnbd_image_picker
Feb 3, 2021
afb8614
review
Feb 3, 2021
4c32c3d
review
Feb 3, 2021
fd4595c
review
Feb 3, 2021
beca962
exclude image picker on stable
Feb 3, 2021
9bb9101
exclude image picker in build all plugins because http dep conflicts
Feb 4, 2021
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
6 changes: 6 additions & 0 deletions packages/image_picker/image_picker_platform_interface/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
## 2.0.0-nullsafety

* Migrate to null safety.
* Breaking Changes:
* Removed the deprecated methods: `ImagePickerPlatform.retrieveLostDataAsDartIoFile`,`ImagePickerPlatform.pickImagePath` and `ImagePickerPlatform.pickVideoPath`.

## 1.1.6

* Fix test asset file location.
Expand Down
88 changes: 24 additions & 64 deletions ...r/image_picker_platform_interface/lib/src/method_channel/method_channel_image_picker.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import 'dart:io';

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

import 'package:image_picker_platform_interface/image_picker_platform_interface.dart';

Expand All @@ -20,14 +20,14 @@ class MethodChannelImagePicker extends ImagePickerPlatform {
MethodChannel get channel => _channel;

@override
Future<PickedFile> pickImage({
@required ImageSource source,
double maxWidth,
double maxHeight,
int imageQuality,
Future<PickedFile?> pickImage({
Copy link
Member

Choose a reason for hiding this comment

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

Do we want to use Optional, instead of nullable here? Future<Optional<PickedFile>>? I've seen Optional classes in package:quiver docs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmm I'm not sure what's the general guideline here. Maybe @blasten knows?

Copy link

Choose a reason for hiding this comment

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

Quiver appears to be a workaround for the lack of explicit nullability in the type system. With nnbd, I presume that package might get deprecated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So Future<PickedFile?> is ok? There is no preferred way?

Copy link

@blasten blasten Feb 2, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I guess my question would be can the future be completed with error? Why yes or why not?

For example, is null being returned when an error is more appropriate?

future.catchError(....)
// or
try {
 await future;
} on ... catch {
}

// vs

if (await future == null) {
 /// 
}

required ImageSource source,
double? maxWidth,
double? maxHeight,
int? imageQuality,
CameraDevice preferredCameraDevice = CameraDevice.rear,
}) async {
String path = await pickImagePath(
String? path = await _pickImagePath(
source: source,
maxWidth: maxWidth,
maxHeight: maxHeight,
Expand All @@ -37,15 +37,13 @@ class MethodChannelImagePicker extends ImagePickerPlatform {
return path != null ? PickedFile(path) : null;
}

@override
Future<String> pickImagePath({
@required ImageSource source,
double maxWidth,
double maxHeight,
int imageQuality,
Future<String?> _pickImagePath({
required ImageSource source,
double? maxWidth,
double? maxHeight,
int? imageQuality,
CameraDevice preferredCameraDevice = CameraDevice.rear,
}) {
assert(source != null);
if (imageQuality != null && (imageQuality < 0 || imageQuality > 100)) {
throw ArgumentError.value(
imageQuality, 'imageQuality', 'must be between 0 and 100');
Expand All @@ -72,26 +70,24 @@ class MethodChannelImagePicker extends ImagePickerPlatform {
}

@override
Future<PickedFile> pickVideo({
@required ImageSource source,
Future<PickedFile?> pickVideo({
required ImageSource source,
CameraDevice preferredCameraDevice = CameraDevice.rear,
Duration maxDuration,
Duration? maxDuration,
}) async {
String path = await pickVideoPath(
String? path = await _pickVideoPath(
source: source,
maxDuration: maxDuration,
preferredCameraDevice: preferredCameraDevice,
);
return path != null ? PickedFile(path) : null;
}

@override
Future<String> pickVideoPath({
@required ImageSource source,
Future<String?> _pickVideoPath({
required ImageSource source,
CameraDevice preferredCameraDevice = CameraDevice.rear,
Duration maxDuration,
Duration? maxDuration,
}) {
assert(source != null);
return _channel.invokeMethod<String>(
'pickVideo',
<String, dynamic>{
Expand All @@ -104,7 +100,7 @@ class MethodChannelImagePicker extends ImagePickerPlatform {

@override
Future<LostData> retrieveLostData() async {
final Map<String, dynamic> result =
final Map<String, dynamic>? result =
await _channel.invokeMapMethod<String, dynamic>('retrieve');

if (result == null) {
Expand All @@ -113,64 +109,28 @@ class MethodChannelImagePicker extends ImagePickerPlatform {

assert(result.containsKey('path') ^ result.containsKey('errorCode'));

final String type = result['type'];
final String? type = result['type'];
assert(type == kTypeImage || type == kTypeVideo);

RetrieveType retrieveType;
RetrieveType? retrieveType;
if (type == kTypeImage) {
retrieveType = RetrieveType.image;
} else if (type == kTypeVideo) {
retrieveType = RetrieveType.video;
}

PlatformException exception;
PlatformException? exception;
if (result.containsKey('errorCode')) {
exception = PlatformException(
code: result['errorCode'], message: result['errorMessage']);
}

final String path = result['path'];
final String? path = result['path'];

return LostData(
file: path != null ? PickedFile(path) : null,
exception: exception,
type: retrieveType,
);
}

@override
// ignore: deprecated_member_use_from_same_package
Future<LostDataResponse> retrieveLostDataAsDartIoFile() async {
final Map<String, dynamic> result =
await _channel.invokeMapMethod<String, dynamic>('retrieve');
if (result == null) {
// ignore: deprecated_member_use_from_same_package
return LostDataResponse.empty();
}
assert(result.containsKey('path') ^ result.containsKey('errorCode'));

final String type = result['type'];
assert(type == kTypeImage || type == kTypeVideo);

RetrieveType retrieveType;
if (type == kTypeImage) {
retrieveType = RetrieveType.image;
} else if (type == kTypeVideo) {
retrieveType = RetrieveType.video;
}

PlatformException exception;
if (result.containsKey('errorCode')) {
exception = PlatformException(
code: result['errorCode'], message: result['errorMessage']);
}

final String path = result['path'];

// ignore: deprecated_member_use_from_same_package
return LostDataResponse(
file: path == null ? null : File(path),
exception: exception,
type: retrieveType);
}
}
94 changes: 12 additions & 82 deletions ...ker/image_picker_platform_interface/lib/src/platform_interface/image_picker_platform.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,80 +39,6 @@ abstract class ImagePickerPlatform extends PlatformInterface {
_instance = instance;
}

/// Returns a [String] containing a path to the image that was picked.
///
/// The `source` argument controls where the image comes from. This can
/// be either [ImageSource.camera] or [ImageSource.gallery].
///
/// If specified, the image will be at most `maxWidth` wide and
/// `maxHeight` tall. Otherwise the image will be returned at it's
/// original width and height.
///
/// The `imageQuality` argument modifies the quality of the image, ranging from 0-100
/// where 100 is the original/max quality. If `imageQuality` is null, the image with
/// the original quality will be returned. Compression is only supported for certain
/// image types such as JPEG and on Android PNG and WebP, too. If compression is not supported for the image that is picked,
/// a warning message will be logged.
///
/// Use `preferredCameraDevice` to specify the camera to use when the `source` is [ImageSource.camera].
/// The `preferredCameraDevice` is ignored when `source` is [ImageSource.gallery]. It is also ignored if the chosen camera is not supported on the device.
/// Defaults to [CameraDevice.rear].
///
/// In Android, the MainActivity can be destroyed for various reasons. If that happens, the result will be lost
/// in this call. You can then call [retrieveLostDataAsDartIoFile] when your app relaunches to retrieve the lost data.
@Deprecated('Use pickImage instead.')
Future<String> pickImagePath({
@required ImageSource source,
double maxWidth,
double maxHeight,
int imageQuality,
CameraDevice preferredCameraDevice = CameraDevice.rear,
}) {
throw UnimplementedError('legacyPickImage() has not been implemented.');
}

/// Returns a [String] containing a path to the video that was picked.
///
/// The [source] argument controls where the video comes from. This can
/// be either [ImageSource.camera] or [ImageSource.gallery].
///
/// The [maxDuration] argument specifies the maximum duration of the captured video. If no [maxDuration] is specified,
/// the maximum duration will be infinite.
///
/// Use `preferredCameraDevice` to specify the camera to use when the `source` is [ImageSource.camera].
/// The `preferredCameraDevice` is ignored when `source` is [ImageSource.gallery]. It is also ignored if the chosen camera is not supported on the device.
/// Defaults to [CameraDevice.rear].
///
/// In Android, the MainActivity can be destroyed for various fo reasons. If that happens, the result will be lost
/// in this call. You can then call [retrieveLostDataAsDartIoFile] when your app relaunches to retrieve the lost data.
@Deprecated('Use pickVideo instead.')
Future<String> pickVideoPath({
@required ImageSource source,
CameraDevice preferredCameraDevice = CameraDevice.rear,
Duration maxDuration,
}) {
throw UnimplementedError('pickVideoPath() has not been implemented.');
}

/// Retrieve the lost image file when [pickImagePath] or [pickVideoPath] failed because the MainActivity is destroyed. (Android only)
///
/// Image or video can be lost if the MainActivity is destroyed. And there is no guarantee that the MainActivity is always alive.
/// Call this method to retrieve the lost data and process the data according to your APP's business logic.
///
/// Returns a [LostDataResponse] if successfully retrieved the lost data. The [LostDataResponse] can represent either a
/// successful image/video selection, or a failure.
///
/// Calling this on a non-Android platform will throw [UnimplementedError] exception.
///
/// See also:
/// * [LostDataResponse], for what's included in the response.
/// * [Android Activity Lifecycle](https://developer.android.com/reference/android/app/Activity.html), for more information on MainActivity destruction.
@Deprecated('Use retrieveLostData instead.')
Future<LostDataResponse> retrieveLostDataAsDartIoFile() {
throw UnimplementedError(
'retrieveLostDataAsDartIoFile() has not been implemented.');
}

// Next version of the API.

/// Returns a [PickedFile] with the image that was picked.
Expand Down Expand Up @@ -141,11 +67,13 @@ abstract class ImagePickerPlatform extends PlatformInterface {
///
/// In Android, the MainActivity can be destroyed for various reasons. If that happens, the result will be lost
/// in this call. You can then call [retrieveLostData] when your app relaunches to retrieve the lost data.
Future<PickedFile> pickImage({
@required ImageSource source,
double maxWidth,
double maxHeight,
int imageQuality,
///
/// If no images were picked, the return value is null.
Future<PickedFile?> pickImage({
Copy link

Choose a reason for hiding this comment

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

Same comment about Future<PickedFile?>. We would need to either:

  1. (Preferred) Remove the nullability.
  2. Explain when the Future completes as null, and what to do in those cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done. Added docs to explain the null value.

required ImageSource source,
double? maxWidth,
double? maxHeight,
int? imageQuality,
CameraDevice preferredCameraDevice = CameraDevice.rear,
}) {
throw UnimplementedError('pickImage() has not been implemented.');
Expand All @@ -165,10 +93,12 @@ abstract class ImagePickerPlatform extends PlatformInterface {
///
/// In Android, the MainActivity can be destroyed for various fo reasons. If that happens, the result will be lost
/// in this call. You can then call [retrieveLostData] when your app relaunches to retrieve the lost data.
Future<PickedFile> pickVideo({
@required ImageSource source,
///
/// If no images were picked, the return value is null.
Future<PickedFile?> pickVideo({
cyanglaz marked this conversation as resolved.
Show resolved Hide resolved
required ImageSource source,
CameraDevice preferredCameraDevice = CameraDevice.rear,
Duration maxDuration,
Duration? maxDuration,
}) {
throw UnimplementedError('pickVideo() has not been implemented.');
}
Expand Down
6 changes: 3 additions & 3 deletions packages/image_picker/image_picker_platform_interface/lib/src/types/lost_data_response.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ class LostDataResponse {
/// The file that was lost in a previous [pickImage] or [pickVideo] call due to MainActivity being destroyed.
///
/// Can be null if [exception] exists.
final File file;
final File? file;
ditman marked this conversation as resolved.
Show resolved Hide resolved

/// The exception of the last [pickImage] or [pickVideo].
///
Expand All @@ -43,10 +43,10 @@ class LostDataResponse {
/// You should handle this exception as if the [pickImage] or [pickVideo] got an exception when the MainActivity was not destroyed.
///
/// Note that it is not the exception that caused the destruction of the MainActivity.
final PlatformException exception;
final PlatformException? exception;

/// Can either be [RetrieveType.image] or [RetrieveType.video];
final RetrieveType type;
final RetrieveType? type;

bool _empty = false;
}
2 changes: 1 addition & 1 deletion packages/image_picker/image_picker_platform_interface/lib/src/types/picked_file/base.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ abstract class PickedFileBase {
/// If `end` is present, only up to byte-index `end` will be read. Otherwise, until end of file.
///
/// In order to make sure that system resources are freed, the stream must be read to completion or the subscription on the stream must be cancelled.
Stream<Uint8List> openRead([int start, int end]) {
Stream<Uint8List> openRead([int? start, int? end]) {
Copy link

Choose a reason for hiding this comment

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

Should this be named parameters instead? openRead(end: 10) is better than openRead(null, 10)

Copy link
Member

Choose a reason for hiding this comment

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

These follow the File API. Anyway, this code will eventually be removed (it's available, and reusable in the cross_file package now)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's unrelated to NNBD and I'd prefer not to break the API. Also it's going to be removed soon (probably the very next patch), let's not introduce 2 consecutive breaking changes for people to migrate.

Copy link

Choose a reason for hiding this comment

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

sgtm

throw UnimplementedError('openRead() has not been implemented.');
}
}
8 changes: 4 additions & 4 deletions packages/image_picker/image_picker_platform_interface/lib/src/types/picked_file/html.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,19 +10,19 @@ import './base.dart';
/// It wraps the bytes of a selected file.
class PickedFile extends PickedFileBase {
final String path;
final Uint8List _initBytes;
final Uint8List? _initBytes;

/// Construct a PickedFile object from its ObjectUrl.
///
/// Optionally, this can be initialized with `bytes`
/// so no http requests are performed to retrieve files later.
PickedFile(this.path, {Uint8List bytes})
PickedFile(this.path, {Uint8List? bytes})
: _initBytes = bytes,
super(path);

Future<Uint8List> get _bytes async {
if (_initBytes != null) {
return Future.value(UnmodifiableUint8ListView(_initBytes));
return Future.value(UnmodifiableUint8ListView(_initBytes!));
}
return http.readBytes(Uri.parse(path));
}
Expand All @@ -38,7 +38,7 @@ class PickedFile extends PickedFileBase {
}

@override
Stream<Uint8List> openRead([int start, int end]) async* {
Stream<Uint8List> openRead([int? start, int? end]) async* {
final bytes = await _bytes;
yield bytes.sublist(start ?? 0, end ?? bytes.length);
}
Expand Down
2 changes: 1 addition & 1 deletion packages/image_picker/image_picker_platform_interface/lib/src/types/picked_file/io.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class PickedFile extends PickedFileBase {
}

@override
Stream<Uint8List> openRead([int start, int end]) {
Stream<Uint8List> openRead([int? start, int? end]) {
return _file
.openRead(start ?? 0, end)
.map((chunk) => Uint8List.fromList(chunk));
Expand Down
8 changes: 5 additions & 3 deletions ...ges/image_picker/image_picker_platform_interface/lib/src/types/picked_file/lost_data.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ class LostData {
/// The file that was lost in a previous [pickImage] or [pickVideo] call due to MainActivity being destroyed.
///
/// Can be null if [exception] exists.
final PickedFile file;
final PickedFile? file;

/// The exception of the last [pickImage] or [pickVideo].
///
Expand All @@ -40,10 +40,12 @@ class LostData {
/// You should handle this exception as if the [pickImage] or [pickVideo] got an exception when the MainActivity was not destroyed.
///
/// Note that it is not the exception that caused the destruction of the MainActivity.
final PlatformException exception;
final PlatformException? exception;

/// Can either be [RetrieveType.image] or [RetrieveType.video];
final RetrieveType type;
///
/// If the lost data is empty, this will be null.
final RetrieveType? type;
Copy link

Choose a reason for hiding this comment

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

the docs doesn't say null though

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done


bool _empty = false;
}
Loading