Add FFI and JNI support to Swift and Kotlin#11352
Add FFI and JNI support to Swift and Kotlin#11352tarrinneal wants to merge 107 commits intoflutter:mainfrom
Conversation
stuartmorgan-g
left a comment
There was a problem hiding this comment.
Some very early review notes; since it's going to be a long review I thought I should post things incrementally. So far I've mostly just looked at the smaller bits, not the full generators yet (and not the tests).
Please also add a real PR description, with issue references and a high level overview of the PR (what it does, what generators it covers, what's in and out of scope here, etc.)
stuartmorgan-g
left a comment
There was a problem hiding this comment.
Still working, but here's another incremental comment drop.
There was a problem hiding this comment.
We should consider changing to two example apps, one that uses method-channel-based Pigeon, and one that use the FFI-based Pigeon. I would expect the common use case to just be one or the other, so putting both in the same app could be confusing.
There was a problem hiding this comment.
Do we have the infrastructure to handle two example apps?
There was a problem hiding this comment.
Yes, if you have example/thing1/ and example/thing2/ as example apps, our tooling will understand that and do things with both of them.
| // #enddocregion config | ||
| @HostApi() | ||
| abstract class NativeInteropExampleApi { | ||
| void doSomething(); |
There was a problem hiding this comment.
I'm not actually seeing any generated code corresponding to this. Is this file unused except for excerpting? If so that's pretty confusing for anyone looking at the example to see how to use this.
There was a problem hiding this comment.
Does this need to be checked in? If so we should use some kind of naming convention for it to indicate that it's generated if possible.
| expect(listEquals(echoObject, list), true); | ||
| }); | ||
|
|
||
| // // Currently need set up |
| override fun noop() { | ||
| api?.let { | ||
| try { | ||
| return api!!.noop() |
There was a problem hiding this comment.
My Kotlin-fu is not strong, but surely there's a way to do a guard that doesn't require force unwrapping.
| return ''; | ||
| } | ||
| typeArgumentString += '>'; | ||
| return typeArgumentString; |
There was a problem hiding this comment.
You can do final typeArgumentString; at the top, make the internals assignments instead of +=, and then return '<$typeArgumentString>';. That avoids the reassignable variable and the string concatenation.
| indent.writeln('}'); | ||
| } else { | ||
| indent.format(''' | ||
| var sdkPath = '/Applications/Xcode.app/Contents/Developer/Platforms/iPhoneOS.platform/Developer/SDKs/iPhoneOS.sdk'; |
There was a problem hiding this comment.
We can punt on it for now, but at some point we'll need to figure out how to allow configuring iOS vs macOS vs both as SDK options.
| final indent = Indent(); | ||
| indent.writeln('// ignore_for_file: prefer_const_constructors'); | ||
| indent.writeln("import 'package:jnigen/jnigen.dart';"); | ||
| indent.writeln("import 'package:logging/logging.dart';"); |
There was a problem hiding this comment.
When we write these configs, we should also parse the pubspec and check that these dependencies exist, and if not either add them automatically, or tell the user they should add them. (Same for ffigen.)
There was a problem hiding this comment.
Same for the runtime dependencies (jni, objective_c)
stuartmorgan-g
left a comment
There was a problem hiding this comment.
I tried playing with this in local_auth_android, and ran into some issues pretty quickly, although I may have been holding it wrong. I would definitely recommend trying to do an initial conversion of a plugin (that one already uses kotlin generation) to see if things work.
| if (generatorOptions.useJni) { | ||
| indent.writeln('import androidx.annotation.Keep'); | ||
| } | ||
| indent.writeln('import io.flutter.plugin.common.BasicMessageChannel'); |
There was a problem hiding this comment.
We should make all of these conditional; they are unused when using JNI.
| if (generatorOptions.package != null && !generatorOptions.useJni) { | ||
| indent.writeln('package ${generatorOptions.package}'); | ||
| } |
There was a problem hiding this comment.
Were you including the namepace in the class list in the jnigen config file?
We definitely want to use the package namespace if we are going to avoid a lot of disruption to clients.
| @ConfigurePigeon( | ||
| PigeonOptions( | ||
| dartOptions: DartOptions(), | ||
| kotlinOptions: KotlinOptions(useJni: true), |
There was a problem hiding this comment.
The docs say appDirectory is required with JNI, but this isn't using it. And using it didn't work for me, unless I was doing something wrong. What's the right way to configure at the Pigeon level for a standard plugin-with-example?
There was a problem hiding this comment.
good point. appDirectory in the tests is encoded with a default value so isn't required. That's clearly confusing though so I'll add it here. What was the behavior you were seeing? This didn't do anything at all? were the jnigen bindings not showing up? not in the right folder?
There was a problem hiding this comment.
IIRC it was trying to find the example in ./ so jnigen failed? I'll have to try again from scratch since I foolishly didn't capture the details.
There was a problem hiding this comment.
ok, don't worry about it until I push up changes, I'm looking into it already.
This PR introduces optional Native Interop to Pigeon, enabling direct communication between Dart and native code without the overhead of traditional MethodChannel serialization. It leverages FFI (Foreign Function Interface) for Swift (iOS/macOS) and JNI (Java Native Interface) for Kotlin (Android).
This represents a significant architectural shift, moving from message-based passing to direct memory sharing and function calls. It also updates the concurrency model for asynchronous methods, moving from completion handlers/callbacks to modern language features: async/await in Swift and Coroutines in Kotlin.
Generators Covered
Swift Generator: Updated to support FFI bindings and async/await for asynchronous methods.
Kotlin Generator: Updated to support JNI bindings and Kotlin Coroutines for asynchronous methods.
Dart Generator: Updated to handle the generated interop bindings on the Dart side.
What's In Scope
Tests: Added ni_tests.dart and associated generated files and integration tests to verify the feature.
What's Out of Scope
work toward flutter/flutter#182230
design doc flutter/flutter#181430