Adding deferred Automatic event initialization for SwiftUI issue #682
Conversation
Support enabling automatic events after initialization to avoid UIKit conflicts during app startup. Solves accent color loading issues in SwiftUI apps that initialize Mixpanel early when enabling automaticEvents from init.
Wrapped access to _trackAutomaticEventsEnabled in read/write locks to ensure thread safety when getting or setting the property.
There was a problem hiding this comment.
Pull request overview
This PR adds support for deferred automatic event initialization to solve SwiftUI early initialization issues. The main purpose is to allow apps to initialize Mixpanel with automatic events disabled and enable them later once the UI is ready, avoiding accent color loading conflicts during app startup.
Key changes:
- Converted
trackAutomaticEventsEnabledfrom a stored property to a computed property with getter/setter that can trigger re-initialization - Added
enableAutomaticEvents()convenience method for explicit enabling - Implemented comprehensive test coverage for deferred initialization scenarios
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| Sources/MixpanelInstance.swift | Converted trackAutomaticEventsEnabled to computed property with re-initialization logic in setter; added enableAutomaticEvents() method |
| MixpanelDemo/MixpanelDemoTests/MixpanelAutomaticEventsTests.swift | Added 5 new tests covering deferred initialization, idempotency, session tracking, and enable/disable scenarios |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func testMultipleDeferredEnablesAreIdempotent() { | ||
| let testToken = randomId() | ||
| let testMixpanel = Mixpanel.initialize( | ||
| token: testToken, trackAutomaticEvents: false, flushInterval: 60) | ||
| testMixpanel.minimumSessionDuration = 0 | ||
|
|
||
| // Enable multiple times | ||
| testMixpanel.trackAutomaticEventsEnabled = true | ||
| waitForTrackingQueue(testMixpanel) | ||
| let countAfterFirstEnable = eventQueue(token: testMixpanel.apiToken).count | ||
|
|
||
| testMixpanel.trackAutomaticEventsEnabled = true | ||
| waitForTrackingQueue(testMixpanel) | ||
| let countAfterSecondEnable = eventQueue(token: testMixpanel.apiToken).count | ||
|
|
||
| testMixpanel.enableAutomaticEvents() | ||
| waitForTrackingQueue(testMixpanel) | ||
| let countAfterThirdEnable = eventQueue(token: testMixpanel.apiToken).count | ||
|
|
||
| // The first event count should remain the same despite multiple enables | ||
| XCTAssertEqual( | ||
| countAfterFirstEnable, countAfterSecondEnable, | ||
| "Second enable should not duplicate first open event") | ||
| XCTAssertEqual( | ||
| countAfterSecondEnable, countAfterThirdEnable, | ||
| "Third enable should not duplicate first open event") | ||
|
|
||
| removeDBfile(testMixpanel.apiToken) | ||
| } |
There was a problem hiding this comment.
This test claims to verify idempotent behavior when enabling automatic events multiple times, but it only checks that the event count stays the same. It doesn't verify that NotificationCenter observers or SKPaymentQueue observers aren't duplicated.
Consider adding assertions to verify that notification handlers don't fire multiple times after multiple enables. For example, trigger a notification and verify the session event is only tracked once, not multiple times.
| func testDisablingAndReEnablingAutomaticEvents() { | ||
| let testToken = randomId() | ||
| let testMixpanel = Mixpanel.initialize( | ||
| token: testToken, trackAutomaticEvents: true, flushInterval: 60) | ||
| testMixpanel.minimumSessionDuration = 0 | ||
|
|
||
| waitForTrackingQueue(testMixpanel) | ||
| let initialEventCount = eventQueue(token: testMixpanel.apiToken).count | ||
| XCTAssertTrue( | ||
| initialEventCount > 0, "Should have first open event initially") | ||
|
|
||
| // Disable automatic events | ||
| testMixpanel.trackAutomaticEventsEnabled = false | ||
| XCTAssertFalse( | ||
| testMixpanel.trackAutomaticEventsEnabled, | ||
| "Should be disabled after setting to false") | ||
|
|
||
| // Re-enable automatic events | ||
| testMixpanel.trackAutomaticEventsEnabled = true | ||
| XCTAssertTrue( | ||
| testMixpanel.trackAutomaticEventsEnabled, | ||
| "Should be re-enabled after setting to true") | ||
|
|
||
| removeDBfile(testMixpanel.apiToken) | ||
| } |
There was a problem hiding this comment.
This test only verifies that the property value changes when disabling/re-enabling automatic events, but doesn't test that automatic events actually stop being tracked when disabled. The implementation doesn't remove notification observers when trackAutomaticEventsEnabled is set to false, so events would continue to be tracked.
Consider adding assertions to verify that after disabling, no session events are tracked when notifications fire. For example:
// After disabling
testMixpanel.automaticEvents.perform(
#selector(AutomaticEvents.appWillResignActive(_:)),
with: Notification(name: Notification.Name(rawValue: "test")))
waitForTrackingQueue(testMixpanel)
let eventCountAfterDisable = eventQueue(token: testMixpanel.apiToken).count
XCTAssertEqual(eventCountAfterDisable, initialEventCount, "No new events should be tracked when disabled")| open var showNetworkActivityIndicator = true | ||
|
|
||
| private var _trackAutomaticEventsEnabled: Bool | ||
| /// This allows enabling or disabling collecting common mobile events, |
There was a problem hiding this comment.
The comment on line 118 is incomplete. It should describe what the property does.
Consider completing the documentation:
/// This allows enabling or disabling collecting common mobile events.
/// When set to true, automatically tracks events like first open, app updated, and session events.
/// Setting this to true after initialization will initialize automatic event tracking.| } | ||
| #if os(iOS) || os(tvOS) || os(visionOS) | ||
| if newValue && !MixpanelInstance.isiOSAppExtension() { | ||
| // Re-initialize automatic event tracking if being enabled | ||
| automaticEvents.delegate = self | ||
| automaticEvents.initializeEvents(instanceName: self.name) | ||
| } | ||
| #endif |
There was a problem hiding this comment.
The re-initialization logic in the setter has a race condition. The readWriteLock.write block only protects the assignment to _trackAutomaticEventsEnabled, but the subsequent calls to automaticEvents.delegate = self and automaticEvents.initializeEvents(instanceName: self.name) happen outside the lock. This means multiple threads could concurrently execute the re-initialization code if they both read the new value as true before the write lock is released.
Consider moving the re-initialization logic inside the write lock:
readWriteLock.write {
_trackAutomaticEventsEnabled = newValue
#if os(iOS) || os(tvOS) || os(visionOS)
if newValue && !MixpanelInstance.isiOSAppExtension() {
automaticEvents.delegate = self
automaticEvents.initializeEvents(instanceName: self.name)
}
#endif
}| } | |
| #if os(iOS) || os(tvOS) || os(visionOS) | |
| if newValue && !MixpanelInstance.isiOSAppExtension() { | |
| // Re-initialize automatic event tracking if being enabled | |
| automaticEvents.delegate = self | |
| automaticEvents.initializeEvents(instanceName: self.name) | |
| } | |
| #endif | |
| #if os(iOS) || os(tvOS) || os(visionOS) | |
| if newValue && !MixpanelInstance.isiOSAppExtension() { | |
| // Re-initialize automatic event tracking if being enabled | |
| automaticEvents.delegate = self | |
| automaticEvents.initializeEvents(instanceName: self.name) | |
| } | |
| #endif | |
| } |
| // Re-initialize automatic event tracking if being enabled | ||
| automaticEvents.delegate = self | ||
| automaticEvents.initializeEvents(instanceName: self.name) | ||
| } |
There was a problem hiding this comment.
Calling initializeEvents() multiple times will register duplicate NotificationCenter observers and add multiple SKPaymentQueue observers. The AutomaticEvents.initializeEvents() method (lines 84-96 in AutomaticEvents.swift) calls addObserver without first removing existing observers, which will cause notification handlers to fire multiple times.
Consider adding a guard to prevent re-initialization, or removing observers before re-adding them. For example:
if newValue && !MixpanelInstance.isiOSAppExtension() {
if !automaticEvents.hasAddedObserver {
automaticEvents.delegate = self
automaticEvents.initializeEvents(instanceName: self.name)
}
}| // Re-initialize automatic event tracking if being enabled | |
| automaticEvents.delegate = self | |
| automaticEvents.initializeEvents(instanceName: self.name) | |
| } | |
| if !automaticEvents.hasAddedObserver { | |
| automaticEvents.delegate = self | |
| automaticEvents.initializeEvents(instanceName: self.name) | |
| } |
| if newValue && !MixpanelInstance.isiOSAppExtension() { | ||
| // Re-initialize automatic event tracking if being enabled | ||
| automaticEvents.delegate = self | ||
| automaticEvents.initializeEvents(instanceName: self.name) |
There was a problem hiding this comment.
The setter does not handle the case when trackAutomaticEventsEnabled is set to false. There is no logic to stop tracking automatic events, remove notification observers, or remove the SKPaymentQueue observer. This means once automatic events are enabled, they cannot be properly disabled.
| automaticEvents.initializeEvents(instanceName: self.name) | |
| automaticEvents.initializeEvents(instanceName: self.name) | |
| } else if !newValue { | |
| // Disable automatic event tracking and remove observers | |
| automaticEvents.stopEvents() | |
| automaticEvents.delegate = nil |
|
Hello @jaredmixpanel and @santigracia, is there any update on this PR? I'm interested in this behaviour to fix the SwiftUI accent color issue due to UIKit (see this issue). Thank you. Axel |
|
Hello @santigracia @jaredmixpanel, I just tested the branch attached to this PR and from my tests in a real app, this is still not enough so the Mixpanel SDK does not break my app accentColor defined in my assets catalog. If I setup the SDK with trackAutomaticEvents set to false in my SwiftUI App init, I still have the issue because of Reachability and other observers that are setup during the SDK init. I observed that there are other places that needs to be modified to properly fix this bug. The accentColor is not messed up if I uncomment:
When I uncomment all these parts of the SDK, I can get my accent color in my app with trackAutomaticEvents enabled or disabled. Can you please update your SDK? Because right now, even with trackAutomaticEvents, it messes with the app accent color. Thank you, |
|
For your information, I spent some time again to really understand the issue. What I found is that the SDK accesses the UIScreen.main.bounds.size from the AutomaticProperties.properties that are used early by:
I suspect it's called too early and can lead to a race condition when the SDK is initialised. Example: for a totally new project where the accent color is set to orange in the assets catalog. If the app init only contains the Mixpanel.initialize call, then it doesn't break the accentColor. 
If I access a value stored in a UserDefaults.standard, there is no issue in a light App init. 
But if I access a value stored in a UserDefaults(suiteName:), the bug starts appearing (so I suspect it's also the case as soon as another SDK also does that). 
So I think it's actually not related to listening to UIKit notifications as we initially thought but instead because of accessing a property too early in a situation where a race condition with other code (like the UserDefaults). Disabling only the setCurrentRadio or MixpanelInstance.reachability is not sufficient if we track an event early in the App init (as the Mixpanel track method will access this AutomaticProperties.properties property). I'm not quite sure how this bug can be 100% fixed but in the meantime, I'll use a version of the SDK where I comment the line that accesses the UIScreen.main.bounds.size in the AutomaticProperties. And I could still send these two values |
2 participants
Support enabling automatic events after initialization to avoid UIKit conflicts during app startup. Solves accent color loading issues in SwiftUI apps that initialize Mixpanel early when enabling automaticEvents from init.