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

Support Xcode 16.2 onPreferenceChange SDK change #48

Merged
merged 5 commits into from
Dec 12, 2024

Conversation

philippzagar
Copy link
Member

Support Xcode 16.2 onPreferenceChange SDK change

♻️ Current situation & Problem

As shown in #47 and StanfordSpezi/SpeziLLM#76, Apple chose to change the signature of the onPreferenceChange view modifier in SwiftUI with the latest Xcode 16.2 RC (also the betas) and the iOS 18.2 SDKs.
In its current state, this prevents the SpeziViews package from compiling in these environments.

The onPreferenceChange view modfier now takes a @Sendable closure, therefore we cannot capture @MainActor isolated properties on the View directly anymore: https://developer.apple.com/documentation/swiftui/view/onpreferencechange(_:perform:)?changes=latest_minor
However, as the @Sendable closure is still run on the MainActor's serial executor (at least in my testing on 18.2 RC SDKs), we can use MainActor.assumeIsolated to avoid scheduling a MainActor Task, which could delay execution and cause unexpected UI behavior.

⚙️ Release Notes

  • Support Xcode 16.2 onPreferenceChange SDK change
  • Use Swift 6 language mode in the UITest app

📚 Documentation

Properly documented in-line why this change was necessary

✅ Testing

Tested the changes locally on Xcode 16.2 RC as well as iOS 18.2 betas.
We should lifting the CI builds / testing to these versions as well, as soon as they're out (should only be a couple of days).

📝 Code of Conduct & Contributing Guidelines

By submitting creating this pull request, you agree to follow our Code of Conduct and Contributing Guidelines:

@philippzagar philippzagar added the bug Something isn't working label Dec 9, 2024
@philippzagar philippzagar self-assigned this Dec 9, 2024
@philippzagar
Copy link
Member Author

There's also a concurrency warning in ReceiveValidationModifier, as the ValidationState.Binding (a non-Sendable type) is used in the (now) @Sendable closure of .onPreferenceChange().
An easy fix would be to annotate ValidationState.Binding as @unchecked Sendable, but that's technically no correct as the nested ValidationContext cannot be easily made Sendable in its current form.
Looking forward to your opinion @Supereg!

@philippzagar philippzagar requested a review from Supereg December 9, 2024 08:27
@philippzagar
Copy link
Member Author

philippzagar commented Dec 11, 2024

@Supereg Please do a quick review of the PR, would be great to get this code in soon as the Xcode 16.2 RC was released nearly a week ago (Dec 5), meaning that the public release will probably come in a couple of days.
As lots of Spezi packages and apps depend on SpeziViews, it would be great to do this before the public release 🚀

Just noticed: I didn't take a look into why the snapshot tests fail yet, will probably be better if you take a quick glance into that, would be way quicker!

Copy link

codecov bot commented Dec 12, 2024

Codecov Report

Attention: Patch coverage is 69.23077% with 4 lines in your changes missing coverage. Please review.

Project coverage is 74.91%. Comparing base (f875144) to head (2cf85ba).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...iViews/Views/Layout/HorizontalGeometryReader.swift 69.24% 4 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #48      +/-   ##
==========================================
- Coverage   74.97%   74.91%   -0.06%     
==========================================
  Files          52       52              
  Lines        1574     1586      +12     
==========================================
+ Hits         1180     1188       +8     
- Misses        394      398       +4     
Files with missing lines Coverage Δ
Sources/SpeziValidation/ValidationRule.swift 53.71% <ø> (ø)
Sources/SpeziViews/Views/Button/AsyncButton.swift 66.95% <ø> (ø)
...iViews/Views/Layout/HorizontalGeometryReader.swift 86.67% <69.24%> (-13.33%) ⬇️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f875144...2cf85ba. Read the comment docs.

@philippzagar philippzagar enabled auto-merge (squash) December 12, 2024 12:46
@philippzagar
Copy link
Member Author

@PSchmiedmayer Please merge the PR with bypassing merge requirements. Minor decrease in test coverage as we added a "safety" case in case SwiftUI ever moves the onPreferenceChange() closure off the MainActor.

@PSchmiedmayer PSchmiedmayer merged commit 2997114 into main Dec 12, 2024
13 checks passed
@PSchmiedmayer PSchmiedmayer deleted the fix/xcode-16-2-concurrency branch December 12, 2024 19:46
@PSchmiedmayer
Copy link
Member

Amazing, thank you for the fix @philippzagar!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants