snowplow / snowplow-javascript-tracker

Snowplow event tracker for client-side and server-side JavaScript. Add analytics to your websites, web apps and servers.
http://snowplowanalytics.com
BSD 3-Clause "New" or "Revised" License
543 stars 220 forks source link

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

Open ghost opened 1 year ago

ghost commented 1 year ago

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

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 commented 1 year ago

Merged in v4. :)