Add useQuery lazy mode to defer query execution

[hwillson]

Initial work to support deferring useQuery execution until some point in the future (after useQuery has been initialized).

If a lazy option is set to a boolean, useQuery will return a tuple instead of a result object. The first position of the tuple is the standard query result object, and the second position is a function that can be triggered to kickstart the normal useQuery query cycle.

If lazy is true, the initial query result with have a loading status of false, and a data value of undefined. This will only change after query execution has been triggered using the delayed start function (execute in the following example):

const [{ loading, data }, execute] = useQuery(GET_ROCKET_INVENTORY, {
  lazy: true
});

If lazy is false, a tuple is still returned, but useQuery will fire immediately, just like calling useQuery without a lazy option. So these are the same:

const { loading, data } = useQuery(GET_ROCKET_INVENTORY);

const [{ loading, data }] = useQuery(GET_ROCKET_INVENTORY, {
  lazy: false
});

UPDATE

The above is no longer accurate. useQuery will always run the defined query, but useLazyQuery can be used to defer query execution. It works like:

const [{ loading, data }, execute] = useLazyQuery(GET_ROCKET_INVENTORY);

Fixes apollographql/apollo-feature-requests#119.

TODO:

• Handle skip
• Verify SSR
• Consider a called option (like useMutation)
• Handle execute variables
• Tests, tests, tests

[danielkcz]

Thanks for accepting my suggestion and working on it.

At first glance, it feels somewhat strange having a different return shape based on the existence of a boolean option value. Good thing is that we can name the execute however we want which is certainly useful. I am honestly surprised it's possible to have correct type declaration for that, never seen that before 😮 👏

On the point of called value, without it, we would have to track it in a separate state which means an extra re-render (before/after calling execute). Adding it means an extra variable has to be added to the standard query result in case of lazy mode.

[danielkcz]

Another important aspect regarding the called. Since the lazy mode has loading = false initially, it would mean we would have to explicitly check if data or error is set to display "loader". With called we can make the simple assertion as called && loading which definitely feels better semantically.

[danielkcz]

I wonder, why not to have the same order of variables in that tuple as the useMutation has? Is it intentional to keep people aware of a slightly different mechanics or just an oversight? I think it should be the same to lower the cognitive barrier. It's easy for TypeScript people to see the mistake right away, but regular JS folks might be bumping into it fairly often.

[hwillson]

Thanks for the called feedback @FredyC - great points. As for the useMutation order, we actually switched it around in #3199 (those changes haven't been published yet). So useMutation and these changes now follow more closely with the approach the React core is using.

[danielkcz]

Yea, just commented on useMutation change which I don't think is actually correct. However, for the useQuery the exact opposite applies and I would be contradicting myself here so don't mind me on this 🙊 Ultimately, it is fine either way because people can just as easily make a custom hook to switch those values over to each liking.

[hwillson]

Haha - we've struggled with this as well; there are pros/cons to both approaches for sure. For now we're going to go with the current approach of results first, function second (and we'll adjust if/as needed).

[pie6k]

I'd say better api would be like

const [shouldFetch, setShouldFetch] = useState(false)
useQuery(GET_ROCKET_INVENTORY, {
  enabled: shouldFetch,
});

and then it'll not get executed until shouldFetch is set to true.

[danielkcz]

@pie6k That's already there with skip :) Please read through my original proposition at apollographql/apollo-feature-requests#119 and please let's discuss there and keep the PR to comments about the actual code changes.

[robertvansteen]

Wouldn't it be better to have a seperate hook for this? It feels a bit odd that the return types changes based on the parameters passed to the hook. A hook like useLazyQuery is more explicit about what to expect.

[danielkcz]

@rovansteen Considering its a totally optional feature and you must explicitly specify that option, there is no benefit of separating to an extra hook. Besides, you can always make your own useLazyQuery that wraps this. #powerofhooks !

[hwillson]

@rovansteen I agree it is a bit strange, but I was interested in saving a few extra bytes. I'll defer to whichever approach @benjamn prefers, during code review.

[benjamn]

As anticipated, I would be happier with a separate useLazyQuery method, instead of passing { lazy: true } to useQuery and having to use type inference to sort out the correct return type.

[hwillson]

As of 157339f, useQuery will now always run the specified query, and useLazyQuery can be used for deferred query execution. E.g.:

const [{ loading, data }, execute] = useLazyQuery(GET_ROCKET_INVENTORY);

[danielkcz]

I agree it's probably more coherent to have a separate hook for that.

Once again about the order of arguments in the tuple. This is a similar case to useMutation. If you are going to switch it there, it would make sense to switch it here as well. Point is that without calling execute there will be no result and as such it's definitely more important.

Wouldn't be also desirable to get the Promise with a result from execute? In most cases query result is for rendering but in some cases, we might want to check it imperatively to for example send error report or redirect a user to a different page and show a toast error message. These are all doable with the inline result, but require useEffect and careful handling of deps which is tricky with objects.

Does a skip make any sense for useLazyQuery? It's possibly confusing, especially given it's always flipped to the false. What about excluding it from types there? It can surely be in underlying structures, but it shouldn't be documented as something that exists imo.

[hwillson]

All great points @FredyC, and now that we have a separate hook for this, these are easier to implement. I'll adjust - thanks!

[hwillson]

@FredyC Returning a promise with the lazy execution loaded result will require a bit more refactoring than we want to tackle before the RA 3 launch. It's a good idea, but we'll have to schedule it for a future point/minor release.

[benjamn]

Looking good!