-
Notifications
You must be signed in to change notification settings - Fork 27
Pass selection options to useRealtme hook #21
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
Conversation
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/zoo/react-supabase/FMSb3v7TwKf6bt24qbLkNNQpP1Wx |
@tmm Any idea why the Size PR check is failing ? It doesn't seem related to the code updates |
src/hooks/realtime/use-realtime.ts
Outdated
export function useRealtime<Data = any>( | ||
table: string, | ||
options: string | ({ table: string } & UseSelectConfig<Data>), | ||
compareFn: UseRealtimeCompareFn<Data> = (a, b) => | ||
(<CompareFnDefaultData<Data>>a).id === | ||
(<CompareFnDefaultData<Data>>b).id, | ||
): UseRealtimeResponse<Data> { |
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.
Can we keep the table
as a string and add a new config
parameter?
export type UseRealtimeConfig<Data> = {
select?: Omit<UseSelectConfig<Data>, 'pause'>
}
export function useRealtime<Data = any>(
table: string,
config: UseRealtimeConfig<Data>,
compareFn: UseRealtimeCompareFn<Data> = (a, b) =>
(<CompareFnDefaultData<Data>>a).id ===
(<CompareFnDefaultData<Data>>b).id,
): UseRealtimeResponse<Data>
I appreciate wanting to keep things backward compatible, but it's early enough in the project that I'm fine introducing a breaking change for the sake of a better API foundation for the future.
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.
@tmm Code is updated
Indeed this update makes the arguments and the fn api much more explicit and open for extension 🚀
Related to #17
With this update the hook
useRealtime
accepts bothstring
table name or object{ table: string } & UseSelectConfig<Data>
This update is backward compatible so no breaking change is introduced.
Docs are updated with the following example: