refactor(core): move global environment behind KlaviyoEnv holder for Swift 6#568
Draft
refactor(core): move global environment behind KlaviyoEnv holder for Swift 6#568
Conversation
…Swift 6
Step 1.1 of the Swift 6 strict-concurrency migration. Replaces the public
mutable global `environment` with a typed holder `KlaviyoEnv.current`,
gated behind `@_spi(KlaviyoPrivate)`. The original `environment` symbol
remains as a deprecated SPI shim for source compatibility while call
sites migrate (chunk 1.2.*).
- New canonical entrypoint: `KlaviyoEnv.current` (SPI-gated, marked
`nonisolated(unsafe)` per the documented Apple stopgap for nonisolated
global shared mutable state under -strict-concurrency=complete).
- Deprecated shim: `environment` is now `@available(*, deprecated, renamed:
"KlaviyoEnv.current")` and SPI-gated.
- All cross-module imports of KlaviyoCore in this repo flipped to
`@_spi(KlaviyoPrivate) import KlaviyoCore` (40 files).
- Three public inits in KlaviyoCore (`KlaviyoAPI.init(send:)`,
`KlaviyoRequest.init(id:endpoint:)`, `AppLifeCycleEvents.init`)
reference `environment` in their default-argument values, which the
compiler rejects for non-SPI public ABI; gated those inits SPI to
match. All current callers already have SPI access via the import flip.
- Dropped `@usableFromInline` on `runtimeWarn` (no `@inlinable` callers
in KlaviyoCore; was inadvertently exposing its default args to the
cross-module surface).
Race surface is unchanged — labeled, not eliminated. Real isolation
lands when the closures inside `KlaviyoEnvironment` get rehomed into
per-feature containers in later chunks.
Out of scope (separate chunks):
- Migrating `environment.X` call sites to `KlaviyoEnv.current.X`.
- The `networkSession` global at KlaviyoEnvironment.swift:300.
- Sibling test apps (klaviyo-{ios,swift}-test-app).
`nonisolated(unsafe)` was added in Swift 5.10 (Xcode 15.3). The package declares swift-tools-version 5.9 and CI builds against Xcode 15.2, so the syntax fails to parse there. Gate it with a compiler check; the Swift 5.9 path is plain `static var current`, which is fine because strict concurrency isn't enabled on that toolchain anyway. Also picks up SwiftFormat's reformatting of the holder declaration (modifier order: `public nonisolated(unsafe) static var`).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Step 1.1 of the Swift 6 strict-concurrency migration. Replaces the public mutable global
environmentatSources/KlaviyoCore/KlaviyoEnvironment.swift:13with a typed holderKlaviyoEnv.current, gated behind@_spi(KlaviyoPrivate). The originalenvironmentsymbol remains as a deprecated SPI shim for source compatibility while call sites migrate (chunk 1.2.*).Due Diligence
(No new tests — this is a pure refactor with zero behavior change. Existing test suite passes.)
Release/Versioning Considerations
PatchContains internal changes or backwards-compatible bug fixes.MinorContains changes to the public API.MajorContains breaking changes.The new
KlaviyoEnvsymbol and the deprecatedenvironmentshim are both@_spi(KlaviyoPrivate)— they can only be reached via@_spi(KlaviyoPrivate) import KlaviyoCore. The pre-existingenvironmentglobal was plainpublic; tightening it to SPI is a deliberate access narrowing. All call sites in this repo (and our sibling test apps, which already use the SPI import) are unaffected.Changelog / Code Overview
New canonical entrypoint:
nonisolated(unsafe)is the documented Apple stopgap for nonisolated global shared mutable state under-strict-concurrency=complete. Race surface is unchanged — labeled, not eliminated. Real isolation lands when the closures insideKlaviyoEnvironmentget rehomed into per-feature containers in later chunks.Deprecated source-compat shim:
Knock-on changes required by the SPI gating:
All cross-module imports of
KlaviyoCoreflipped to@_spi(KlaviyoPrivate) import KlaviyoCore(40 source + test files). Mechanical change; no call-site edits.Three public inits in KlaviyoCore reference
environmentin their default-argument values, which the compiler rejects for non-SPI public ABI (default args become part of the public interface):KlaviyoAPI.init(send:)KlaviyoRequest.init(id:endpoint:)AppLifeCycleEvents.init(lifeCycleEvents:)These are gated
@_spi(KlaviyoPrivate)to match. All current callers already have SPI access via the import flip; customer-facing surface (KlaviyoSDK,Klaviyo.shared, etc.) is unchanged.Dropped
@usableFromInlineonruntimeWarninLoggerClient.swift— no@inlinablecallers exist in KlaviyoCore, and the annotation was inadvertently exposing its default args to the cross-module surface.What chunk 1.2. will do (out of scope here):*
environment.Xcall sites toKlaviyoEnv.current.X(deprecation warnings flag them today).environmentshim once call sites are migrated.Out of scope for the broader 1.x effort, separate chunks:
networkSessionglobal atKlaviyoEnvironment.swift:300.klaviyo-{ios,swift}-test-app(no changes needed — they already use@_spi(KlaviyoPrivate) import KlaviyoCore).Test Plan
xcodebuild build -scheme klaviyo-swift-sdk-Package -destination "platform=iOS Simulator,name=iPhone 17 Pro"— succeeds, with the expected deprecation warnings on existingenvironment.Xcall sites (to be cleared in chunk 1.2.*).make test-library— full test suite passes.'environment' is deprecated: renamed to 'KlaviyoEnv.current') on all in-repo call sites.Related Issues/Tickets
Swift 6 strict concurrency initiative.