snowplow / snowplow-react-native-tracker

Snowplow event tracker for react-native apps
Apache License 2.0
31 stars 18 forks source link

Add generic type for SelfDescribing event data #156

Closed henbrand closed 2 years ago

henbrand commented 2 years ago

Hi! 👋

For additional type safety I wanted to be able to type the data object for trackSelfDescribingEvent, currently I have patched the library to allow me to do so. Let me know if you'd like me to open a pull request!

Here is the diff that solved my problem:

diff --git a/node_modules/@snowplow/react-native-tracker/dist/index.d.ts b/node_modules/@snowplow/react-native-tracker/dist/index.d.ts
index d326b4b..c56a6dd 100644
--- a/node_modules/@snowplow/react-native-tracker/dist/index.d.ts
+++ b/node_modules/@snowplow/react-native-tracker/dist/index.d.ts
@@ -29,7 +29,7 @@ declare type ScreenSize = [number, number];
 /**
  * SelfDescribing type
  */
-declare type SelfDescribing = {
+declare interface SelfDescribing<TData extends object> {
     /**
      * Schema
      */
@@ -37,7 +37,7 @@ declare type SelfDescribing = {
     /**
      * Data
      */
-    data: Record<string, unknown>;
+    data: TData;
 };
 /**
  * EventContext type
@@ -547,7 +547,7 @@ declare type ReactNativeTracker = {
      * @param argmap - The self-describing event properties
      * @param contexts - The array of event contexts
      */
-    readonly trackSelfDescribingEvent: (argmap: SelfDescribing, contexts?: EventContext[]) => Promise<void>;
+    readonly trackSelfDescribingEvent: <TData>(argmap: SelfDescribing<TData>, contexts?: EventContext[]) => Promise<void>;
     /**
      * Tracks a screen-view event
      *
matus-tomlein commented 2 years ago

Thanks for the suggestion @henbrand, that's really cool and a good step towards type safety!

It would be awesome if you could make a PR for this. I just have one concern regarding the diff that you sent above – the TData in your code extends object, wouldn't it better to have it extend Record<string, unknown> as previously required by the SelfDescribing type? Perhaps we could take inspiration from the way it's implemented in the Snowplow JavaScript tracker, see here.

henbrand commented 2 years ago

@matus-tomlein - could you give me correct permissions (Direct Access should work) to open a PR?

matus-tomlein commented 2 years ago

@henbrand Can you please fork the repository, make the change and submit a PR to this repository? That should work without needing any extra permissions. Let me know if there is any problem with it.