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

Add new tracker for React Native #1371

Open
wants to merge 16 commits into
base: wip/react-native
Choose a base branch
from

Conversation

matus-tomlein
Copy link
Contributor

@matus-tomlein matus-tomlein commented Nov 5, 2024

Adds the @snowplow/react-native-tracker package to the monorepo.

This aims to move the React Native tracker to instead of using the iOS and Android trackers as the base for the React Native tracker, to use the JavaScript tracker. The motivation for this is to:

  1. support Expo Go (managed workflow) and React Native for Web
  2. better support React Native new architecture without the interoperability layer
  3. make the tracker easier to maintain going forward (simplifies the project)
  4. be able to use JS tracker plugins (or some of them) in the RN tracker and vice versa

The package will continue with the @snowplow/react-native-tracker starting from version 4.1 (or later). We decided for this as opposed to introducing a new package to make it more clear long term that this package replaces the old RN tracker and avoid having two RN packages. We will need to skip v3 (RN tracker is now on v2) in order to sync with the JS tracker versions.

This PR implements the core of the tracker without additional features (such as session, lifecycle tracking) which will be introduced in follow up PRs. It tries to maintain the API from the RN tracker but for the createTracker method, this was not possible and I think it's better to follow the newTracker convention in the JS tracker. So the newTracker method signature is a breaking change.

I have deployed the package under the version 4.0.1-dev.1 and added a very simple example app in the examples repo using it: snowplow-incubator/snowplow-javascript-tracker-examples#60 (we'll improve it over time)

@matus-tomlein matus-tomlein changed the base branch from master to wip/react-native November 5, 2024 09:50
@matus-tomlein matus-tomlein marked this pull request as draft November 5, 2024 09:52
Copy link

bundlemon bot commented Nov 5, 2024

BundleMon

Files added (6)
Status Path Size Limits
trackers/javascript-tracker/dist/sp.js
+24.49KB 30KB / +10%
libraries/browser-tracker-core/dist/index.mod
ule.js
+23.44KB 25KB / +10%
libraries/tracker-core/dist/index.module.js
+19.48KB 20KB / +10%
trackers/browser-tracker/dist/index.umd.min.j
s
+17.4KB 20KB / +10%
trackers/javascript-tracker/dist/sp.lite.js
+17.33KB 20KB / +10%
trackers/browser-tracker/dist/index.module.js
+3.49KB 5KB / +10%

Total files change +105.62KB 0%

Final result: ✅

View report in BundleMon website ➡️


Current branch size history

@matus-tomlein matus-tomlein marked this pull request as ready for review November 7, 2024 10:36
trackers/react-native-tracker/README.md Outdated Show resolved Hide resolved
trackers/react-native-tracker/src/types.ts Outdated Show resolved Hide resolved
trackers/react-native-tracker/src/subject.ts Outdated Show resolved Hide resolved
trackers/react-native-tracker/src/subject.ts Outdated Show resolved Hide resolved
trackers/react-native-tracker/src/subject.ts Show resolved Hide resolved
trackers/react-native-tracker/src/subject.ts Show resolved Hide resolved
trackers/react-native-tracker/src/events.ts Show resolved Hide resolved
@matus-tomlein matus-tomlein force-pushed the issue/create_react_native_tracker branch from 7ef3a0b to 9094cb3 Compare November 11, 2024 07:48

Choose a reason for hiding this comment

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

Am I correct that this gets rid of the removeTracker method?

I know this doesn't have the native tie-in plugins yet, but will that be something that's safe to get rid of? Thinking of the things like application_foreground that might get duplicated if you can't remove a tracker.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for looking into this and the feedback!

That's correct, we don't have the removeTracker function here. We can look into it adding it though, it's a good feedback.

But I'm not sure I understand the relation to the application_foreground event. The tracker shouldn't be removed when the app goes to background/foreground. In order to calculate session metrics correctly, the same tracker instance should be maintained. So normally you would only create a new tracker on app start and then keep the tracker instance for the duration of the app lifetime.

Copy link

@Mookiies Mookiies Nov 12, 2024

Choose a reason for hiding this comment

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

As an example we have a legacy version of the app and a new part, each with their respective trackers both track the application lifecycle. When we swap between the two we removeTracker. If we didn't I think we'd get duplicate events sent, which is why I mention application_foreground.

So the case I'm thinking about is: if you setup a plugin to automatically send events for you, how to you get it to stop?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, I see, that makes sense! Will think about how we can introduce the removeTracker option here or at least have a way to pause the tracker.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants