Allow useShopQuery to be disabled via options

[davidhousedev]

As we've built more components that use useShopQuery, we've encountered some situations where we want to invoke the query conditionally. Currently, that's very challenging in useShopQuery without breaking the rules of hooks. React Query gets around this by exposing an enabled option on their queries which allow the hook to be called but not run. As a result, they also return a status in the return type.

We can get around this in useQuery by rejecting the promise early in the query function, but we're having a tougher time with useShopQuery. That said, disabling a useQuery is non-trivial. One option is to return a Promise.reject from the query function, but this will log an expected behavior as an error from within useQuery. The other option is to have the function return a type union, but the typings for useQuery expect that the query function only returns one interface so you need to dip into generic typing on useQuery.

Edit: My mistake! I was misinterpreting what I was seeing in my terminal.

[frehner]

It's not very clear in the docs, but you can conditionally not call useShopQuery by passing in an empty string for the query prop.

Does this help, or are you looking for something different?

[juanpprieto]

Slightly controversial, but I would argue that in .server components the traditional rules of hooks don't really apply.

Both useShopQuery and useQuery are not "real" hooks. They are named like they are for consistency. 🤔

With that in mind, conditionally fetching should be ok

function ServerComponent({x}) {
  let data;
  if (x) {
   const query = useShopQuery({..})
   data = query.data;
  }

  return data && (...)
}

[davidhousedev]

@juanpprieto I had this exact thought a few weeks ago and brought up the question out-of-band. @blittle informed me that hooks are re-run in the event of errors, so rules of hooks may apply? I hope we can get to the bottom of this!

Frankly, hooks have proven to be very difficult to work with because we end up rolling our own enable/disable logic (as is implemented in React Query at the library level). I would love to be able to call Hydrogen data fetching APIs conditionally!

Also, thanks for the suggestion around conditional querying! It feels a little less intuitive from an ongoing maintenance perspective than using useQuery for everything and subscribing to the non-rate-limited API token.

[frehner]

@davidhousedev Does the empty string option for useShopQuery not fulfill your needs?

(A basic and probably not syntactically correct example)

const queryString = shouldQuery ? 'query {}' : '';

useShopQuery(queryString)

[adityavaishnav30]

Hi

[davidhousedev]

Hey @frehner my apologies for the late response! I think from a concrete requirements standpoint, passing an empty string would meet our needs. That said, there are two reasons why we're preferring to use useQuery for everything:

1. The return types on useQuery are much easier to work with. They are much easier to narrow because they use a union between an error object and a data object.
2. We're trying to keep conventions consistent in our codebase. We're querying three different external APIs in our Storefront repo: the Shopify Storefront API, Contentful's GraphQL API, and our own internal REST api. By consolidating on useQuery usage for all three of our API clients, it makes it much easier for new devs to onboard to the codebase.

Lastly, from a DX perspective, it's counter-intuitive to pass in an empty string to disable a query. Granted, it's also counter-intuitive to throw new DisabledHookError() as we're doing in our useQuery implementations, but at least we only have one counter-intuitive convention rather than two 😅.