react-apollo icon indicating copy to clipboard operation
react-apollo copied to clipboard

useLazyQuery execution function should return a promise

Open slorber opened this issue 6 years ago • 37 comments

People don't know if they should use a query or a mutation to login an user when that login performs no side-effect (ie, just query for an auth token).

https://stackoverflow.com/questions/50189364/shouldnt-the-login-be-a-query-in-graphql

But Apollo users tend to currently use mutations because they want the operation to execute only after user press a submit button.

Recently a new useLazyQuery was added (by @FredyC I think?), and it could be useful to solve this problem: keep a query, but actually execute it after user press a button.

Unlike mutations, it does not return a promise when executed.

I suggest the following should be possible:

image

edit: actually the screenshot is wrong but you probably understand the point of returning a promise instead of void here (at least in TS typings)

Note other apis do return a promise when called (see "refetch" for example). I think useLazyQuery should follow the exact same convention and not return void.

slorber avatar Sep 16 '19 17:09 slorber

@slorber I already suggested it initially. @hwillson response: https://github.com/apollographql/react-apollo/pull/3214#issuecomment-512309558

Definitely would like to see it implemented as well.

danielkcz avatar Sep 16 '19 18:09 danielkcz

Thanks

@hwillson any idea when you plan to implement this? Is there a way to wire a query to a user button press and still get the result? I asked my team to use a mutation instead for now

slorber avatar Sep 17 '19 11:09 slorber

Quick and dirty way for lazy query execution is something like this...

const variablesRef = useRef({})
const [shouldExecute, executeQuery] = useState(false)
const { data, loading } = useQuery({ variables: variablesRef.current, skip: !shouldExecute })

const onClick = () => {
  variablesRef.current = { username: "daniel" }
  executeQuery(true)
}

Fairly ugly, but can be extracted into a custom useLazyQuery hook as a temporary solution and then use the real thing when it's ready. These are exactly reasons why I've suggested useLazyQuery because it can get pretty wild :)

Obviously, if you need to do some side effect based on query result (eg. redirect after login), you need to have useEffect with data as deps and check for shouldExecute as well.

danielkcz avatar Sep 17 '19 17:09 danielkcz

yeah, I'll use a mutation for now, way simpler :D

slorber avatar Sep 18 '19 10:09 slorber

How to solve the problem? I mean I need a promise as returned value, not undefined.

@ViacheslavNikolaienkoDevPro see my previous comment...

danielkcz avatar Sep 23 '19 10:09 danielkcz

If anyone is impatient (like me), I made a small wrapper around useLazyQuery that can return Promise. It seems to work just fine and at least I don't need to rewrite anything when official support drops in.

export function useLazyQuery<TData = any, TVariables = OperationVariables>(
  query: DocumentNode,
  options?: LazyQueryHookOptions<TData, TVariables>,
): LazyQueryHookTuple<TData, TVariables> {
  const [execute, result] = Hooks.useLazyQuery<TData, TVariables>(query, options)

  const resolveRef = React.useRef<
    (value?: TData | PromiseLike<TData>) => void
  >()
  const rejectRef = React.useRef<(reason?: any) => void>()

  React.useEffect(() => {
    if (result.called) {
      if (result.data !== undefined && resolveRef.current) {
        resolveRef.current(result.data)
      } else if (result.error !== undefined && rejectRef.current) {
        rejectRef.current(result.error)
      } else {
        return
      }
      resolveRef.current = undefined
      rejectRef.current = undefined
    }
  }, [result.data, result.error, result.called])

  const queryLazily: LazyQueryExecute<TData, TVariables> = React.useCallback(
    (variables, context) => {
      execute({ variables, context })
      return new Promise<TData>((resolve, reject) => {
        resolveRef.current = resolve
        rejectRef.current = reject
      })
    },
    [execute],
  )

  return [
    queryLazily,
    result
  ]
}

danielkcz avatar Oct 02 '19 14:10 danielkcz

And I just came to a realization that the Promise should not be rejected at all. Mainly because it's usually redundant as an error will appear in the result itself and can be handled more gracefully in a render phase. The rejected promise is impossible to be caught by any React mechanism (error boundary) and would need to be handled per each case on the call site, otherwise, the annoying unhandledRejection would appear in logs.

In my opinion, it's enough if the Promise resolves with the whole result object when "loading" is done.

Perhaps that's something to consider for a real implementation later.

export function useLazyQuery<TData = any, TVariables = OperationVariables>(
  query: DocumentNode,
  options?: LazyQueryHookOptions<TData, TVariables>,
): LazyQueryHookTuple<TData, TVariables> {
  const [execute, result] = Hooks.useLazyQuery<TData, TVariables>(query, options)

  const resolveRef = React.useRef<
    (value?: LazyQueryResult<TData>| PromiseLike<LazyQueryResult<TData>>) => void
  >()

  React.useEffect(() => {
    if (result.called && !result.loading && resolveRef.current) {
      resolveRef.current(result)
      resolveRef.current = undefined
    }
  }, [result.loading, result.called])

  const queryLazily: LazyQueryExecute<TData, TVariables> = React.useCallback(
    (variables, context) => {
      execute({ variables, context })
      return new Promise<LazyQueryResult<TData>>((resolve) => {
        resolveRef.current = resolve
      })
    },
    [execute],
  )

  return [
    queryLazily,
    result
  ]
}

danielkcz avatar Oct 03 '19 01:10 danielkcz

You could also bind directly to the client if you only care about an invokable function and a cache update:

export function useLazyQuery<TData = any, TVariables = OperationVariables>(query: DocumentNode) {
    const client = useApolloClient();
    return React.useCallback(
        (variables: TVariables) =>
            client.query<TData, TVariables>({
                query: query,
                variables: variables,
            }),
        [client]
    );
}

vektah avatar Oct 08 '19 05:10 vektah

great idea @vektah , that looks like the simplest option for now ;)

slorber avatar Oct 30 '19 15:10 slorber

I've just implemented this for my project, just in case somebody find it useful:

function useLazyQuery(query, options) {
    const ref = React.useRef()

    const [variables, runQuery] = React.useState(false)

    ref.current = useQuery(query, {
        ...options,
        variables,
        skip: !variables
    })

    const runner = (variables) => {
        runQuery(variables)
    }

    return [runner, ref.current]
}

So you can do this:

const [runQuery, { loading, data }] = useLazyQuery(QUERY, {
    onCompleted: () => doSomething()
})

// ...

const handleClick = (ev) => {
    const variables = { ... }
    runQuery(variables)
}

EDIT: Added options parameter so we can use onCompleted

sanchan avatar Dec 03 '19 04:12 sanchan

@FredyC Where are these type annotations coming from? I don't see them exposed in @apollo/react-hooks

a8t avatar Dec 19 '19 23:12 a8t

@a8t I wrapped around the existing ones because of slightly different API.

import { Context, OperationVariables } from '@apollo/react-common'
import * as Hooks from '@apollo/react-hooks'

export type LazyQueryHookOptions<
  TData,
  TVariables
> = Hooks.LazyQueryHookOptions<TData, TVariables>

export type LazyQueryExecute<TData, TVariables> = (
  variables?: TVariables,
  context?: Context,
) => Promise<Maybe<TData>>

danielkcz avatar Dec 20 '19 04:12 danielkcz

@FredyC what about LazyQueryHookTuple, where does it come from?

meglio avatar Jan 08 '20 15:01 meglio

Vote for this implementation. useLazyQuery should return a Promise, else people would need to use useMutation (as a workaround/hack) for something that should be a 'query', which is exactly like using POST to achieve something that theoretically & philosophically should be GET in the old days. If Apollo really wants to push the idea that GraphQL will replace REST then don't let GraphQL falls into the same trap REST did.

RiseOoi avatar Jan 11 '20 14:01 RiseOoi

so finally can useLazyQuery return a Promise?

tiavina-mika avatar Feb 11 '20 13:02 tiavina-mika

My workaround (simplified):

const useImperativeQuery = (query) => {
  const { refetch } = useQuery(query, { skip: true });
	
  const imperativelyCallQuery = (variables) => {
    return refetch(variables);
  } 
	
  return imperativelyCallQuery;
}

Usage:

const query = gql`
  ...
`;

const Component = () => {
  const callQuery = useImperativeQuery(query)


  const handleClick = async () => {
    const{ data, error } = await callQuery()
  }
  
  return <button onClick={handleClick}>Call Query</button>
}

MarkPolivchuk avatar Feb 14 '20 00:02 MarkPolivchuk

@MarkPolivchuk nice. Does refetch use the same options you pass into useQuery?

const useImperativeQuery = (query, options = {}) => {
  const { refetch } = useQuery(query, { skip: true, ...options }); // <- will these options persist
	
  const imperativelyCallQuery = (variables) => {
    return refetch(variables); // <- when this call happens?
  } 
	
  return imperativelyCallQuery;
}

dan-wu39 avatar Feb 27 '20 01:02 dan-wu39

Any updates on this feature? From the many workarounds proposed above, it seems like this should be baked into useLazyQuery.

jay-khatri avatar Mar 16 '20 20:03 jay-khatri

@benjamn @hwillson any updates on this?

lifeiscontent avatar Mar 27 '20 01:03 lifeiscontent

@benjamn @hwillson any word? Running into this issue as well and would rather avoid workarounds if this idea is already on the way!

bfullam avatar Apr 07 '20 12:04 bfullam

A typed version of @MarkPolivchuk's solution:

import { useQuery, QueryHookOptions } from '@apollo/react-hooks';
import { OperationVariables } from 'apollo-client';
import { DocumentNode } from 'graphql';
import { QueryResult } from '@apollo/react-common';

/**
 * Small wrapper around `useQuery` so that we can use it imperatively.
 *
 * @see Credit: https://github.com/apollographql/react-apollo/issues/3499#issuecomment-586039082
 *
 * @example
 * const callQuery = useImperativeQuery(query, options)
 * const handleClick = async () => {
 *   const{ data, error } = await callQuery()
 * }
 */
export default function useImperativeQuery<
  TData = any,
  TVariables = OperationVariables
>(
  query: DocumentNode,
  options: QueryHookOptions<TData, TVariables> = {}
): QueryResult<TData, TVariables>['refetch'] {
  const { refetch } = useQuery<TData, TVariables>(query, {
    ...options,
    skip: true,
  });

  const imperativelyCallQuery = (queryVariables: TVariables) => {
    return refetch(queryVariables);
  };

  return imperativelyCallQuery;
}


@dan-wu39 The useQuery options are different from the imperativelyCallQuery arguments. See the useQuery api and the types above.


+1 for adding return promise to useLazyQuery action.

nharlow89 avatar Apr 07 '20 22:04 nharlow89

@nharlow89 @MarkPolivchuk one thing to note with using refetch is that it will always bypass the cache;

so to answer @dan-wu39 's question, the options will be passed through, but they get overwritten to network-only regardless what fetchPolicy you provide:

https://github.com/apollographql/apollo-client/blob/7c9afe085371e624dcb3e6eb0c888965c982746e/src/core/ObservableQuery.ts#L265

tmax avatar Apr 09 '20 05:04 tmax

Why even have a useLazyQuery if useQuery combined with skip and refetch already does the job (if indeed it's intended skip is ignored in refetch calls) ? Maybe useLazyQuery could be dropped and this usecase be more of a documentation problem ? I might be missing the point of useLazyQuery though.

const { refetch: getThings, loading } = useQuery(getThingsGQL, { variables, skip: true});
const onChange = (event) => { 
  getThings({ variables: newVariables(variables, event) })
    .then(handleThings);
};

return loading ? <Spinner /> : <input onChange={ onChange } />;

zapo avatar Apr 09 '20 13:04 zapo

@zapo: I found using skip with refetch to not work as expected: See https://github.com/apollographql/react-apollo/issues/3921 (Thus making useLazyQuery unavoidable)

anantkamath avatar Apr 09 '20 14:04 anantkamath

Hey, you are complicating your life too much with skip and refetch. Using the client directly works fine.

This solution works perfectly for me: https://github.com/apollographql/react-apollo/issues/3499#issuecomment-539346982

export function useLazyQuery<TData = any, TVariables = OperationVariables>(query: DocumentNode) {
    const client = useApolloClient();
    return React.useCallback(
        (variables: TVariables) =>
            client.query<TData, TVariables>({
                query: query,
                variables: variables,
            }),
        [client]
    );
}

slorber avatar Apr 10 '20 08:04 slorber

@slorber The only issue with that solution is that it's no reactive; i.e., the component won't receive re-render in response to future cache updates. This was my first thought as well :) If the component doesn't need to receive updates, then this is definitely the simplest way to go.

fredvollmer avatar Apr 10 '20 11:04 fredvollmer

@nharlow89 that worked perfectly (with a bit of import modification), thanks!

I had to change a bit, so my imports now look like this:

import { DocumentNode } from 'graphql';
import { OperationVariables, QueryHookOptions, QueryResult, useQuery } from '@apollo/client';

Otherwise this wrapper will do for now until fixed.

sunapi386 avatar May 03 '20 22:05 sunapi386

What's more confusing is that both refetch and fetchMore from useLazyQuery do return a promise.

mikew avatar Jun 03 '20 21:06 mikew

My workaround (simplified):

const useImperativeQuery = (query) => {
  const { refetch } = useQuery(query, { skip: true });
	
  const imperativelyCallQuery = (variables) => {
    return refetch(variables);
  } 
	
  return imperativelyCallQuery;
}

Usage:

const query = gql`
  ...
`;

const Component = () => {
  const callQuery = useImperativeQuery(query)


  const handleClick = async () => {
    const{ data, error } = await callQuery()
  }
  
  return <button onClick={handleClick}>Call Query</button>
}

This worked like a charm! Thank you @MarkPolivchuk 🙏

ambuznego avatar Jun 06 '20 01:06 ambuznego