Set DEFINES_MODULE for FlutterPluginRegistrant to generate modulemap #40302

jmagman · 2019-09-12T01:19:49Z

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:

[module_test_ios] [STDOUT] stdout: import FlutterPluginRegistrant
[module_test_ios] [STDOUT] stdout:        ^
[module_test_ios] [STDOUT] stdout: 
[module_test_ios] [STDOUT] stderr: ** BUILD FAILED **

And pass with the DEFINES_MODULE change.

Checklist

I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
I signed the [CLA].
I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
I updated/added relevant documentation (doc comments with ///).
All existing and new tests are passing.
The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
I am willing to follow-up on review comments in a timely manner.

Breaking Change

Yes, this is a breaking change (Please read [Handling breaking changes]). Replace this with a link to the e-mail where you asked for input on this proposed change.
No, this is not a breaking change.

fluttergithubbot · 2019-09-12T01:19:50Z

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.

jmagman · 2019-09-12T01:21:19Z

packages/flutter_tools/lib/src/plugins.dart

@@ -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' }


This is the actual change, everything else is tests.

codecov · 2019-09-12T01:55:04Z

Codecov Report

Merging #40302 into master will decrease coverage by 0.2%.
The diff coverage is n/a.

@@            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

Flag	Coverage Δ
#flutter_tool	`59.21% <ø> (-0.21%)`	⬇️

Impacted Files	Coverage Δ
packages/flutter_tools/lib/src/plugins.dart	`78.08% <ø> (ø)`	⬆️
...lutter_tools/lib/src/commands/build_appbundle.dart	`90.32% <0%> (-9.68%)`	⬇️
...ckages/flutter_tools/lib/src/flutter_manifest.dart	`79.36% <0%> (-5.83%)`	⬇️
packages/flutter_tools/lib/src/asset.dart	`79.42% <0%> (-4.53%)`	⬇️
...ackages/flutter_tools/lib/src/resident_runner.dart	`52.9% <0%> (-3.81%)`	⬇️
packages/flutter_tools/lib/src/run_hot.dart	`65.51% <0%> (-3.45%)`	⬇️
packages/flutter_tools/lib/src/version.dart	`90.99% <0%> (-1.9%)`	⬇️
packages/flutter_tools/lib/src/cache.dart	`43.5% <0%> (-0.73%)`	⬇️
packages/flutter_tools/lib/src/base/utils.dart	`93.56% <0%> (-0.5%)`	⬇️
.../flutter_tools/lib/src/runner/flutter_command.dart	`80.51% <0%> (-0.44%)`	⬇️
... and 10 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ba13aa9...6920069. Read the comment docs.

xster · 2019-09-13T00:09:39Z

dev/integration_tests/ios_host_app_swift/Host/ViewController.swift

+import Flutter
+import FlutterPluginRegistrant
+
+class ViewController: UIViewController {


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.

Sure, I was copying the same behavior from the iOS ios_host_app. I can add the comment there, too.

xster · 2019-09-13T00:09:57Z

Nice! Thanks for discovering this. LGTM!

…tic framework

…lutter#40302)

jmagman added tool Affects the "flutter" command-line tool. See also t: labels. a: existing-apps Integration with existing apps via the add-to-app flow labels Sep 12, 2019

jmagman requested a review from xster September 12, 2019 01:19

jmagman self-assigned this Sep 12, 2019

fluttergithubbot added the team Infra upgrades, team productivity, code health, technical debt. See also team: labels. label Sep 12, 2019

googlebot added the cla: yes label Sep 12, 2019

jmagman commented Sep 12, 2019

View reviewed changes

This was referenced Sep 12, 2019

[google_sign_in] Plugin cannot be added to existing Swift app as a framework #40317

Closed

Make FlutterPluginRegistrant a static framework so add-to-app can use static framework plugins #40401

Merged

xster reviewed Sep 13, 2019

View reviewed changes

jmagman force-pushed the modular-headers branch from c0dafed to d54b528 Compare September 13, 2019 01:27

jmagman added 2 commits September 12, 2019 18:57

Set DEFINES_MODULE for FlutterPluginRegistrant to generate modulemap

e643773

Add view controller comments, switch one of the test plugins to a sta…

6920069

…tic framework

jmagman force-pushed the modular-headers branch from d54b528 to 6920069 Compare September 13, 2019 02:02

jmagman merged commit b80b9be into flutter:master Sep 13, 2019

jmagman deleted the modular-headers branch September 13, 2019 18:06

Inconnu08 pushed a commit to Inconnu08/flutter that referenced this pull request Sep 30, 2019

Set DEFINES_MODULE for FlutterPluginRegistrant to generate modulemap (f…

2024b12

…lutter#40302)

github-actions bot locked as resolved and limited conversation to collaborators Aug 4, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Set DEFINES_MODULE for FlutterPluginRegistrant to generate modulemap #40302

Set DEFINES_MODULE for FlutterPluginRegistrant to generate modulemap #40302

Set DEFINES_MODULE for FlutterPluginRegistrant to generate modulemap #40302

Set DEFINES_MODULE for FlutterPluginRegistrant to generate modulemap #40302

Conversation

Description

Related Issues

Tests

Checklist

Breaking Change

Choose a reason for hiding this comment

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment