-
-
Notifications
You must be signed in to change notification settings - Fork 937
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(device_info_plus)!: refactor of device_info_plus platform implementation #1293
Conversation
AndroidDeviceInfo._({ | ||
required Map<String, dynamic> data, |
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 private constructor and the fromMap
method could be refactored further, probably, but I didn't do it yet. At least I marked this constructor as private now, which shouldn't be used outside the plugin.
/// Serializes [AndroidDeviceInfo] to map. | ||
@Deprecated('[toMap] method will be discontinued') | ||
@override | ||
Map<String, dynamic> toMap() { | ||
return { | ||
'id': id, |
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.
The original toMap
disappears and now instead the data
returned by the platform channel is returned. Rather than re-creating the map out of the class data, we return the same data the Android code created, which should be 100% serializable.
@override | ||
// ignore: deprecated_member_use_from_same_package | ||
Map<String, dynamic> get data => toMap(); | ||
|
||
@Deprecated('Use [data] getter instead') |
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.
With dart plugins, they can implement the BaseDeviceInfo
directly but still need to provide the data
. At the moment, there's no warranty that the data
is serializable, but that's out of scope for this PR.
Sorry, switching back to draft, need to do some fixes |
// allow for extension of the plugin | ||
return _platform.deviceInfo(); |
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.
Any new platform implementing deviceInfo()
can now return a generic BaseDeviceInfo
with some data
Future<AndroidDeviceInfo> androidInfo() { | ||
throw UnimplementedError('androidInfo() has not been implemented.'); | ||
} | ||
|
||
// Gets the iOS device information. | ||
// ignore: public_member_api_docs | ||
Future<IosDeviceInfo> iosInfo() { | ||
throw UnimplementedError('iosInfo() has not been implemented.'); | ||
} | ||
|
||
// Gets the Linux device information. | ||
// ignore: public_member_api_docs | ||
Future<LinuxDeviceInfo> linuxInfo() { | ||
throw UnimplementedError('linuxInfo() has not been implemented.'); | ||
} | ||
|
||
// Gets the web browser information. | ||
// ignore: public_member_api_docs | ||
Future<WebBrowserInfo> webBrowserInfo() { | ||
throw UnimplementedError('webBrowserInfo() has not been implemented.'); | ||
} | ||
|
||
// Gets the Macos device information. | ||
// ignore: public_member_api_docs | ||
Future<MacOsDeviceInfo> macosInfo() { | ||
throw UnimplementedError('macosInfo() has not been implemented.'); | ||
} | ||
|
||
// Gets the Windows device information | ||
// ignore: public_member_api_docs | ||
Future<WindowsDeviceInfo>? windowsInfo() { | ||
throw UnimplementedError('windowsInfo() has not been implemented.'); |
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.
All platform specific methods are now replaced by a single deviceInfo()
making the platform interface generic
Future<BaseDeviceInfo> deviceInfo() async { | ||
return BaseDeviceInfo( | ||
(await channel.invokeMethod('getDeviceInfo')).cast<String, dynamic>()); |
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.
Platform channels should respond to getDeviceInfo
now
The suggested way seems Ok to me. Like that the platform interface is generic 👍🏻 |
Thanks for the review! Let's go! |
Description
This PR contains a refactor of the platform implementation of
device_info_plus
.The main idea is to make the
device_info_plus_platform_interace
as generic as possible.To do that, the
DeviceInfoPlatform
now contains a single methoddeviceInfo()
.Any platform implementation can implement it, either with a Dart plugin (like Linux, Windows and Web) or through a platform channel like Android, iOS and macOS implementing the
getDeviceInfo
platform channel method.The trick is that now
BaseDeviceInfo
always contains some data, either because it is returned by the platform channel from native code (like iOS, Android and macOS, or is overridden by the Dart implementation (like Linux, Windows and Web).This change should be transparent to users (the example code wasn't even changed), however, this PR has been marked as breaking change due to the changes in the platform channels.
Finally, the plugin exposes a
data
method as replacement for thetoMap
in theBaseDeviceInfo
. This data may not be serializable for dart plugins, but at least it will be there.With this change, devs can now also add new properties to any of the
***DeviceInfo
implementations without having to touch the platform interface package at all.Related Issues
Checklist
CHANGELOG.md
nor thepubspec.yaml
files.flutter analyze
) does not report any problems on my PR.Breaking Change
Does your PR require plugin users to manually update their apps to accommodate your change?
!
in the title as explained in Conventional Commits).