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

Remove semaphores, copy properties, use .merging, don't includeLibInfo in init #191

Merged
merged 6 commits into from
Apr 29, 2023

Conversation

jaredmixpanel
Copy link
Collaborator

@jaredmixpanel jaredmixpanel commented Apr 28, 2023

The properties object passed to initialize() contains both the lib metadata (i.e. mp_lib and $lib_version) and the user specified super properties: https://github.com/mixpanel/mixpanel-react-native/blob/master/index.js#L67

That object is then passed to the native initialize method as the superProperties param to be registered prior to all event tracking (so there is no need to includeLibInfo when we process the properties: https://github.com/mixpanel/mixpanel-react-native/blob/master/ios/MixpanelReactNative.swift#L25

Additionally, make a copy of the properties dictionary to be passed to setAutomaticProperties. So that it is not the same value that is passed to processProperties and use native .merging() method to create a new dictionary instead of using custom mutating merge function.

Because the lib properties are registered as super properties upon initialization, we do not need to includeLibInfo when we call track or trackWithGroups, only people calls will need them.

@jaredmixpanel jaredmixpanel changed the title remove unnecessary call to set automatic properties during initialize Remove semaphores, copy properties, use .merging, don't includeLibInfo in init Apr 28, 2023
@jaredmixpanel jaredmixpanel added the bug Something isn't working label Apr 29, 2023
@jaredmixpanel jaredmixpanel merged commit 83309a4 into master Apr 29, 2023
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.

1 participant