🔥 Remove availability checks made dead by the iOS 15 floor - #159
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
UserDefault.subject is now backed by a lazy var, which introduces a real thread-safety regression (data race) on concurrent first access and should be made safely single-initializing.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Removes availability annotations and runtime #available branches that are now permanently true after raising the minimum supported OS versions (iOS 15 / macOS 12 / tvOS 15 / watchOS 9), and updates related build/CI/docs to match the new floor.
Changes:
- Simplify code across modules by deleting
@availableattributes and unwrapping deadif #availablebranches. - Raise deployment targets in SPM + Xcode project and document the new requirements.
- Update CI Xcode pin and simulator destinations; record changes in the changelog.
File summaries
| File | Description |
|---|---|
| Tests/NetworkingTests/OAuthInterceptor_Tests.swift | Removes redundant test-case availability attribute. |
| Tests/NetworkingTests/APIService+OAuthInterceptor_Tests.swift | Removes redundant test-case availability attribute. |
| Tests/NetworkingTests/APIService_Tests.swift | Removes redundant test-case availability attribute. |
| Tests/ACKategoriesTests/VersionUpdate/VersionUpdateManager_Tests.swift | Removes redundant test-case availability attribute. |
| Tests/ACKategoriesTests/PropertyWrappers/UserDefaultTests.swift | Removes redundant test-case availability attribute. |
| Tests/ACKategoriesTests/EdgeInsetsTests.swift | Removes redundant test-case availability attribute. |
| Sources/PushNotifications/UNNotificationSettingsExtensions.swift | Removes redundant extension availability attribute. |
| Sources/PushNotifications/PushManager.swift | Removes redundant availability attributes from protocols/class/delegate conformance. |
| Sources/Networking/OAuthInterceptor.swift | Removes redundant actor availability attribute. |
| Sources/Networking/Networking.swift | Removes redundant protocol/extension availability attributes. |
| Sources/Networking/APIServicing.swift | Removes redundant protocol availability attributes. |
| Sources/Networking/APIService.swift | Removes redundant class availability attribute. |
| Sources/ACKategoriesTesting/VersionUpdateFetcher_Mock.swift | Removes redundant mock availability attribute. |
| Sources/ACKategoriesTesting/Networking/NetworkMock.swift | Removes redundant mock availability attribute. |
| Sources/ACKategoriesTesting/Networking/APIServiceMock.swift | Removes redundant mock availability attribute. |
| Sources/ACKategories/VersionUpdate/VersionUpdateManager.swift | Removes redundant protocol/class availability attributes. |
| Sources/ACKategories/VersionUpdate/MinBuildNumberFetcher.swift | Removes redundant protocol availability attribute. |
| Sources/ACKategories/UIDeviceExtensions.swift | Removes redundant availability attribute on isMac. |
| Sources/ACKategories/UIColorExtensions.swift | Unwraps dead #available branches in image() generation. |
| Sources/ACKategories/UI/UISearchBarExtensions.swift | Removes dead runtime availability branch in deprecated textField shim. |
| Sources/ACKategories/SwiftUIExtensions/View+FrameSize.swift | Removes redundant SwiftUI extension availability attribute. |
| Sources/ACKategories/SwiftUIExtensions/View+Frame.swift | Removes redundant availability attributes, keeps preview. |
| Sources/ACKategories/SwiftUIExtensions/SwiftUILayoutGuides.swift | Removes redundant availability attributes; trims preview gate to @available(iOS 16.0, *). |
| Sources/ACKategories/SwiftUIExtensions/SwiftUIColorsTheme.swift | Removes redundant availability attributes from theme helpers. |
| Sources/ACKategories/SwiftUIExtensions/FontModifier.swift | Removes redundant availability attribute (iOS 15+ floor). |
| Sources/ACKategories/SwiftUIExtensions/ACKHostingController.swift | Removes redundant availability attribute for hosting controller. |
| Sources/ACKategories/PublisherExtensions.swift | Removes redundant availability attributes on Combine helpers. |
| Sources/ACKategories/PropertyWrappers/UserDefault.swift | Removes availability gating; replaces associated-object subject storage with lazy subject. |
| Sources/ACKategories/EdgeInsetsExtensions.swift | Removes redundant SwiftUI extension availability attribute. |
| Sources/ACKategories/Combine+Concurrency.swift | Removes redundant availability attributes on Combine concurrency helpers. |
| README.md | Adds Requirements section and updates wording for modern OS floor. |
| Package.swift | Raises SPM platform minimums to iOS 15 / macOS 12 / tvOS 15 / watchOS 9. |
| CHANGELOG.md | Documents the availability cleanup, floor bump, and CI adjustments under “Next”. |
| ACKategoriesExample/Screens/UIControl blocks/UIControlBlocksViewController.swift | Removes dead iOS availability branch around systemGray6. |
| ACKategories.xcodeproj/project.pbxproj | Raises Xcode project deployment targets (including example target override). |
| .github/workflows/tests.yml | Pins Xcode 26.6 and updates simulator destinations via env vars. |
| .github/workflows/docbuild.yml | Pins Xcode 26.6 for DocC build workflow. |
| .github/workflows/build.yml | Pins Xcode 26.6 for Carthage/SPM build workflow. |
Review details
- Files reviewed: 38/38 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🟡 Changes recommended
The PR’s stated scope/notes (notably around UISearchBar.textField and thread-safety wording) conflict with the actual breaking changes and release-note wording, so the intent and documentation should be reconciled before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
CHANGELOG.md:16
- This line claims
UserDefault“no longer has racy state”, but the type still has no synchronization aroundwrappedValueaccess/mutation andCurrentValueSubject.sendisn’t documented as thread-safe. It’d be more accurate to say the initialization race from the associated-object accessor is removed, while the wrapper still isn’t intended for concurrent use.
- Back `UserDefault.subject` with a stored property seeded in `init` instead of `objc_getAssociatedObject`
- The associated-object accessor was a check-then-act race: two threads racing the first access each created a subject, the loser's instance was dropped from the association table and its subscribers were silently orphaned. The subject is now created exactly once, so `UserDefault` no longer has racy state
- Files reviewed: 34/34 changed files
- Comments generated: 3
- Review effort level: Lite
olejnjak
left a comment
There was a problem hiding this comment.
I am thinking that we might rewrite the UserDefault.wrappedValue getter so it reads the value directly from subject and setter so it sends new value to subject when write to UserDefaults succeeds, it would be less work when reading a value as there is no unwanted decoding.
We can discuss it here, but definitely it should not be part of this PR.
|
Agreed on the motivation, though it also means the wrapper stops seeing writes made to the key outside it, so I have taken just the setter half here and left the getter rewrite for you to call. |
`init` used to build the subject with the default value and then send the persisted one into it, because Swift forbids touching `self` before every stored property is assigned. Extract the getter's body into a static `storedValue(forKey:in:default:errorLogger:)` so `init` can read the store before `subject` exists, and drop the trailing `send`. The read logic now lives in exactly one place. Also rewrite the array-wrapping comment. It justified the workaround with an iOS 12 encoder limitation, two deployment-target floors out of date, so as written it invited the next reader to delete the workaround — which would break decoding of everything already persisted. It now states the prohibition and its cost instead. No behavior change: the existing tests pass unedited, including the one asserting the publisher replays the default followed by the new value.
The setter sent `newValue` into the subject unconditionally, outside the `do/catch` around encoding. When `JSONEncoder` threw, the error was logged, nothing reached `UserDefaults`, and subscribers were still told the value had changed — so `wrappedValue` and `$value` disagreed permanently, and the value subscribers held disappeared on the next launch. Not hypothetical: `JSONEncoder` rejects non-conforming floats by default, so any `Codable` model carrying a computed ratio can hit it. The new test covers exactly that, and asserts a later successful write is still published.
These were the only two files under `Sources/` with no reference in `project.pbxproj`. SPM globs the tree so package consumers have had them since 6.16.0, where the CHANGELOG announced both — but the Carthage xcframework shipped without them and no `xcodebuild` job ever compiled them for any platform. Added to the framework target only, unmodified. Verified: Debug builds for iOS, macOS, watchOS and tvOS simulators all succeed with no diagnostics citing either file, and `xcodebuild docbuild` succeeds clean. The DocC link fixes ride along because they surfaced only now, when the documentation build first saw this file. Environment values become plain code spans instead of symbol links — DocC files extensions of SwiftUI types under `SwiftUICore`, an internal module name not worth hardcoding — and the notes referencing `FitReadableContentWidth`/`FitLayoutMarginsWidth` are dropped, since neither type has ever existed here. Also corrects the 6.16.0 entry for `WithLayoutMargins`, whose link text said #150 while pointing at pull request 151.
cc45b57 to
dfdae2b
Compare
Follow-up to #158. Now that the floor is iOS 15 / macOS 12 / tvOS 15 / watchOS 9, a block of availability code is permanently true and can go.
Stacked on #158 — base is
xcode27, so this diff shows only the cleanup. GitHub retargets it tomainonce #158 merges.What changed
@availableattributes deleted across Networking, PushNotifications, ACKategoriesTesting, the SwiftUI extensions, Combine/Publisher helpers and the tests.if #availableruntime branches unwrapped, with their deadelsepaths removed.SwiftUILayoutGuides.swiftpreview gates aNavigationSplitViewat iOS 16, so@available(iOS 16.0, macOS 10.15, watchOS 6.0, tvOS 13.0, *)becomes@available(iOS 16.0, *). Deleting it outright would have broken the iOS 16 gate.Split into six commits by module, so it reads a piece at a time.
Highest version found among everything deleted: iOS 14.0, macOS 12.0, tvOS 14.0, watchOS 7.0 — all at or below the new floor. Note the
macOS 12.0one (extension URLSession: Network) sits exactly on the floor with zero headroom, becauseURLSession.data(for:)needs macOS 12.Deliberately untouched
FontModifier.swift'sif #available(iOS 26.0, ...)— a live backport of the nativelineHeight, not an expired gate.@available(*, deprecated:)/(*, unavailable)attribute.UISearchBar.textFieldkeeps its public member and its deprecation attribute; only the runtime check inside was unwrapped. Removing the shim would be a second breaking change on top of the floor bump.The one behavioural change, worth a look in review
UserDefault.subjectwas backed byobjc_getAssociatedObjectpurely because a stored property cannot carry limited availability — the comment in the file said exactly that. With the attribute gone the workaround has no reason to exist, so it is now alazystored property.lazyrather than eager is deliberate: the old getter built the subject on first access, seeding fromwrappedValueread at that moment. Eager initialisation would seed fromUserDefaultsat construction time instead, which is observable if the backing store changes in between.The caveat, stated precisely. The old check-then-set was already logically racy — two threads could each build a subject, one silently losing its subscribers — so that failure mode is not new. What is new:
objc_get/setAssociatedObjectare lock-protected by the ObjC runtime, so the storage access itself was safe, whereaslazy varcompiles to an unsynchronised read-modify-write. A concurrent first access is now a genuine data race, reportable under TSan.UserDefaulthas no synchronisation anywhere and neither form was usable concurrently, but the failure mode changed in kind, so it is documented in the source and in the CHANGELOG rather than glossed as a pure simplification. Happy to drop that commit if you would rather this PR stayed purely subtractive.Verification
Locally on Xcode 27.0 (27A5252f):
xcodebuild build× iOS / macOS / tvOS / watchOSxcodebuild testiOSxcodebuild testmacOSswift build/swift testswiftlintTest counts are identical to #158's, so nothing was silently skipped.
ACKategoriesResponderTestscould not be verified locally: the example-app test runner crashes at launch under the Xcode 27 beta simulator. It fails identically on #158's branch, which CI passes green, so this is an environment problem rather than a regression — CI covers that scheme.A coverage gap this PR walked into (pre-existing, not introduced here)
The riskiest edit in this PR — the iOS 16 trim — is compiled by nothing in CI.
SwiftUILayoutGuides.swifthas no reference inACKategories.xcodeproj/project.pbxproj, so noxcodebuildjob ever sees it, and its contents sit behind#if os(iOS)/#if DEBUG, so the SPM jobs running on the macOS host skip them too.I verified the trim by hand instead:
swiftc -typecheck -D DEBUGatarm64-apple-ios15.0-simulatoris clean, and lowering the attribute toiOS 15.0produceserror: 'NavigationSplitView' is only available in iOS 16.0 or newer— so iOS 16 is both necessary and sufficient, and no sibling declaration in that file needed the same treatment.Worth a separate PR:
SwiftUILayoutGuides.swiftandView+FrameSize.swiftare both missing from the Xcode project. That meansWithLayoutMargins,fitToReadableContentWidth()andmeasureLayoutMargins()(shipped in #151) plusView+FrameSize(#153) are absent from the Carthage xcframework entirely.CLAUDE.mddocuments the second file but not the first.