create event interface #7
Conversation
| @@ -108,7 +108,7 @@ dependencies { | |||
| // For < 0.71, this will be from the local maven repo | |||
| // For > 0.71, this will be replaced by `com.facebook.react:react-android:$version` by react gradle plugin | |||
| //noinspection GradleDynamicVersion | |||
| implementation "com.facebook.react:react-android:0.73.1" | |||
There was a problem hiding this comment.
I think we should keep a pinned version
| export interface Spec extends TurboModule { | ||
| createEvent(event: IEvent): void; | ||
| } | ||
| export interface KlaviyoSpec extends TurboModule, KlaviyoEventAPI {} |
There was a problem hiding this comment.
This may cause a turbo modules issue. I was about to suggest we extract this interface to its own file but I happened on this first which explains some naming conventions involved in turbo modules code gen. It mentions the filename, but doesn't say whether this interface has to be called Spec.
There was a problem hiding this comment.
Ah yea here we go https://dev.to/amazonappdev/a-guide-to-turbo-modules-in-react-native-5aa3
This interface type must be named Spec for a Turbo Module.
Maybe lets leave this file as-is from the code generator, and have a IKlaviyo in a separate file
There was a problem hiding this comment.
i mean sure the naming of the file can be Spec if thats required but i dont see a point in defining another interface that mirrors this spec interface
| properties?: Object; | ||
| type KlaviyoEventPropertyType = EventProperty | string; | ||
|
|
||
| export interface KlaviyoEventAPI { |
|
|
||
| interface IKlaviyo { |
There was a problem hiding this comment.
I think we'll wind up with IKlaviyo again for initialize, but we can add it back
There was a problem hiding this comment.
this can just implement Spec instead if thats the case, as long as its a single interface
| KlaviyoReactNativeSdk.createEvent(event); | ||
| export const Klaviyo: KlaviyoSpec = { | ||
| createEvent(name: EventType, properties?: Record<any, Object>): void { | ||
| this.createCustomEvent(name, properties); |
There was a problem hiding this comment.
does this need .toString() ?
No description provided.