Skip to content
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

Avoid MainActor.assumeIsolated #265

Merged
merged 1 commit into from
Jun 23, 2024
Merged

Avoid MainActor.assumeIsolated #265

merged 1 commit into from
Jun 23, 2024

Conversation

hallee
Copy link
Contributor

@hallee hallee commented Jun 23, 2024

MainActor.assumeIsolated should generally be avoided since it's a last-resort method to get around the concurrency system, but also it's limited to newer platform versions. I assume it wasn't intentional to limit PulseUI's supported platforms this way?

'assumeIsolated(_:file:line:)' is only available in iOS 17.0 or newer

@kean
Copy link
Owner

kean commented Jun 23, 2024

I assume it wasn't intentional to limit PulseUI's supported platforms this way?

No, and I'm surprise Xcode didn't throw any error when I built it using Xcode 15.4 :/
Thanks!

@kean kean merged commit e253ebe into kean:main Jun 23, 2024
4 of 10 checks passed
@AgapovOne
Copy link
Contributor

But docs are clear:

iOS 13.0+
iPadOS 13.0+
Mac Catalyst 13.0+
macOS 10.15+
tvOS 13.0+
visionOS 1.0+
watchOS 6.0+

Actually it was in one Xcode version that assumeIsolated got only on 17.0+, but they backported it later

swiftlang/swift#70474

@kean
Copy link
Owner

kean commented Jun 24, 2024

I would not be surprised if there is an issue with the annotations, as it happened before kean/Nuke#792.

I removed the use of this API in the latest patch just to be safe. I need to allocate some time to do a proper Swift 6 migration, and running it on the lower OS versions the framework supports seems to be a requirement. It will be a good time to invest in some UI test automation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants