-
Notifications
You must be signed in to change notification settings - Fork 27.1k
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
Set DEFINES_MODULE for FlutterPluginRegistrant to generate modulemap #40302
Conversation
It looks like this pull request may not have tests. Please make sure to add tests before merging. While there are exceptions to this rule, if this patch modifies code it is probably not an exception. Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
@@ -383,6 +383,7 @@ Depends on all your plugins, and provides a function to register them. | |||
s.source_files = "Classes", "Classes/**/*.{h,m}" | |||
s.source = { :path => '.' } | |||
s.public_header_files = './Classes/**/*.h' | |||
s.pod_target_xcconfig = { 'DEFINES_MODULE' => 'YES' } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the actual change, everything else is tests.
Codecov Report
@@ Coverage Diff @@
## master #40302 +/- ##
==========================================
- Coverage 59.42% 59.21% -0.21%
==========================================
Files 192 192
Lines 18602 18602
==========================================
- Hits 11055 11016 -39
- Misses 7547 7586 +39
Continue to review full report at Codecov.
|
import Flutter | ||
import FlutterPluginRegistrant | ||
|
||
class ViewController: UIViewController { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file is built but nothing really runs it currently right? If so, add a comment. People looking at projects in a 'integration_tests' folder might expect this to be test harnessed somewhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I was copying the same behavior from the iOS ios_host_app. I can add the comment there, too.
Nice! Thanks for discovering this. LGTM! |
c0dafed
to
d54b528
Compare
d54b528
to
6920069
Compare
Description
Let CocoaPods generate a module map for FlutterPluginRegistrant so it can be imported by existing Swift apps that don't
use_frameworks!
as a static library.Bonus: existing Objective-C apps can use
@import FlutterPluginRegistrant
syntax and any existing imports or includes will become module imports and leverage the module cache.See http://blog.cocoapods.org/CocoaPods-1.5.0/.
Related Issues
Fixes #40289.
Gives a workaround to #31104 with Swift apps importing static libraries (don't use
use_frameworks!
in the Podfile).Tests
I added a Swift app to the module_test_ios. They fail before without the DEFINES_MODULE change:
And pass with the DEFINES_MODULE change.
Checklist
///
).flutter analyze --flutter-repo
) does not report any problems on my PR.Breaking Change