line / flutter_line_sdk

A Flutter plugin that lets developers access LINE's native SDKs in Flutter apps with Dart.
https://developers.line.biz/
Apache License 2.0
213 stars 43 forks source link

Migrate to Android v2 plugin spec #34

Closed onevcat closed 4 years ago

onevcat commented 4 years ago

Currently, the Android part is triggering a flutter warning saying "Using Registrar based plugin is already deprecated".

I checked the reason and followed the guide here and here to upgrade both the Android plugin and the sample app. Now, the plugin should be also usable in an Add-to-App flutter integration.

@plateaukao Although it seems to be working, I am not sure about several points, so please take a look at it.

  1. I didn't find a replacing class of FlutterApplication, so I just followed the guide above and removed the whole MyApplication.kt thing. It was for #29 a while ago. Not sure if there is an alternative or not.
  2. Due to 1, and as we discussed before, I upgraded the minSdkVersion to 21. Not sure if it is fine, and not sure whether we also need to update the targetSdkVersion or not.
  3. I have some concern about the lineApiClient = createLineApiClient(activity, channelId) code in LineSdkWrapper.kt. Can it still work fine if an activity is detached and a new one is attacked? If not, how can we make the plugin also support using in activities (such as flutter and our plugin is only used as a part in a native Android app, but not the whole app). Should we require user to call setupSDK again to make sure the lineApiClient gets the latest context object?
plateaukao commented 4 years ago
1. I didn't find a replacing class of `FlutterApplication`, so I just followed the guide above and removed the whole `MyApplication.kt` thing. It was for #29 a while ago. Not sure if there is an alternative or not.

It should be fine, since the fix in MyApplication.kt is for fixing issues with SDK level 19. Now it's upgraded to level 21.

2. Due to 1, and as we discussed before, I upgraded the `minSdkVersion` to 21. Not sure if it is fine, and not sure whether we also need to update the `targetSdkVersion` or not.

Original sdk level 17 is to align line-sdk android library. But since LINE Client support is already upgraded to level 21, I think it's fine for flutter line sdk to upgrade minSdkVerion to 21.

3. I have some concern about the `lineApiClient = createLineApiClient(activity, channelId)` code in `LineSdkWrapper.kt`. Can it still work fine if an activity is detached and a new one is attacked? If not, how can we make the plugin also support using in activities (such as flutter and our plugin is only used as a part in a native Android app, but not the whole app). Should we require user to call `setupSDK` again to make sure the `lineApiClient` gets the latest context object?

For this, I am not sure either. For creating lineApiClient, we can still use activity.applicationcontext. As for more details, I have to study first about add-to-App implementation, and reply you about item 3.

onevcat commented 4 years ago

There is actually an applicationcontext in the FlutterPlugin.FlutterPluginBinding which Flutter passes in when onAttachedToEngine called. Is it shared across the whole app life cycle? If so, I guess it would be totally safe to use that instead.

(And it also matters how the context is used in lineApiClient.)

Feel free to modify it if you find anything! :P