[shared_preferences] switch to ffi/jni pigeon - #12329
Conversation
|
@stuartmorgan-g this is a working example of switching to ffi/jni pigeon. the pr contains all of the pigeon changes, but you can just look at the shared preferences stuff |
stuartmorgan-g
left a comment
stuartmorgan-g
left a comment
There was a problem hiding this comment.
The iOS part generally looks good, but one question on the binary file. The Android part I have more questions about the structure of main package vs example package.
| @@ -52,3 +52,89 @@ dependencies { | |||
| androidTestImplementation("androidx.test.espresso:espresso-core:3.2.0") | |||
| api("androidx.test:core:1.4.0") | |||
| } | |||
| // Gradle stub for listing dependencies in JNIgen. If found in | |||
There was a problem hiding this comment.
I know this isn't in our direct control, but something we should raise for long-term jnigen discussions: I worry about the maintenance of this code. It's in a user-controlled file, not a generated file, which means that if it needs to change in the future there will need to be some potentially non-trivial migration.
(On the other hand, that's also true of the Flutter plugin system itself.)
stuartmorgan-g
left a comment
stuartmorgan-g
left a comment
There was a problem hiding this comment.
The Android structure is looking good now! One significant remaining issue for actually adopting this PR, but that's not related to the Pigeon generation itself, so in terms of validating the other PR I think we're in good shape here. I'll start on what it looks like should be the final review of the Pigeon PR shortly!
| sdk: flutter | ||
| integration_test: | ||
| sdk: flutter | ||
| jnigen: ^0.17.0 |
There was a problem hiding this comment.
Is this change still needed?
| @HostApi() | ||
| abstract class SharedPreferencesApi { | ||
| /// Removes property from shared preferences data set. | ||
| @TaskQueue(type: TaskQueueType.serialBackgroundThread) |
There was a problem hiding this comment.
All the implementations on the native side need to change to manually dispatch to a background thread (like they did before we had task queues, but presumably it's easier in Kotlin) so that we aren't regressing the old issue of doing file access on the main thread.
| @HostApi() | ||
| abstract class SharedPreferencesAsyncApi { | ||
| /// Adds property to shared preferences data set of type `bool`. | ||
| @TaskQueue(type: TaskQueueType.serialBackgroundThread) |
There was a problem hiding this comment.
Same for this class.
2 participants
for example