[quick_actions] add custom modulemap with test submodule #6169

hellohuanlin · 2022-08-01T18:59:50Z

This is to prepare for backfilling unit tests, which is a pre-requisite for swift migration.

This plugin is chosen to be the first plugin for Swift migration, because:

It's relatively smaller, but not too small
It has good UI test coverage. Good candidate to showcase ui test in swift.
It has 0% unit tests coverage. Good candidate to showcase how to backfill unit tests
Needs to refactor a few internal states for unit tests. Good candidate to showcase pre-migration refactoring.

List which issues are fixed by this PR. You must list at least one issue.

flutter/flutter#108750

Pre-launch Checklist

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

hellohuanlin · 2022-08-01T19:02:42Z

packages/quick_actions/quick_actions_ios/example/ios/Runner/Info.plist

+	<key>CADisableMinimumFrameDurationOnPhone</key>
+	<true/>
+	<key>UIApplicationSupportsIndirectInputEvents</key>
+	<true/>


these changes are automatically added when i do flutter build ios

These should be fine I think?

Yes, these and the alwaysOutOfDate change are from auto-migrations.

hellohuanlin · 2022-08-01T19:06:16Z

packages/quick_actions/quick_actions_ios/example/ios/Podfile

@@ -1,5 +1,5 @@
 # Uncomment this line to define a global platform for your project
-# platform :ios, '9.0'
+# platform :ios, '11.0'


these changes are automatically added when i do flutter build ios

You need to manually undo them; it's a change that's on master but hasn't rolled out to stable, and we keep our plugin OS support the same as Flutter stable's OS support (except in cases where we can't for technical reasons).

stuartmorgan-g · 2022-08-01T19:38:11Z

packages/quick_actions/quick_actions_ios/example/ios/Podfile

@@ -1,5 +1,5 @@
 # Uncomment this line to define a global platform for your project
-# platform :ios, '9.0'
+# platform :ios, '11.0'


You need to manually undo them; it's a change that's on master but hasn't rolled out to stable, and we keep our plugin OS support the same as Flutter stable's OS support (except in cases where we can't for technical reasons).

stuartmorgan-g · 2022-08-01T19:38:46Z

packages/quick_actions/quick_actions_ios/example/ios/Runner/Info.plist

+	<key>CADisableMinimumFrameDurationOnPhone</key>
+	<true/>
+	<key>UIApplicationSupportsIndirectInputEvents</key>
+	<true/>


These should be fine I think?

stuartmorgan-g · 2022-08-01T19:41:17Z

packages/quick_actions/quick_actions_ios/ios/Classes/FLTQuickActionsPlugin_Test.h

+
+/// Initializes a FLTQuickActionsPlugin with the given method channel.
+/// API exposed for unit tests.
+/// @param channel a method channel


Doxygen comments follow the usual comment rules about being sentences. E.g.:

@param channel A method channel. @return The initialized FLTQuickActionsPlugin.

stuartmorgan-g · 2022-08-01T19:42:07Z

packages/quick_actions/quick_actions_ios/ios/quick_actions_ios.podspec

  s.documentation_url = 'https://pub.dev/packages/quick_actions'
-  s.source_files = 'Classes/**/*'
+  s.source_files = 'Classes/**/*.{h,m}'
  s.public_header_files = 'Classes/**/*.h'


This is making the test header a public header. We've made that mistake in other plugins, but we shouldn't replicate it here.

@stuartmorgan Looks like the test header must be public. I got this error when not adding the test header to this public_header_files:

- ERROR | [iOS] xcodebuild: /Users/admin/Library/Developer/Xcode/DerivedData/App-cgyuubhexmicjpawtwtidfnntejc/Build/Products/Debug-iphonesimulator/quick_actions_ios/quick_actions_ios.framework/Modules/module.modulemap:8:12: error: header 'FLTQuickActionsPlugin_Test.h' not found

If I understand this error correctly, modulemap file defines the module by mapping headers in the output directory. The Test submodule is no difference than regular module in terms of visibility, except that it's scoped under a submodule.

CC @jmagman probably know more about modulemap.

Please ignore my previous comment, I misunderstood how private headers work. It turned out that private headers ARE copied to the output directory (so that modulemap can refer to them).

jmagman · 2022-08-02T18:52:37Z

packages/quick_actions/quick_actions_ios/example/ios/Runner/Info.plist

+	<key>CADisableMinimumFrameDurationOnPhone</key>
+	<true/>
+	<key>UIApplicationSupportsIndirectInputEvents</key>
+	<true/>


Yes, these and the alwaysOutOfDate change are from auto-migrations.

jmagman · 2022-08-02T18:56:59Z

packages/quick_actions/quick_actions_ios/ios/Classes/FLTQuickActionsPlugin.m

  [registrar addApplicationDelegate:instance];
 }

+- (instancetype)initWithChannel:(FlutterMethodChannel *)channel {


should -init and +new be marked unavailable in the header then?

jmagman · 2022-08-02T18:57:11Z

packages/quick_actions/quick_actions_ios/ios/Classes/FLTQuickActionsPlugin_Test.h

+/// API exposed for unit tests.
+/// @param channel A method channel.
+/// @return The initialized FLTQuickActionsPlugin.
+- (instancetype)initWithChannel:(FlutterMethodChannel *)channel;


Designated initializer?

Got this error. Looks like designated initializer does not support class categories:

'objc_designated_initializer' attribute only applies to init methods of interface or class extension declarations

hellohuanlin · 2022-08-03T17:48:52Z

@stuartmorgan friendly ping

hellohuanlin · 2022-08-04T19:10:09Z

packages/quick_actions/quick_actions/CHANGELOG.md

@@ -1,5 +1,6 @@
-## NEXT
+## 0.6.0+12


Hmmm, looks like the version bump is not picked up by the CI.

You changed the version in the wrong package; the changes are in quick_actions_ios.

Good catch!

stuartmorgan-g

LGTM

jmagman · 2022-08-04T21:17:54Z

packages/quick_actions/quick_actions_ios/ios/quick_actions_ios.podspec

-  s.source_files = 'Classes/**/*'
-  s.public_header_files = 'Classes/**/*.h'
+  s.source_files = 'Classes/**/*.{h,m}'
+  s.public_header_files = 'Classes/*.h'


Doesn't this also match Classes/PrivateHeaders/*.h?

Oh I removed /**, so it won't include the private headers.

jmagman · 2022-08-04T21:20:22Z

packages/quick_actions/quick_actions_ios/ios/Classes/FLTQuickActionsPlugin.h


 @interface FLTQuickActionsPlugin : NSObject <FlutterPlugin>
+
+/// Unavailable. Please use `initWithChannel:` instead.


But that's "private", how would they be able to do it? I think registerWithRegistrar: is the only way for developers to create an instance.

Oh the unit tests will import the Test submodule that contains initWithChannel: private API. But I think you made a good point that a public header shouldn't mention about this private API. I will remove this comment in next PR.

jmagman · 2022-08-04T21:20:49Z

packages/quick_actions/quick_actions_ios/ios/Classes/FLTQuickActionsPlugin_Test.h

+/// API exposed for unit tests.
+/// @param channel A method channel.
+/// @return The initialized FLTQuickActionsPlugin.
+- (instancetype)initWithChannel:(FlutterMethodChannel *)channel;


…lutter/plugins#6169)

github-actions bot added p: quick_actions platform-ios labels Aug 1, 2022

hellohuanlin commented Aug 1, 2022

View reviewed changes

hellohuanlin marked this pull request as ready for review August 1, 2022 19:04

hellohuanlin requested a review from stuartmorgan-g as a code owner August 1, 2022 19:04

hellohuanlin requested review from cyanglaz and jmagman August 1, 2022 19:05

hellohuanlin commented Aug 1, 2022

View reviewed changes

stuartmorgan-g suggested changes Aug 1, 2022

View reviewed changes

hellohuanlin requested a review from stuartmorgan-g August 1, 2022 20:56

jmagman reviewed Aug 2, 2022

View reviewed changes

hellohuanlin force-pushed the quick_actions_modulemap branch 2 times, most recently from e76e91f to 8a780b8 Compare August 2, 2022 21:45

hellohuanlin added 6 commits August 3, 2022 10:48

[quick_actions]add custom modulemap with test module

2d4d526

update changelog and pubspec

97615b5

nit

beefee6

move public header files under PublicHeaders/ directory

635396c

revert ios 11 version bump

c58a91c

mark init as ns_unavailable

db79c26

hellohuanlin force-pushed the quick_actions_modulemap branch from 8a780b8 to db79c26 Compare August 3, 2022 17:48

use private_header_files

e732cfc

hellohuanlin force-pushed the quick_actions_modulemap branch from 4c3dc31 to e732cfc Compare August 4, 2022 18:23

hellohuanlin commented Aug 4, 2022

View reviewed changes

fix version bump

9fef548

stuartmorgan-g approved these changes Aug 4, 2022

View reviewed changes

hellohuanlin added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 4, 2022

auto-submit bot merged commit 0146406 into flutter:main Aug 4, 2022

jmagman reviewed Aug 4, 2022

View reviewed changes

engine-flutter-autoroll mentioned this pull request Aug 5, 2022

Roll Plugins from 5f2e02cf63b4 to f07efc23410b (4 revisions) flutter/flutter#109044

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 5, 2022

01464063c [quick_actions] add custom modulemap with test submodule (f…

8b33d2a

…lutter/plugins#6169)

yutaaraki-toydium pushed a commit to yutaaraki-toydium/plugins that referenced this pull request Aug 12, 2022

[quick_actions] add custom modulemap with test submodule (flutter#6169)

1049e0c

moisefeelin pushed a commit to feelinproject/plugins that referenced this pull request Aug 26, 2022

[quick_actions] add custom modulemap with test submodule (flutter#6169)

ae3e0e2

mauricioluz pushed a commit to mauricioluz/plugins that referenced this pull request Jan 26, 2023

[quick_actions] add custom modulemap with test submodule (flutter#6169)

8fd6f2e


		@interface FLTQuickActionsPlugin : NSObject <FlutterPlugin>

		/// Unavailable. Please use `initWithChannel:` instead.

[quick_actions] add custom modulemap with test submodule #6169

[quick_actions] add custom modulemap with test submodule #6169

Uh oh!

Conversation

hellohuanlin commented Aug 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hellohuanlin Aug 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hellohuanlin commented Aug 3, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stuartmorgan-g left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hellohuanlin Aug 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

hellohuanlin commented Aug 1, 2022 •

edited

Loading

hellohuanlin Aug 1, 2022 •

edited

Loading

hellohuanlin Aug 4, 2022 •

edited

Loading