Skip to content

[camera_avfoundation] Implementation swift migration #8988

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

Conversation

RobertOdrowaz
Copy link
Contributor

@RobertOdrowaz RobertOdrowaz commented Apr 3, 2025

Migrates camera implementation as part of flutter/flutter#119109

This PR includes the CameraPlugin class migration. Some additional changes regarding imports were necessary to keep Swift Package Manager compatibility. SPM does not support mixed languages in a single target. I assume there might be a better solution so I'm open to suggestions

After splitting only a new target for SPM is created in this PR and the objc implementation is moved to a dedicated directory.

During the plugin's migration from ObjC to Swift, there will be a transitional period during which the implementation will be mixed, with some parts already migrated to Swift and some still in ObjC. This is somewhat problematic with SwiftPM. SwiftPM does support packages with mixed language implementation but different languages have to be in separate targets. Every target defines a separate module which means that in some cases both have to be imported. However, this only applies to SwiftPM with Cocoapods we can mix Swift and ObjC code in the same pod and have a single module. Simmingly the best solution is to use conditional imports for the additional module when SwiftPM during the migration.

@testable import camera_avfoundation

#if canImport(camera_avfoundation_objc)
  @testable import camera_avfoundation_objc
#endif

Pre-Review Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

Footnotes

  1. Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. 2 3

@RobertOdrowaz RobertOdrowaz force-pushed the feature/camera-implementation-swift-migration branch from 9ba0c7b to 24ef036 Compare April 4, 2025 14:54
@RobertOdrowaz RobertOdrowaz force-pushed the feature/camera-implementation-swift-migration branch 2 times, most recently from 1d00092 to 912ce03 Compare April 5, 2025 11:52
@RobertOdrowaz RobertOdrowaz force-pushed the feature/camera-implementation-swift-migration branch 2 times, most recently from c0faae7 to d99fe5c Compare April 12, 2025 17:53
@hellohuanlin
Copy link
Contributor

Every target defines a separate Swift module which means that in some cases both will have to be imported.

I'm confused, the ObjC target doesn't define a "swift" module right? isn't it still a clang module?

@@ -13,9 +13,13 @@ A Flutter plugin to use the camera from your Flutter app.
s.author = { 'Flutter Dev Team' => '[email protected]' }
s.source = { :http => 'https://github.com/flutter/packages/tree/main/packages/camera_avfoundation' }
s.documentation_url = 'https://pub.dev/packages/camera_avfoundation'
s.source_files = 'camera_avfoundation/Sources/camera_avfoundation/**/*.{h,m}'
s.public_header_files = 'camera_avfoundation/Sources/camera_avfoundation/include/**/*.h'
s.module_map = 'camera_avfoundation/Sources/camera_avfoundation/include/CameraPlugin.modulemap'
Copy link
Contributor

@hellohuanlin hellohuanlin Apr 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's been a while since i touched this. Why we are removing the module map?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not supported with Swift static libraries.

[!] Using Swift static libraries with custom module maps is currently not supported. Please build `camera_avfoundation` as a framework or remove the custom module map.

It's still the same issue you've encountered here flutter/plugins#6369

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh i completely forgot about this! thanks for the reference.

dependencies: [],
exclude: ["include/camera_avfoundation-umbrella.h", "include/CameraPlugin.modulemap"],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same question to the umbrella header.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct me if I'm missing something but I don't see much value in having a custom umbrella header without a custom module map. Without the module map cocoapods generate the public umbrella header that includes everything either way

@RobertOdrowaz RobertOdrowaz force-pushed the feature/camera-implementation-swift-migration branch from d99fe5c to 57e7fe1 Compare April 15, 2025 06:31
@RobertOdrowaz
Copy link
Contributor Author

RobertOdrowaz commented Apr 15, 2025

I'm confused, the ObjC target doesn't define a "swift" module right? isn't it still a clang module?

Yes, there won't be a swift module for the ObjC target. I've corrected the description

@RobertOdrowaz RobertOdrowaz added the autosubmit Merge PR when tree becomes green via auto submit App label Apr 16, 2025
@auto-submit auto-submit bot merged commit 7728bba into flutter:main Apr 16, 2025
82 checks passed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autosubmit Merge PR when tree becomes green via auto submit App p: camera platform-ios platform-macos
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants