Support skipToken in useReadQuery

[brexmatt]

• useReadQuery reads a query given a query reference returned by useBackgroundQuery.
• useBackgroundQuery supports passing in a skipToken, which results in an undefined query reference
• However, useReadQuery will throw if given an undefined query reference
• It would be nice if useReadQuery could either accept a skipToken or returned undefined data when given an undefined query reference

function PetStore() {
  const params = new URLSearchParams(window.location);
  const [dogsQueryRef] = useBackgroundQuery(
    DogsQuery,
    params.get("dogs") ? {} : skipToken
  );

  return <DogList queryRef={dogsQueryRef} />;
}

function DogList({ queryRef }) {
  const { data } = useReadQuery(queryRef || skipToken);
  const dogs = data?.dogs || DEFAULT_DOGS;

  return dogs.map(dog => <Dog dog={dog} />);
}

[jerelmiller]

Hey @brexmatt 👋 Appreciate the feature request!

This is something we discussed before we released useReadQuery and decided not to implement/allow. I'll do my best to explain the reasons and provide an alternative suggestion for you.

It would be nice if useReadQuery could ... accept a skipToken...

We opted not to have this passed in to useReadQuery since useReadQuery doesn't handle fetching itself and gets confusing when combined with useBackgroundQuery. Take this example:

const [queryRef] = useBackgroundQuery(QUERY)

// ...

const { data } = useReadQuery(skipToken)

What would you expect the value of data to be here? You could argue that undefined makes sense since you're passing the skipToken to it, but since you've already fetched the data via useBackgroundQuery, its odd that you wouldn't want to just read that data. Allowing this would force you to have to find the useBackgroundQuery that created the queryRef in order to determine if it can be skipped or not.

We opted to avoid confusion here and make it difficult for devs to do the "wrong" thing (or perhaps less optimal thing), so we reserve skipToken for useBackgroundQuery only.

That leads to your 2nd point:

It would be nice if useReadQuery could ...return undefined data when given an undefined query reference

This would be more natural than the skipToken since it's less confusing on the outcome, but when you step back, what does it mean to actually read an "undefined" query?

I like to think of this particular situation with 3 rendered states (I'll exclude the error state here and just talk about this from the "best case" scenario):

• The skipped state, which I'll refer to as the placeholder state (note: this might include just rendering null)
• The loading state
• The loaded state

When asking for useReadQuery to handle undefined as an input, essentially what is being asked is that both the placeholder state and loaded state should be rendered by the same component.

To me, that is a violation of the single responsibility principle. Having written many components in the past that handle both the placeholder UI and loaded UI states, I find these components to get messy fast and hard to maintain as new design requirements force the eventual divergence from each other.

In other words, I've build and seen many components that over time end up looking like this (despite your best intentions when you first write the component):

const MyComponent() {
  // here for illustration
  const { data } = useReadQuery(undefined)

  return (
    <div className="container">
      {data && (
        <h1 className="title">The things</h1>
      )}
      <div className="inner-container">
        {!data && <span className="placeholder-text">Not loaded yet</span>
        {data && <MyOtherComponent things={data.things} />
      </div>
    </div>
  );
}

Hopefully you get the picture, but imagine this many times bigger. Placeholder and loaded UIs start to get intermingled typically because every dev wants to keep things DRY.

Instead, we recommend splitting at the component level in the parent and creating separate components that handle the placeholder and loaded UIs. We believe this will typically scale much better in the long term.

For your example, this would look something like this:

function PetStore() {
  const params = new URLSearchParams(window.location);
  const [dogsQueryRef] = useBackgroundQuery(
    DogsQuery,
    params.get("dogs") ? {} : skipToken
  );

  return dogsQueryRef
    ? <DogList queryRef={dogsQueryRef} />
    : <DefaultDogs />
}

function DogList({ queryRef }) {
  const { data } = useReadQuery(queryRef);

  return data.dogs.map(dog => <Dog dog={dog} />);
}

function DefaultDogs() {
  return DEFAULT_DOGS.map(dog => <Dog dog={dog} />);
}

---

To me, that is a violation of the single responsibility principle

As an aside to my point above, yes I understand that other hooks would enable you to do this (for example, when using useSuspenseQuery with skipToken). While we could allow it, we are doing our best to try and encourage "best practices" wherever we can. This also helps us simplify the logic of useReadQuery a bit since we can eliminate the need for defensive coding inside the hook by recommending that users split their components in the parent to handle this case.

---

I hope this makes sense and gives you a decent idea of why/where we landed with this!

Edit: That being said, is there something you find very difficult or impossible to do with the current design that could be solved by allowing undefined as an input?

[brexmatt]

Hey @jerelmiller, thanks so much for the prompt and detailed response!

The use case I have is pretty complex and it's basically for migrating from useQuery everywhere to useBackgroundQuery.

We have usage of useQuery all over our codebase and it's unpredictable what pages a query will be invoked on. Thus, I was hoping to create some wrappers around useBackgroundQuery and useReadQuery like this:

// **LEGACY CODE**

// Legacy component to fetch/display card data
// Used all over the codebase
function CardController() {
  const { data } = useQuery(PrimaryCardQuery);
  return (
    <>
      ...
      {/* display "data" */}
    </>
  );
}

// Legacy view component passed to React Router
function WalletView() {
  return (
    <>
      ...
      <CardController />
    </>
  );
}

// ** v3.8 UTILITIES TO PREVENT WATERFALLS **

// Context to store queryRefs that can be used by descendent components
const BackgroundQueriesContext = React.createContext(new Map([
  // Example entry:
  // [PrimaryCardQuery, primaryCardQueryRef]
]))

// Update WalletView to fetch queries at top of component tree
function WalletView() {
  const [primaryCardQueryRef] = useBackgroundQuery(PrimaryCardQuery);

  const context = new Map([
    [PrimaryCardQuery, primaryCardQueryRef]
    // can include others
  ]);

  return (
    <BackgroundQueriesContext.Provider value={context}>
      ...
      <CardController />
    </BackgroundQueriesContext.Provider>
  );
}

// Update CardController to read the query that was executed earlier
// Reminder: used all over the codebase
function CardController() {
  const context = React.useContext(BackgroundQueriesContext);

  const primaryCardQueryRef = context.get(PrimaryCardQuery);

  const { data } = useReadQuery(primaryCardQueryRef);

  return (
    <>
      ...
      {/* display "data" */}
    </>
  );
}

// Problem: 
// - CardController requires that an ancestor component kicked off the query and stored it in context
// - It is used all over the codebase (Think 100s of times) and we don't want to refactor every usage 
//   to kick off the query earlier

// Solution:
// - A hook that reads the query from context if available, otherwise kicks it off
function usePrefetchableQuery(query, options) {
  const context = React.useContext(BackgroundQueriesContext);
  const prefetchedQueryRef = context.get(query); // could be undefined

  // Start the fetch now if was not already prefetched
  const [nonPrefetchedQueryRef] = useBackgroundQuery(
    query,
    // options could include skip: true, which would result in undefined queryRef
    prefetchedQueryRef ? skipToken : options
  );

  // could be undefined if not in context or options included skip: true
  const queryRef = prefetchedQueryRef || nonPrefetchedQueryRef;

  // Problem: will throw if queryRef is undefined
  return useReadQuery(queryRef);
}

// Update CardController to use the util
function CardController() {
  // - Will read the data from context if available, otherwise kick it off
  // - Safe to call anywhere
  const { data } = usePrefetchableQuery(primaryCardQueryRef);

  return (
    <>
      ...
      {/* display "data" */}
    </>
  );
}

Does this make sense?

[jerelmiller]

Thanks for the code sample @brexmatt! This is useful to understand what you're up against here!

I'm hesitant to add support for undefined for this specific use case, but can empathize with what you're trying to do here.

Maybe a wacky idea, but are you against using a render props approach to help solve this? I know its not as sexy as a custom hook, but it would give you the ability to do conditional logic that a pure hooks approach wouldn't. This might be a decent migration step that you can refactor out of over time to avoid having to do this migration all at once.

function PrefetchableQuery({ children, query, options }) {
  const context = React.useContext(BackgroundQueriesContext);
  const prefetchedQueryRef = context.get(query); // could be undefined

  // Start the fetch now if was not already prefetched
  const [nonPrefetchedQueryRef] = useBackgroundQuery(
    query,
    // options could include skip: true, which would result in undefined queryRef
    prefetchedQueryRef ? skipToken : options
  );

  // could be undefined if not in context or options included skip: true
  const queryRef = prefetchedQueryRef || nonPrefetchedQueryRef;

  return queryRef
    ? <ReadQuery children={children} queryRef={queryRef} />
    : children(undefined)
}

function ReadQuery({ children, queryRef }) {
  return children(useReadQuery(queryRef))
}

function CardController() {
  return (
    <PrefetchableQuery query={query}>
      {(data) => (
        <>
          ...
          {/* display "data" */}
        </>
      )}
    </PrefetchableQuery>
  );
}

Let me know what you think!

[brexmatt]

I like the idea in theory but in practice so many of our query code is tied up in hooks I think widely adopting that pattern would be too large of a refactor. Anyway, appreciate your responses here!

[jerelmiller]

Well dang, I was hoping it could be a decent stop-gap for you in the interim, but totally understand! I hope you can understand why we are hesitant to add support for it right now and am sorry this probably isn't quite the answer you were looking to hear. I hope you're able to find a solution that works well for you during the migration process!

Talking with the team earlier, we are going to leave this issue open to noodle on it a bit longer. There very well be legitimate use cases here that we just aren't seeing yet. Please chime in if you have more ideas here. Thanks again for opening this request!

[brexmatt]

Sounds good, again, really appreciate your attention to this! We're still very early in factoring POCs with the new APIs

[vimutti77]

My use case is I have a context that provides data from useQuery, along with a consumer hook that retrieves data from the context and computes a result. However, in certain scenarios within my project, the context provider might be optional. In those cases, the consumer will return an empty array.

This is the example code.

const MyDataContext = React.createContext(undefined)

const MyDataProvider = ({ children }) => {
  const { data } = useQuery(MyDataDocument)

  return <MyDataContext.Provider value={data?.myData}>{children}</MyDataContext.Provider>
}

const useMyData = (options) => {
  const myData = React.useContext(MyDataContext)

  return useMemo(() => {
    if (!myData) return []
    return computeData(myData, options)
  }, [myData])
}

const SomeChildComponent = () => {
  const myData = useMyData(someOptions)

  return (
    <>
      {myData.map((x) => (
        <ListOfMyData data={x} />
      ))}
      <OtherComponent />
    </>
  )
}

// There is provider in this page
const Page1 = () => {
  return (
    <MyDataProvider>
      <SomeChildComponent />
    </MyDataProvider>
  )
}

// No provider in this page
const Page2 = () => {
  return <SomeChildComponent />
}

Despite the absence of a context provider on Page 2, it continues to function correctly

I want to transform the code to utilize useBackgroundQuery and useReadQuery. So, I transform it into this.

const MyDataContext = React.createContext(undefined)

const MyDataProvider = ({ children }) => {
  const queryRef = useBackgroundQuery(MyDataDocument)

  return <MyDataContext.Provider value={queryRef}>{children}</MyDataContext.Provider>
}

const useMyData = (options) => {
  const queryRef = React.useContext(MyDataContext)
  const { data } = useReadQuery(queryRef)
  const myData = data?.myData

  return useMemo(() => {
    if (!myData) return []
    return computeData(myData, options)
  }, [myData])
}

const SomeChildComponent = () => {
  const myData = useMyData(someOptions)

  return (
    <>
      {myData.map((x) => (
        <ListOfMyData data={x} />
      ))}
      <OtherComponent />
    </>
  )
}

// There is provider in this page
const Page1 = () => {
  return (
    <MyDataProvider>
      <SomeChildComponent />
    </MyDataProvider>
  )
}

// No provider in this page
const Page2 = () => {
  return <SomeChildComponent />
}

After running the code, everything works fine on Page 1, but an error is thrown on Page 2.

What I want is for useReadQuery to return empty data when queryRef is undefined, rather than throwing an error.
Alternatively, please allow me to pass a skipToken to useReadQuery.