authpass / biometric_storage

Flutter plugin to store data behind biometric authentication (ie. fingerprint)
https://pub.dev/packages/biometric_storage
MIT License
184 stars 107 forks source link

Update DartPluginFile usage for windows #47

Closed brianblanchard-wf closed 2 years ago

brianblanchard-wf commented 2 years ago

Description

import 'package:biometric_storage_example/main.dart' as entrypoint; import 'dart:io'; // flutter_ignore: dart_io_import. import 'package:biometric_storage/biometric_storage.dart';

@pragma('vm:entry-point') class _PluginRegistrant {

@pragma('vm:entry-point') static void register() { if (Platform.isAndroid) { } else if (Platform.isIOS) { } else if (Platform.isLinux) { } else if (Platform.isMacOS) { } else if (Platform.isWindows) { try { Win32BiometricStoragePlugin.register(); } catch (err) { print( 'biometric_storage threw an error: $err. ' 'The app may not function as expected until you remove this plugin from pubspec.yaml' ); rethrow; }

}

}

}

typedef _UnaryFunction = dynamic Function(List args); typedef _NullaryFunction = dynamic Function();

void main(List args) { if (entrypoint.main is _UnaryFunction) { (entrypoint.main as _UnaryFunction)(args); } else { (entrypoint.main as _NullaryFunction)(); } }

which will cause the following error when building the application

Error: Undefined name 'Win32BiometricStoragePlugin'. .dart_tool/flutter_build/generated_main.dart:38 Win32BiometricStoragePlugin.registerWith();


- The flutter tooling is expecting
  - The `Win32BiometricStoragePlugin` class to be importable from `package:biometric_storage/biometric_storage.dart'` 
  - The `Win32BiometricStoragePlugin` to define a static function `registerWith()` 

## Changes
- Removed the noop win32 file/implementation
- Changed the default `BiometricStorage.instance` to be `MethodChannelBiometricStorage`
- Exported the `Win32BiometricStoragePlugin` from `lib/biometric_storage.dart`
- Defined `Win32BiometricStoragePlugin.registerWith` static method that registers an instance of `Win32BiometricStoragePlugin` as the instance provided by `BiometricStorage.instance`
- Updated some various iOS project files based on Flutter tooling changes
- Update the example to include `biometric_storage` as a normal dependency and not a dev dependency
- Bumped the version to `4.0.0` because this is most likely a breaking change
- Bumped the min Flutter SDK to `2.8.0` because I believe prior to this stable version the issue described above does not reproduce.

## Important Notes
- If you try to reproduce this issue with the current release's example app and Flutter 2.8.0 it will NOT reproduce. The dependency on `biometric_storage` is a dev dependency instead of a normal dependency. It seems the the `generated_main.dart` file is not generated if a `dev_dependency` provides a Dart only plugin implementation. I have updated the example to provide `biometric_storage` as a normal `dependency` in this PR.

## Testing
- I have verified this fixes the build issue on iOS and Android but cannot verify that the Linux and Windows implementations of this plugin are still working as expected because I do not have the proper hardware.
miDeb commented 2 years ago

This does not fix it for me compiling on linux for either android or linux itself:

ERROR: .dart_tool/flutter_build/generated_main.dart:61:9: Error: Undefined name 'Win32BiometricStoragePlugin'.
ERROR:         Win32BiometricStoragePlugin.registerWith();
ERROR:         ^^^^^^^^^^^^^^^^^^^^^^^^^^^

I think this is because pubspec.yaml still references Win32BiometricStoragePlugin. If I remove the reference, the error does not occur.

brianblanchard-wf commented 2 years ago

@miDeb my bad, I forgot to export the Win32BiometricStoragePlugin. Can you try again with the latest commit?

brianblanchard-wf commented 2 years ago

@hpoul as yes, my bad I thought I did that.

miDeb commented 2 years ago

@brianblanchard-wf I can confirm that it works for me now.

brianblanchard-wf commented 2 years ago

@hpoul regarding federated plugins, it might be worth a discussion in breaking out the different platform implementations into different packages similar to how some of the official flutter plugins (like shared_preferences) do it. So you would have something like biometric_storage, biometric_storage_platform_interface, biometric_storage_windows, etc that can all live in this repo as sub packages.

Let me know if that is something that is interesting to you and I can take a stab at it in my free time.

For sure out of scope for this PR, I was just looking to unblock consumers on the latest stable version of Flutter 😄

brianblanchard-wf commented 2 years ago

@hpoul I did a quick test and it seems if I just change the windows plugin entry in the pubspec.yaml to this:

windows:
   pluginClass: none

then no other code changes are required. Do you want to go the route of the current implementation with dartPluginClass or just update the pubspec.yaml to the above?

hpoul commented 2 years ago

@brianblanchard-wf are you saying that the fileName property is working? 🤔️ To me this would look nicer, but since it's not documented, it might be better to go the route of exporting the class from the main library file 😅️ thanks, I'll merge it this way and see if it works for everyone 🤞