-
Notifications
You must be signed in to change notification settings - Fork 480
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.