[ios]Fix compile error when conforming UIApplication to Launcher due to MainActor annotation #7100
Conversation
…cation to Launcher due to main actor annotation bump version
hellohuanlin
force-pushed
the
urllauncher_fix_main_actor_completion_conformance
branch
from
78758e6 to
be43856
Compare
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact "@test-exemption-reviewer" in the #hackers channel in Chat (don't just cc them here, they won't see it! Use Discord!). If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
If we were to just add the annotations to |
|
test-exempt: Cannot be tested with the tooling currently in CI; will automatically be tested after the next major Xcode update in CI. |
| extension UIApplication: Launcher {} | ||
| // TODO(hellohuanlin): This wrapper is a workaround for iOS 18 Beta 3 where completionHandler is annotated with @MainActor @Sendable, resulting in compile error when conforming UIApplication to Launcher. We should try again in newer betas. | ||
| /// A default URL launcher. | ||
| final class DefaultLauncher: Launcher { |
There was a problem hiding this comment.
@hellohuanlin Launcher is documented to only exist for testing purposes.
I know you reviewed the PR, based on your DeviceProtocol example, that's the idiomatic way you suggest doing this, or is there another way?
There was a problem hiding this comment.
The idiomatic way is (as shown in wwdc's demo code a few years ago):
(1) for real dependency, extend system type to conform to our protocol (e.g. extension UIApplication: Launcher)
(2) for fake dependency, define a custom type that conforms to the same protocol (e.g. class MockLauncher: Launcher)
In this case, (1) doesn't work anymore. So we have to define the custom type (e.g. class DefaultLauncher: Launcher).
However, I highly suspect Apple may fix it in newer betas, so that we can revert this PR. As discussed in the PR description, Apple seems to suggest that if a module is not ready to adopt swift concurrency, it should just work as before. So they are not supposed to break us.
There was a problem hiding this comment.
looks like ios18 and xcode update break it...
Yes it breaks in older Xcode. |
|
(modulo the Swift formatter CI failure) |
hellohuanlin
added
the
autosubmit
…her due to MainActor annotation (flutter/packages#7100)
…her due to MainActor annotation (flutter/packages#7100)
…her due to MainActor annotation (flutter/packages#7100)
flutter/packages@ea35fc6...3379e51 2024-07-12 [email protected] [pigeon] Fix C++ enum naming (flutter/packages#7094) 2024-07-12 [email protected] [ci] Split build_all_packages by JDK version. (flutter/packages#7106) 2024-07-12 [email protected] [ios]Fix compile error when conforming UIApplication to Launcher due to MainActor annotation (flutter/packages#7100) 2024-07-11 [email protected] [go_router] Makes GoRouterState lookup more robust. (flutter/packages#6920) 2024-07-11 [email protected] [ci][web] Run tests in canvaskit mode. (flutter/packages#6879) 2024-07-10 49699333+dependabot[bot]@users.noreply.github.com [image_picker]: Bump androidx.core:core from 1.10.1 to 1.13.1 in /packages/image_picker/image_picker_android/android (flutter/packages#6648) 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
|
This seems to be not live on pub.dev yet, the last version was 6.3.0 from 3 months ago. When will this be released? |
@ciriousjoker url_launcher_ios is on 6.3.1 https://pub.dev/packages/url_launcher_ios/changelog |
This comment was marked as outdated.
This comment was marked as outdated.
|
@ahmedmahershaaban All you need to do is run |
|
|
@xiaoliang-cn If you have a situation where |
iOS 18 Beta 3 added
@MainActor @Sendablefor openURL's completion handler:The addition of
@MainActoris the one that caused the compile error. It was not there for Beta 1.This PR we simply use a
DefaultLauncherwrapper. As expected, passing in a non-isolated closure to a function that expects a main-actor isolated closure is fine, since non-isolated closure can be called in any isolation context; plus the minimal concurrency checking also silences the Sendable warning.But we should check again in newer betas if we can revert this change. Because this compile error forces some libraries to update for swift concurrency, which seems to be against the whole idea of having 3 different levels of concurrency checking. So I suspect that Apple is still hashing things out and may change it back (either by removing
@MainActorfrom signature, or compiler change to allow this conformance under minimal checking)List which issues are fixed by this PR. You must list at least one issue.
Fixes flutter/flutter#151467
Pre-launch Checklist
dart format.)[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style, or this PR is exempt from CHANGELOG changes.///).If you need help, consider asking for advice on the #hackers-new channel on Discord.