Skip to content
This repository has been archived by the owner on Jan 18, 2024. It is now read-only.

[xdl] fix analytics for expo start #2357

Merged
merged 2 commits into from
Jul 15, 2020
Merged

Conversation

esamelson
Copy link
Contributor

This PR fixes an issue with expo-cli@3.22.x where the number of logged Start Project and Serve Manifest events are significantly lower than expected.

Paired with @animeeples to investigate and we found the root cause to be that by the time the Serve Manifest event should be sent, the _userId field in Analytics.ts is not set, meaning the following statement evaluates to false and the event is never sent to segment:

if (_segmentNodeInstance && _userId) {

Following the chain of methods that set the _userId field, it looks like this issue was introduced by 553eb01 which eliminated a call to UserManager.ensureLoggedInAsync() (553eb01#diff-bf9f0db58154788a372e25a5c3898adbL2009). This method will set the _userId field on Analytics.ts if it is not already set.

This PR restores the previous functionality by adding a call to UserManager.ensureLoggedInAsync() in the analogous location, in order to keep these two metrics the same as before and preserve comparisons to historical data.

Looking further ahead, it would be good to

  • refactor this code to be more intentional about when we set the _userId field in Analytics, so it's not such a hidden side effect
  • reevaluate whether or not we want to track any/all segment events for users who are not logged in

But this PR is a first step/quick fix to get the metrics back to the place they were before 3.22.0.

packages/xdl/src/Project.ts Show resolved Hide resolved
@brentvatne brentvatne merged commit 81cb78b into master Jul 15, 2020
@brentvatne brentvatne deleted the @eric/fix-start-analytics branch July 15, 2020 02:43
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants