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

Returning Promise for the identify() to avoid race condition #200

Merged
merged 1 commit into from Jun 16, 2023

Conversation

zihejia
Copy link
Collaborator

@zihejia zihejia commented Jun 14, 2023

address #147

@zihejia zihejia added the bug Something isn't working label Jun 14, 2023
@@ -187,13 +187,24 @@ export class Mixpanel {
* Mixpanel using the same disinct_id will be considered associated with the
* same visitor/customer for retention and funnel reporting, so be sure that the given
* value is globally unique for each individual user you intend to track.
*
* @returns {Promise} A promise that resolves when the identify is successful.
* It does not return any value.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a breaking API change?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's still compatible with the old way of calling the api.

resolver resolve: RCTPromiseResolveBlock,
rejecter reject: RCTPromiseRejectBlock) -> Void {
resolver resolve: @escaping RCTPromiseResolveBlock,
rejecter reject: @escaping RCTPromiseRejectBlock) -> Void {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we also need to update the Android side?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

android is a sync call so it should be fine

Copy link
Collaborator

@jaredmixpanel jaredmixpanel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@zihejia zihejia added enhancement New feature or request and removed bug Something isn't working labels Jun 14, 2023
@zihejia zihejia merged commit cf1e6a9 into master Jun 16, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants