segmentio / typewriter

Type safety + intellisense for your Segment analytics
https://segment.com/docs/protocols/typewriter/
MIT License
228 stars 53 forks source link

React Native client only generates types for events that have properties #284

Open stevehanson opened 1 year ago

stevehanson commented 1 year ago

Thank you for adding React Native support for this library! I am starting on integrating Segment into my React Native project and am using analytics-react-native. I just added Typewriter v8, and it is working great, except it only generates types for events that have custom properties. Here are my observations for events that have no custom properties:

I've verified that if I go into Segment and add even a single property to the event and then rerun npx typewriter -u, or if I just manually add properties to plan.json, the events are added to segment.tsx as expected.

Proposed solution

Ideally, if I had an event called My Custom Event that takes no properties, something like the following would be generated in segment.tsx:

// generated segment.tsx

export type MyCustomEvent = undefined
// alternatively: export type MyCustomEvent = Record<string, never> | undefined

...
// this part is the same as for other events
extendedClient.myCustomEvent = (message: MyCustomEvent) => {
  client.track('My Custom Event', message as unknown as JsonMap)
}

The second argument of client.track is already typed to accept undefined, so this doesn't result in any errors with the client or TypeScript.

Here is my typewriter.yml:

client:
  language: typescript
  sdk: analytics-react-native
  languageOptions: {}
trackingPlans:
  - id: my_tracking_plan_id
    path: src/__generated__

I'd be happy to take a pass at fixing this and opening a PR but might need a bit of help getting pointed in the right direction. Thanks again for this library! 🙂

Steps to reproduce

Versions

mattvanvoorst-contentful commented 1 year ago

Just wanted to add to this, that the exact same happens for the JS client. Unless at least one custom property is defined in a tracking plan, Typewriter won't generate the typed method for it.

andreiho commented 1 year ago

@oscb Tagging for visibility. This is blocking us as well from migrating to v8. I tried going through to code in an attempt to fix it and open a PR, but couldn't figure out where exactly it goes wrong. My hunch is that because quicktype doesn't actually generate any types due to properties being empty, no actual client code is generated either.

tmf-chris commented 1 year ago

Also have this issue; is blocking me using the client

oscb commented 1 year ago

Sorry for the lack of responses, we are a little short-staffed to keep up with Typewriter lately.

I'm looking into bringing more people to maintain this project so I cannot give an ETA at this moment on when this will get fixed, but this is the first priority. As mentioned it is mostly due to quicktype, it doesn't generate types for empty schemas.

I'll pin this and keep the thread updated when we have a clear path forward.

andreiho commented 1 year ago

Hey guys. Bumping this again. Unfortunately due to compliance reasons and dependabot vulnerabilities with the got package, we have to upgrade from v7, but I just can't see any easy way forward without addressing some of the issues inadvertently introduced in v8.

This issue where types/methods are not generated for events without properties is the biggest, a lot of our events are like this. At this point I'm close to just going and updating the tracking plan to add a dummy optional property to those events just to make typewriter generate the code. But it'd be great if I didn't have to do that as when this is eventually fixed, it would be again super cumbersome to clean things up.

Another issue which blocks us is https://github.com/segmentio/typewriter/issues/301, where enum properties are now generated as enums, thus requiring tons of code changes which is extremely time-consuming.

These 2 issues would not be such a big deal had we not had 600+ events, of which most are either without any properties, or with some properties defined as enums. So I would like to know, whether there's any chance these issues could be resolved in the near future, so I don't waste time working around them? Alternatively, what would it take to patch v7 and upgrade got to a secure version, in order to get rid of the vulnerability?


UPDATE: I created a branch for patching vulnerable dependencies in v7: https://github.com/segmentio/typewriter/pull/302

bruno-azenha commented 1 year ago

First of all, thank you so much for all the work that has been put on Typewriter. We love the guarantees that it gives us around our event tracking.

We're using the JS integration and also being impacted by this. Our events that have no properties never get their functions generated in the segment.js. Please let us know if there are any updates or timelines on this.

Thanks a lot!