Repository navigation
feat(events)!: Add UnknownEvent to the Event union - #1734
gjtorikian wants to merge 1 commit into
Conversation
`UnknownEvent` was declared but not part of `Event`, and `deserializeEvent` cast its default branch to `Event`, so an exhaustive `switch` over `event.event` with an `assertNever` default compiled clean and threw at runtime the first time the API shipped an event type the SDK did not know. The `Event` union always trails the API, so this eventually hit every such consumer (#864, #1360, #1362, #1453, #1685). Add `UnknownEvent` to `Event` and a parallel `UnknownEventResponse` to `EventResponse`, exposing the known members as `KnownEvent` and `KnownEventResponse`. `EventName` stays the union of known names. `UnknownEvent.data` is typed `never`. TypeScript keeps a union member whose discriminant is `string` in every narrowed branch, so any other type would widen `event.data` in known branches (`User` would become `User | Record<string, unknown>`); `never` is absorbed and leaves known payloads intact. The raw payload is still passed through at runtime and can be read by annotating it: `const payload: Record<string, unknown> = event.data`. The deserializer's switch now lives in `deserializeKnownEvent`, typed against `KnownEvent`, because object literals with a union-typed `event` (the grouped cases) are otherwise also matched against `UnknownEvent`. BREAKING CHANGE: `Event['event']` is now `string` instead of the literal union; use `EventName` or `KnownEvent['event']` for the literal union. A `default` branch of a `switch` over `event.event` is no longer narrowed, so an `assertNever(event)` there fails to compile, which is the intended signal. Object literals typed as `Event` whose `event` is a union of literals need an explicit known event type. Fixes #1696
[Medium impact] The change reaches event-list and webhook consumers of the shared event path. The PR appears safe to merge, with a non-blocking improvement needed in the known-payload type test. Findings
Fix with agent prompt### Issue 1
src/events/events.spec.ts:1760-1761
The known-payload check does not prove that `email` remains a `string`: template interpolation also accepts `unknown`. If `event.data.email` becomes `unknown`, this assertion still compiles, leaving callers' typed reads unprotected. Assign it to a `string` variable, or call a string method, before returning it so the test catches the widening it promises to prevent.
```suggestion
// The known payload type is intact: `data` is `User`, not a union.
return `user:${event.data.email.toLowerCase()}`;
```
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.SummaryAdds unknown event members to
Acknowledged by Reviews (1) · Last reviewed commit: "feat(events)!: Add UnknownEvent to the E..." · Reviewed by Greptile |
| // The known payload type is intact: `data` is `User`, not a union. | ||
| return `user:${event.data.email}`; |
There was a problem hiding this comment.
Payload type check misses widening
The known-payload check does not prove that email remains a string: template interpolation also accepts unknown. If event.data.email becomes unknown, this assertion still compiles, leaving callers' typed reads unprotected. Assign it to a string variable, or call a string method, before returning it so the test catches the widening it promises to prevent.
| // The known payload type is intact: `data` is `User`, not a union. | |
| return `user:${event.data.email}`; | |
| // The known payload type is intact: `data` is `User`, not a union. | |
| return `user:${event.data.email.toLowerCase()}`; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/events/events.spec.ts
Line: 1760-1761
Comment:
**Payload type check misses widening**
The known-payload check does not prove that `email` remains a `string`: template interpolation also accepts `unknown`. If `event.data.email` becomes `unknown`, this assertion still compiles, leaving callers' typed reads unprotected. Assign it to a `string` variable, or call a string method, before returning it so the test catches the widening it promises to prevent.
```suggestion
// The known payload type is intact: `data` is `User`, not a union.
return `user:${event.data.email.toLowerCase()}`;
```
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Description
UnknownEventwas declared inevent.interface.tsbut never added to theEventunion, anddeserializeEvent'sdefaultbranch cast its result toEvent. A consumer writing an exhaustiveswitchoverevent.eventwith anassertNeverdefault therefore compiled clean and threw at runtime the first time the API shipped an event type the SDK did not know (#864, #1360, #1362, #1453, #1685 are all this failure). This was the second half of #1572, which was closed without it landing.This PR:
UnknownEventtoEventand a parallelUnknownEventResponsetoEventResponse.KnownEvent/KnownEventResponse;EventNameis nowKnownEvent['event'], solistEventsfilters and webhook endpoint events keep their literal typing.deserializeKnownEvent, typed againstKnownEvent, withdeserializeEventpassing through anything it returnsundefinedfor. The previousas Eventcast is gone.tsc) that a knowncasekeeps its narrowed payload whiledefaultcan no longer beassertNever'd.Why
UnknownEvent.dataisneverTypeScript keeps a union member whose discriminant is
stringin every narrowed branch: incase 'user.created':,eventisUserCreatedEvent | UnknownEvent. With the previously declareddata: Record<string, unknown>,event.datawould becomeUser | Record<string, unknown>in every branch andevent.data.emailwould beunknown, breaking essentially every consumer (and ~80 cases in the SDK's own serializer).neveris absorbed by those unions, so known branches are unchanged. The payload is still passed through unchanged at runtime (snake_case keys, no deserialization) and, sinceneveris assignable to anything, can be read by annotating it:This is the only typing that preserves narrowed reads; there is no TypeScript type for "string except these literals".
Breaking changes
Event['event']is nowstringrather than the literal union. Code that used it as anEventName(for example indexingRecord<EventName, Handler>withevent.event) must narrow first or useEventName/KnownEvent['event'].defaultbranch of aswitchoverevent.eventis no longer narrowed, soassertNever(event)there fails to compile. That is the intended signal; handle the unknown case instead.Eventwhoseeventis a union of literals (for example built inside a groupedcaseclause) are also matched againstUnknownEventby TypeScript's discriminated-union assignability and need an explicit known event type. The SDK's own serializer was adjusted for this; consumers that only read events are unaffected.Note: the first half of #1572 (plumbing an
AbortSignalthroughRequestOptions) did not land either;RequestOptionsstill has nosignal. That is a separate change.Fixes #1696
Documentation
Does this require changes to the WorkOS Docs? E.g. the API Reference or code snippets need updates.
If yes, link a related docs PR and add a docs maintainer as a reviewer. Their approval is required.