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

Wrongly typed newTracker() function within the "@snowplow/browser-tracker" package #1167

Open
ghost opened this issue Mar 15, 2023 · 1 comment
Labels
type:defect Bugs or weaknesses. The issue has to contain steps to reproduce.

Comments

@ghost
Copy link

ghost commented Mar 15, 2023

Description
The exposed newTracker() function from the @snowplow/browser-tracker package returns BrowserTracker | null. The current interface definition hints to a return type of BrowserTracker only.

This is misleading and does not represent the current implementation correctly.

To Reproduce

  • Add a new tracker by calling newTracker() with a specified tracker identifier e.g. "my-test"
  • Add another tracker by calling newTracker() with the same trackerId
  • The function should have returned null

Expected behavior
I would expect that either the types will tell that the function could possibly return null.
Or that the function would return the already initialized tracker, even though this could be misleading due to the function naming "new".

"Screenshots"
@snowplow/browser-tracker: newTracker()
https://github.com/snowplow/snowplow-javascript-tracker/blob/master/trackers/browser-tracker/src/index.ts#L59

@snowplow/browser-tracker-core: addTracker()
https://github1s.com/snowplow/snowplow-javascript-tracker/blob/master/libraries/browser-tracker-core/src/snowplow.ts#L88

Additional context
We implemented the browser tracker within a React ecosystem and run into an issue where we called newTracker() twice due to an unexpected rerender. We used the returned tracker to add further tracker configurations; but were not able to do so after the second render / initialization.
We prevent calling the newTracker() function twice; and realized while debugging that the provided interface / type definitions are currently wrong.

@igneel64
Copy link
Contributor

Merged in v4. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:defect Bugs or weaknesses. The issue has to contain steps to reproduce.
Projects
None yet
Development

No branches or pull requests

1 participant