feat: add skip to usePreloadedQuery

[chenxin-yan]

As discussed in this discord thread, I've created a minimal demo repo to showcase the DX and UX differences between with/without skip option for usePreloadedQuery

The primary motivation for this is it would be nice to have an elegant and intuitive api for waiting for auth token to be loaded on client without blocking other parts of the component. With current api, to achieve this you can create another client component and wrap it with <Authenticated/>, but it becomes tedious in a lot of cases where you do not want to decouple the rendering logic to another component.

before:

import { preloadQuery } from "convex/nextjs";
import { getAuthToken } from "@/lib/convex";
import { api } from "../../../convex/_generated/api";
import { AuthenticatedContent } from "./components/AuthenticatedContent";
import { TodoList } from "./components/TodoList";

const OldPage = async () => {
  const token = await getAuthToken();
  const preloadedTodos = await preloadQuery(api.tasks.get, {}, { token });

  return (
    <AuthenticatedContent>
      <TodoList preloadedTodos={preloadedTodos} />
    </AuthenticatedContent>
  );
};

export default OldPage;

"use client";

import { type Preloaded, usePreloadedQuery } from "convex/react";
import type { api } from "../../../../convex/_generated/api";

interface Props {
  preloadedTodos: Preloaded<typeof api.tasks.get>;
}

export function TodoList({ preloadedTodos }: Props) {
  const todos = usePreloadedQuery(preloadedTodos);

  return (
    <>
      <h1>some todos:</h1>
      <ul>
        {todos.map((todo) => (
          <li key={todo._id}>{todo.text}</li>
        ))}
      </ul>
      <p>above are my todos</p>
    </>
  );
}

After:

import { getAuthToken } from "@/lib/convex";
import { preloadQuery } from "convex/nextjs";
import { api } from "../../../convex/_generated/api";
import { TodoList } from "./components/TodoList";

const NewPage = async () => {
  const token = await getAuthToken();
  const preloadedTodos = await preloadQuery(api.tasks.get, {}, { token });

  return <TodoList preloadedTodos={preloadedTodos} />;
};

export default NewPage;

"use client";

import { type Preloaded, useConvexAuth, usePreloadedQuery } from "convex/react";
import type { api } from "../../../../convex/_generated/api";

interface Props {
  preloadedTodos: Preloaded<typeof api.tasks.get>;
}

export function TodoList({ preloadedTodos }: Props) {
  const { isLoading } = useConvexAuth();
  const todos = usePreloadedQuery(preloadedTodos, { skip: isLoading });

  return (
    <>
      <h1>some todos:</h1>
      <ul>
        {todos ? (
          todos.map((todo) => <li key={todo._id}>{todo.text}</li>)
        ) : (
          <p>loading...</p>
        )}
      </ul>
      <p>above are my todos</p>
    </>
  );
}

Closes #98

I am not sure if I missed anything from documentation that can handle this more elegantly, but it is weird to me that you can skip useQuery but not usePreloadedQuery.

---

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[ianmacartney]

Great call. I think you’re right. We need to add this or something like it.
I’m biased towards syntax like
usePreloadedQuery(isLoading? “skip”: preloadedTodos)
That way you don’t have to have preloadedTodos type check when skipping. For instance, maybe you want to skip a preloaded query based on some union of arguments where sometimes you’ll render a page with the preloadedTodos as null/undefined.
wdyt @chenxin-yan ?

[ianmacartney]

Another thing that sticks out is now everyone will need to handle undefined even if they never use Skipp. We could solve this with an overload (not my favorite) but on the surface it’s a breaking change right now.

[chenxin-yan]

Great call. I think you’re right. We need to add this or something like it.

I’m biased towards syntax like

usePreloadedQuery(isLoading? “skip”: preloadedTodos)

That way you don’t have to have preloadedTodos type check when skipping. For instance, maybe you want to skip a preloaded query based on some union of arguments where sometimes you’ll render a page with the preloadedTodos as null/undefined.

wdyt @chenxin-yan ?

I agree. It will be more consistent and predictable since useQuery uses similar syntax. I will make the change.

[chenxin-yan]

Another thing that sticks out is now everyone will need to handle undefined even if they never use Skipp. We could solve this with an overload (not my favorite) but on the surface it’s a breaking change right now.

I already have an idea of how to address this. I will test it out when I got back home and let you know.

[chenxin-yan]

@ianmacartney I just update the changes to what we've discussed. You were right. I don't see better way to handle capability other than overriding the function (which is what I implemented). Everything is working and tested in this repo. The only thing am a little unsure about is this:

  const result = useQuery(
    skip
      ? (makeFunctionReference("_skip") as Query)
      : (makeFunctionReference(preloadedQuery._name) as Query),
    skip ? ("skip" as const) : args,
  );

where I have to create this dummy placeholder for makeFunctionReference("_skip") as useQuery does not take in undefined or null. imo it works but might not be the most elegant solution. Just to point it out here in case you have better way of implementing this. Thanks for getting back to me.