[file_selector] Convert iOS to Swift and SPM #6755

stuartmorgan · 2024-05-17T16:45:47Z

Converts the plugin and its tests to Swift, and re-adds the SPM support that was reverted due to modulemap issues.

In order to avoid the ugliness and loss of type saftey of using associated objects in the Swift version, this replaces that with a bridge object that serves as a delegate instead of the plugin, and manages its own lifetime in coordination with the plugin.

While this is one PR, the conversion was done in individual steps, each of which is a commit:

Rewrite just the tests in Swift, with no implementation changes, ensuring that everything still passed.
Rewrite the implementation in Swift, changing the tests only as necessary for the structural changes to the implementation due to the removal of associated objects
Re-add SPM

The changes in the generated Dart files are just due to updating to the latest version of Pigeon (to avoid writing the Swift implementation against an older version of the Swift API, and then having to update again later for breaking changes).

Part of flutter/flutter#119015
Fixes flutter/flutter#146903

Pre-launch Checklist

stuartmorgan · 2024-05-17T16:47:33Z

This is the easiest way to remove the use of modulemaps, right? 😁

Let me know if you want me to actually break this up into separate PRs. The overall change isn't actually that big so this seemed potentially easier, but I could easily split it along commit lines.

jmagman

@hellohuanlin could you review just the files FileSelectorPlugin.swift ViewPresenter.swift for idiomatic Swift?

LGTM

hellohuanlin · 2024-05-22T20:54:29Z

packages/file_selector/file_selector_ios/example/ios/RunnerTests/FileSelectorTests.swift

+
+@testable import file_selector_ios
+
+class TestViewPresenter: ViewPresenter {


nit: final

hellohuanlin · 2024-05-22T20:56:45Z

...r/file_selector_ios/ios/file_selector_ios/Sources/file_selector_ios/FileSelectorPlugin.swift

+ sendResult(
+ urls.map({ document in
+ document.path
+ }))


nit: sendResult(urls.map { $0.path })

Ah right, I forgot Swift has shorthand for this.

can also use key path urls.map(\.path) which is even shorter. sometimes i forget about it (no need to change tho)

hellohuanlin · 2024-05-22T21:00:53Z

...r/file_selector_ios/ios/file_selector_ios/Sources/file_selector_ios/FileSelectorPlugin.swift

+ ?? UIDocumentPickerViewController(
+ documentTypes: config.utis.map({ uti in
+ // See comment in messages.dart for why this is safe.
+ uti!


can cast directly config.utis as! [TypeOfThis] without the loop

Interesting, I guess I've internalized the Dart model of having to cast elements rather than the collections themselves.

hellohuanlin · 2024-05-22T21:01:16Z

...r/file_selector_ios/ios/file_selector_ios/Sources/file_selector_ios/FileSelectorPlugin.swift

+ documentTypes: config.utis.map({ uti in
+ // See comment in messages.dart for why this is safe.
+ uti!
+ }), in: UIDocumentPickerMode.import)


nit: in: .import

Done. Someday I'll internalize this one.

hellohuanlin · 2024-05-22T21:01:51Z

...r/file_selector_ios/ios/file_selector_ios/Sources/file_selector_ios/FileSelectorPlugin.swift

+ documentPicker.delegate = completionBridge
+
+ let presenter =
+ self.viewPresenterOverride ?? UIApplication.shared.delegate?.window??.rootViewController


is viewPresenterOverride created for testing purpose?

Yes, both to avoid actually having modal UI pop up (which causes issues when running multiple tests since we can't dismiss it) and to assert that it's called with expected values.

…ges#6755)

flutter/packages@6525441...1008d9e 2024-05-23 [email protected] Roll Flutter from 73bf206 to 8d955cd (24 revisions) (flutter/packages#6786) 2024-05-23 [email protected] Roll Flutter (stable) from 5dcb86f to a14f74f (3 revisions) (flutter/packages#6784) 2024-05-23 [email protected] [file_selector] Convert iOS to Swift and SPM (flutter/packages#6755) 2024-05-23 49699333+dependabot[bot]@users.noreply.github.com [webview]: Bump androidx.webkit:webkit from 1.7.0 to 1.10.0 in /packages/webview_flutter/webview_flutter_android/android (flutter/packages#5996) 2024-05-23 [email protected] Roll Flutter from d02292d to 73bf206 (31 revisions) (flutter/packages#6780) 2024-05-23 [email protected] [rfw] Adds support for `DecorationImage.filterQuality`. (flutter/packages#6781) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

…r#148980) flutter/packages@6525441...1008d9e 2024-05-23 [email protected] Roll Flutter from 73bf206 to 8d955cd (24 revisions) (flutter/packages#6786) 2024-05-23 [email protected] Roll Flutter (stable) from 5dcb86f to a14f74f (3 revisions) (flutter/packages#6784) 2024-05-23 [email protected] [file_selector] Convert iOS to Swift and SPM (flutter/packages#6755) 2024-05-23 49699333+dependabot[bot]@users.noreply.github.com [webview]: Bump androidx.webkit:webkit from 1.7.0 to 1.10.0 in /packages/webview_flutter/webview_flutter_android/android (flutter/packages#5996) 2024-05-23 [email protected] Roll Flutter from d02292d to 73bf206 (31 revisions) (flutter/packages#6780) 2024-05-23 [email protected] [rfw] Adds support for `DecorationImage.filterQuality`. (flutter/packages#6781) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

Converts the plugin and its tests to Swift, and re-adds the SPM support that was reverted due to modulemap issues. In order to avoid the ugliness and loss of type saftey of using associated objects in the Swift version, this replaces that with a bridge object that serves as a delegate instead of the plugin, and manages its own lifetime in coordination with the plugin. While this is one PR, the conversion was done in individual steps, each of which is a commit: - Rewrite just the tests in Swift, with no implementation changes, ensuring that everything still passed. - Rewrite the implementation in Swift, changing the tests only as necessary for the structural changes to the implementation due to the removal of associated objects - Re-add SPM The changes in the generated Dart files are just due to updating to the latest version of Pigeon (to avoid writing the Swift implementation against an older version of the Swift API, and then having to update again later for breaking changes). Part of flutter/flutter#119015 Fixes flutter/flutter#146903

Convert tests to Swift

a8b038b

stuartmorgan requested a review from jmagman as a code owner May 17, 2024 16:45

github-actions bot added p: file_selector platform-ios labels May 17, 2024

stuartmorgan requested a review from bparrishMines as a code owner May 17, 2024 17:08

stuartmorgan force-pushed the file-selector-ios-swift branch from eb6df7f to cb91c01 Compare May 17, 2024 17:09

stuartmorgan added 3 commits May 17, 2024 13:10

Convert implementation to Swift

02b7305

Re-add SPM support

2b49872

Version bump

6ce49a3

stuartmorgan force-pushed the file-selector-ios-swift branch from cb91c01 to 6ce49a3 Compare May 17, 2024 17:10

stuartmorgan removed the request for review from bparrishMines May 17, 2024 17:20

jmagman approved these changes May 22, 2024

View reviewed changes

jmagman requested a review from hellohuanlin May 22, 2024 19:49

hellohuanlin approved these changes May 22, 2024

View reviewed changes

stuartmorgan added 2 commits May 22, 2024 20:17

Merge branch 'main' into file-selector-ios-swift

d518add

Review feedback

3dd7d9d

stuartmorgan added the autosubmit Merge PR when tree becomes green via auto submit App label May 23, 2024

auto-submit bot merged commit df16d4e into flutter:main May 23, 2024
74 checks passed

engine-flutter-autoroll mentioned this pull request May 23, 2024

Roll Packages from 65254411e677 to df16d4e7bde3 (4 revisions) flutter/flutter#148951

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 23, 2024

df16d4e7b [file_selector] Convert iOS to Swift and SPM (flutter/packa…

4c1e10d

…ges#6755)

engine-flutter-autoroll mentioned this pull request May 23, 2024

Roll Packages from 65254411e677 to 13d111d92ad5 (5 revisions) flutter/flutter#148962

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 23, 2024

df16d4e7b [file_selector] Convert iOS to Swift and SPM (flutter/packa…

2a0dbed

…ges#6755)

engine-flutter-autoroll mentioned this pull request May 23, 2024

Roll Packages from 65254411e677 to 1008d9e9cd07 (6 revisions) flutter/flutter#148980

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 23, 2024

df16d4e7b [file_selector] Convert iOS to Swift and SPM (flutter/packa…

d3b997e

…ges#6755)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[file_selector] Convert iOS to Swift and SPM #6755

[file_selector] Convert iOS to Swift and SPM #6755

stuartmorgan commented May 17, 2024

stuartmorgan commented May 17, 2024

jmagman left a comment

hellohuanlin May 22, 2024

stuartmorgan May 23, 2024

hellohuanlin May 22, 2024

stuartmorgan May 23, 2024

hellohuanlin May 23, 2024

hellohuanlin May 22, 2024

stuartmorgan May 23, 2024

hellohuanlin May 22, 2024

stuartmorgan May 23, 2024

hellohuanlin May 22, 2024

stuartmorgan May 23, 2024


		@testable import file_selector_ios

		class TestViewPresenter: ViewPresenter {

[file_selector] Convert iOS to Swift and SPM #6755

[file_selector] Convert iOS to Swift and SPM #6755

Conversation

stuartmorgan commented May 17, 2024

Pre-launch Checklist

stuartmorgan commented May 17, 2024

jmagman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment