Skip to content
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

Allow directly sharing iOS and macOS plugin native code #115085

Closed
stuartmorgan opened this issue Nov 10, 2022 · 9 comments · Fixed by #115337
Closed

Allow directly sharing iOS and macOS plugin native code #115085

stuartmorgan opened this issue Nov 10, 2022 · 9 comments · Fixed by #115337
Labels
P3 Issues that are less important to the Flutter project package flutter/packages repository. See also p: labels. platform-ios iOS applications specifically platform-mac Building on or for macOS specifically tool Affects the "flutter" command-line tool. See also t: labels.

Comments

@stuartmorgan
Copy link
Contributor

stuartmorgan commented Nov 10, 2022

See previous discussion in #52285. At that time the decision was to use symlinks, but that's kind of a pain due to dart-lang/pub#3143, which means all files have to be symlinked, rather than the directories (which wasn't true when the decision was made initially). For non-trivial plugins that can be a lot of files (e.g., in_app_purchase).

I'd need to look at the logic again, but I think it would be plausible to have a pubspec.yaml entry in the platform section that would allow opting into a shared directory instead of the currently hard-code ios or macos. If it's not too much complexity, I think the maintenance benefits for plugins would be worth the effort.

See also #114179

(/cc @jmagman for thoughts)

@stuartmorgan stuartmorgan added platform-ios iOS applications specifically tool Affects the "flutter" command-line tool. See also t: labels. plugin platform-mac Building on or for macOS specifically labels Nov 10, 2022
@jmagman
Copy link
Member

jmagman commented Nov 10, 2022

At least on the tool side I don't think it would be too difficult.

Would need to handle all the IOSPlugin and MacOSPlugin usage and make a... DarwinPlugin?

final List<String> platforms = <String>[
AndroidPlugin.kConfigKey,
IOSPlugin.kConfigKey,
LinuxPlugin.kConfigKey,
MacOSPlugin.kConfigKey,
WindowsPlugin.kConfigKey,
];

flutter-plugins-dependencies would have to create a new darwin key, for the projects to handle:

String get pluginConfigKey => IOSPlugin.kConfigKey;

String get pluginConfigKey => MacOSPlugin.kConfigKey;

And then the podhelper needs to know about the new key when parsing flutter-plugins-dependencies:

dependencies_hash['plugins'][platform] || []

I'm sure I'm forgetting some places.

@hellohuanlin
Copy link
Contributor

this can be part of effort of swift migration for path_provider/shared_preferences/url_launcher plugins, as discussed in the swift migration doc

@jmagman
Copy link
Member

jmagman commented Nov 10, 2022

The pubspec would also need to support a darwin field, and pub would need to be taught that it means iOS and macOS are supported.

@stuartmorgan
Copy link
Contributor Author

stuartmorgan commented Nov 10, 2022

The pubspec would also need to support a darwin field, and pub would need to be taught that it means iOS and macOS are supported.

I was thinking that rather than a new platform field, it would be a new subfield. Something like:

flutter:
  plugin:
    iOS:
      pluginClass: Foo
      sharedDarwinSource: true
    macOS:
      pluginClass: Foo
      sharedDarwinSource: true

That way we aren't conflating conceptual platforms with the directories.

If we do that, it's entirely local to Flutter, with no pub.dev changes needed.

@stuartmorgan stuartmorgan added the P3 Issues that are less important to the Flutter project label Nov 10, 2022
@jmagman
Copy link
Member

jmagman commented Nov 15, 2022

Adding just that key is easier. Got it working in draft PR #115337, it needs an end-to-end integration test.

sharedDarwinSource concept maybe needs a design doc? Or maybe this issue and that PR are sufficient to get sign off from stakeholders (you me and... @cbracken?).

Also requires documentation updates:
https://docs.flutter.dev/development/packages-and-plugins/developing-packages#step-2c-add-ios-platform-code-swifthm
https://docs.flutter.dev/development/packages-and-plugins/developing-packages#step-2e-add-macos-platform-code-swift

@cbracken
Copy link
Member

Agreed this seems like a pretty sensible approach for code that's common across both platforms (i.e. not UIKit or AppKit specific).

You could imagine cases where there's some shared Darwin code but also some AppKit or UIKit code; in such a case we'd want to support shared Darwin code in darwin and the specific bits under a macos/ios directory.

@stuartmorgan
Copy link
Contributor Author

in such a case we'd want to support shared Darwin code in darwin and the specific bits under a macos/ios directory

Pods don't allow references outside of the pod root, so that gets to be problematic. My thought was that if enough is common that you're using using sharedDarwinSource, you just put your macOS- and iOS-specific bits in the darwin directory, separated out however you want (subdirs, file naming convention, etc.)

@stuartmorgan
Copy link
Contributor Author

sharedDarwinSource concept maybe needs a design doc?

Finally got back to this: https://flutter.dev/go/plugin-shared-ios-macos

@github-actions
Copy link

github-actions bot commented Mar 4, 2023

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 4, 2023
@flutter-triage-bot flutter-triage-bot bot added the package flutter/packages repository. See also p: labels. label Jul 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
P3 Issues that are less important to the Flutter project package flutter/packages repository. See also p: labels. platform-ios iOS applications specifically platform-mac Building on or for macOS specifically tool Affects the "flutter" command-line tool. See also t: labels.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants