Fix: Use fresh super properties from SDK instead of stale static cache #328
Conversation
Super properties passed to init() could not be updated after initialization in React Native with useNative=true. Modified appendLibraryProperties() to fetch fresh values from MixpanelAPI instance instead of using a stale static cache.
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a bug where super properties passed to init() were not updatable after initialization in React Native with useNative=true. The fix changes appendLibraryProperties() to fetch fresh values directly from the MixpanelAPI SDK instance instead of relying on a stale static cache.
Key Changes:
- Modified
appendLibraryProperties()to accept aMixpanelAPIinstance parameter and fetch fresh super properties from the SDK - Updated all callers of
appendLibraryProperties()to pass the MixpanelAPI instance
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| MixpanelReactNativeModule.java | Updated all three call sites (track, set, setOnce) to pass the MixpanelAPI instance to appendLibraryProperties() |
| AutomaticProperties.java | Refactored appendLibraryProperties() to accept MixpanelAPI instance and fetch fresh super properties from SDK instead of using static cache |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
android/src/main/java/com/mixpanel/reactnative/AutomaticProperties.java
Outdated
Show resolved
Hide resolved
android/src/main/java/com/mixpanel/reactnative/AutomaticProperties.java
Outdated
Show resolved
Hide resolved
(from copilot) The documentation mentions 'library properties' but the implementation only appends super properties from the SDK.
… since the parameter is passed by value fixing Creating a new JSONObject when properties is null has no effect since the parameter is passed by value mixpanel#328 (comment)
fixing extra closing parenthesis
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| properties.put(key, sAutomaticProperties.get(key)); | ||
| public static void appendLibraryProperties(MixpanelAPI instance, JSONObject properties) throws JSONException { | ||
| // Get fresh super properties from the Android SDK (not from stale static cache) | ||
| if (instance != null && properties != null) { |
There was a problem hiding this comment.
The method no longer handles null properties by initializing a new JSONObject. Callers must now ensure properties is non-null before calling this method, but the documentation doesn't clearly state this requirement. Consider either restoring the null-safe behavior or explicitly documenting that properties must not be null as a precondition.
| if (instance != null && properties != null) { | |
| if (instance != null) { | |
| if (properties == null) { | |
| properties = new JSONObject(); | |
| } |
1 participant
Super properties passed to init() could not be updated after initialization in React Native with useNative=true. Modified appendLibraryProperties() to fetch fresh values from MixpanelAPI instance instead of using a stale static cache.