- Notifications
You must be signed in to change notification settings - Fork 492
useQuery via object #261
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: main
Are you sure you want to change the base?
useQuery via object #261
Conversation
| My biggest question right now is around the
I'm also hesitant to make |
| I'm leaning towards only having |
| instantiateNoopLogger, | ||
| Logger, | ||
| } from "../browser/logging.js"; | ||
| import { ConvexQueryOptions } from "../browser/query_options.js"; |
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.
@ianmacartney this is another start on this type, in the final form of this PR would be good to unify
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.
Thanks! Missed that - wdyt about QueryOptions instead of ConvexQueryOptions? The naming conflict with react-query is my guess as to why the namespacing? But none of the other types have this namespace, and users likely won't be importing the type
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.
Yeah QueryOptions is good, I was looking too-far into the future here
Agree that Also, if we implement |
| @chenxin-yan I explored it in #262 (not making args missing, but unknown, since you want to do something like One way to alleviate the "I don't like that it's a magic string" would be to |
85f1f0e to ccad381 Compare
I see. Although I'm not a big fan of magic string but I think just use |
| As I was discussing this with a colleague, we realized that skipping when you already have preloaded data is less common of a need, versus the more specific need of requiring auth. If `usePreloadedQuery` accepted a `requireAuth: true` flag which returned the preloaded data until it was authenticated, that would be another way to solve your initial issue. Is that right? …On Mon, Nov 24, 2025 at 1:25 PM Chenxin Yan ***@***.***> wrote: *chenxin-yan* left a comment (get-convex/convex-backend#261) <#261 (comment)> @chenxin-yan <https://github.com/chenxin-yan> I explored it in #262 <#262> (not making args missing, but unknown, since you want to do something like useQuery({query: api.foo.bar, args: { userId }, skip: !userId }). Unfortunately the type discrimination doesn't work, so you're left with !userId ? { skip: true } : {args: { userId}, ...} which may as well be the "skip" sentinel. One way to alleviate the "I don't like that it's a magic string" would be to export const skipToken = "skip" as const"; and use !userId? skipToken : { query, args, ..} I see. Although I'm not a big fan of magic string but I think just use "skip" instead of export const skipToken = "skip" as const"; is superior because your editor will autocomplete for "skip" and you will have to remember to use skipToken in contrast. Anyways, using "skip" in this case if all good imo. — Reply to this email directly, view it on GitHub <#261 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACZQWYLZEQF4JSOTQ7K6SL36NZTXAVCNFSM6AAAAACMXIBBXKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTKNZSG44DOOBUG4> . You are receiving this because you were mentioned.Message ID: ***@***.***> |
| @ianmacartney Agree, and there should be a |
Allow calling useQuery with an object instead of (api.foo.bar, args)
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.