-
-
Notifications
You must be signed in to change notification settings - Fork 70
feat(realtime): enhance RealtimeChannel type #459
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Enhances the RealtimeChannel
typing so that broadcast payloads infer the record shape directly from the generic parameter.
- Introduces
RealtimeBroadcast*Payload
types with a shared base and specific insert/update/delete variants. - Adds overloaded
on<T>
methods forBROADCAST
events (ALL, INSERT, UPDATE, DELETE) returning strongly-typed payloads.
Comments suppressed due to low confidence (2)
src/RealtimeChannel.ts:441
- These new generic overloads and payload types should be covered by unit tests to ensure type inference works as expected and that events dispatch correct payload shapes.
on<T extends { [key: string]: any }>(
src/RealtimeChannel.ts:443
- For the ALL overload, the
event
field is typed as ALL but at runtime the payload.event will be the specific INSERT/UPDATE/DELETE. Update the type to a union of specific events or rename to avoid misleading consumers.
filter: { event: REALTIME_POSTGRES_CHANGES_LISTEN_EVENT.ALL },
type RealtimeBroadcastChangesPayloadBase = { | ||
id: string | ||
schema: string | ||
table: string | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] RealtimeBroadcastChangesPayloadBase overlaps with RealtimePostgresChangesPayloadBase (schema/table). Consider merging shared fields or extending one from the other to reduce duplication.
type RealtimeBroadcastChangesPayloadBase = { | |
id: string | |
schema: string | |
table: string | |
} | |
type RealtimeChangesPayloadBase = { | |
schema: string | |
table: string | |
} | |
type RealtimeBroadcastChangesPayloadBase = RealtimeChangesPayloadBase & { | |
id: string | |
} |
Copilot uses AI. Check for mistakes.
@@ -406,6 +438,42 @@ export default class RealtimeChannel { | |||
payload: T | |||
}) => void | |||
): RealtimeChannel | |||
on<T extends { [key: string]: any }>( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Using T extends { [key: string]: any }
is very loose. Consider constraining to Record<string, unknown>
or a more specific shape to improve type safety.
on<T extends { [key: string]: any }>( | |
on<T extends Record<string, unknown>>( |
Copilot uses AI. Check for mistakes.
@@ -406,6 +438,42 @@ export default class RealtimeChannel { | |||
payload: T | |||
}) => void | |||
): RealtimeChannel | |||
on<T extends { [key: string]: any }>( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The multiple overloads for broadcast events are largely repetitive. You could consolidate them using a single overload with conditional types on filter.event
to reduce duplication.
Copilot uses AI. Check for mistakes.
What kind of change does this PR introduce?
infer payload type sent by
realtime.broadcast_changes
What is the current behavior?
What is the new behavior?
Additional context