search

kean / Pulse

Network logger for Apple platforms
https://pulselogger.com
MIT License
6.18k stars 294 forks source link

Avoid `MainActor.assumeIsolated` #265

Closed hallee closed 1 month ago

hallee commented 2 months ago

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 commented 1 month ago

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!

AgapovOne commented 1 month ago

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

https://github.com/swiftlang/swift/pull/70474

kean commented 1 month ago

I would not be surprised if there is an issue with the annotations, as it happened before https://github.com/kean/Nuke/pull/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.