segmentio / typewriter

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

Support generating union types for enum properties in v8+ #301

Open andreiho opened 1 year ago

andreiho commented 1 year ago

In v7, when defining enum properties, the generated types would just be string, meaning there was no actual type-safety for enum properties.

In v8 with the quicktype upgrade, enum property types are now actual enums, which is great for type-safety, but it also makes upgrading an application with thousands of events a major headache, as all our event calls reference the properties as plain strings, like so analytics.userCreated({ role: 'admin' }) which would need to be changed to analytics.userCreated({ role: analytics.Role.Admin }).

This is extremely cumbersome to do for all events, and also completely unnecessary. It is a much better developer experience to specify strings as property values, as those match exactly what's defined in the tracking plan, and not some abstract enum name auto-generated by the library.

It would be great to add a configuration option to typewriter to optionally generate string unions instead of enums. See here an example: https://github.com/quicktype/quicktype/issues/1234#issuecomment-786132972

tonycrosby commented 8 months ago

Is there any follow-up on this issue? We're in the same position where we need to upgrade from v7 to v9 but so many of the properties in the v9 plan are enum types. The same problem above we also have, but also we have our own enum types that we've had defined in our codebase which now conflict with the enums defined in the Typewriter tracking plan.

We really need to be able to just have the plan generate the same string basic type so we don't need to do a large update / refactor throughout our codebase

dontsave commented 8 months ago

Also looking for a followup. Typewriter is great, but typescript enums are a bitter pill. Not only does this foist a gnarly refactor on our codebase, it also increases our bundle size. Enums are transpiled into literal js objects which get bundled along with all our other runtime JS, even though they only exist to enforce types at build time. Enums also force us to write new import statements every place we want to consume them. That's unnecessary overhead.

String unions don't require imports and are fully erased at compile time and would have no impact on our app beyond enforcing type safety. Would it be a substantial lift to expose an option for typewriter users to switch from enums to unions?