Skip to content
This repository has been archived by the owner on Apr 13, 2023. It is now read-only.

useLazyQuery execution function should return a promise #3499

Open
slorber opened this issue Sep 16, 2019 · 37 comments
Open

useLazyQuery execution function should return a promise #3499

slorber opened this issue Sep 16, 2019 · 37 comments

Comments

@slorber
Copy link

slorber commented Sep 16, 2019

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.

@danielkcz
Copy link
Contributor

danielkcz commented Sep 16, 2019

@slorber I already suggested it initially. @hwillson response: #3214 (comment)

Definitely would like to see it implemented as well.

@slorber
Copy link
Author

slorber commented Sep 17, 2019

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

@danielkcz
Copy link
Contributor

danielkcz commented Sep 17, 2019

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.

@slorber
Copy link
Author

slorber commented Sep 18, 2019

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

@ViacheslavNikolaienkoDevPro

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

@danielkcz
Copy link
Contributor

@ViacheslavNikolaienkoDevPro see my previous comment...

@danielkcz
Copy link
Contributor

danielkcz commented Oct 2, 2019

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
Copy link
Contributor

danielkcz commented Oct 3, 2019

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
  ]
}

@vektah
Copy link

vektah commented Oct 8, 2019

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]
    );
}

@slorber
Copy link
Author

slorber commented Oct 30, 2019

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

@sanchan
Copy link

sanchan commented Dec 3, 2019

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

@a8t
Copy link

a8t commented Dec 19, 2019

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

@danielkcz
Copy link
Contributor

danielkcz commented Dec 20, 2019

@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>>

@meglio
Copy link

meglio commented Jan 8, 2020

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

@RiseOoi
Copy link

RiseOoi commented Jan 11, 2020

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.

hyochan added a commit to hyochan/hackatalk-server that referenced this issue Jan 26, 2020
hyochan added a commit to hyochan/hackatalk-server that referenced this issue Jan 26, 2020
- Move signInEmail query to mutation with lacking useLazyQuery
    - Currently, useLazyQuery does not return promises apollographql/react-apollo#3499.

- Change `status` to `statusMessage` and do migration
   - Fix some of migration scripts.
* Add and update types for User
   - Add `isOnline`, `lastSignedIn` fields.
   - Changed to DATEONLY type for birthday and prevent being parsed to javascript date (sequelize/sequelize#4858).'
@tiavina-mika
Copy link

so finally can useLazyQuery return a Promise?

@MarkPolivchuk
Copy link

MarkPolivchuk commented Feb 14, 2020

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>
}

@dan-wu39
Copy link

dan-wu39 commented Feb 27, 2020

@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;
}

@jay-khatri
Copy link

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

@lifeiscontent
Copy link

@benjamn @hwillson any updates on this?

@bfullam
Copy link

bfullam commented Apr 7, 2020

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

@nharlow89
Copy link

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.

@tmax
Copy link

tmax commented Apr 9, 2020

@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

@zapo
Copy link

zapo commented Apr 9, 2020

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 } />;

@anantkamath
Copy link

anantkamath commented Apr 9, 2020

@zapo:
I found using skip with refetch to not work as expected: See #3921
(Thus making useLazyQuery unavoidable)

@slorber
Copy link
Author

slorber commented Apr 10, 2020

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

This solution works perfectly for me: #3499 (comment)

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]
    );
}

@fredvollmer
Copy link

@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.

@sunapi386
Copy link

@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.

@mikew
Copy link

mikew commented Jun 3, 2020

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

@ambuznego
Copy link

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 🙏

@ayepRahman
Copy link

I just wondering will these changes be implemented in V3...

@GunnarPDX
Copy link

GunnarPDX commented Jun 14, 2020

I was able to sort of work around this issue using this:
( source )

const [getDog, { loading, data }] = useLazyQuery(GET_DOG_PHOTO);  

useEffect(() => {
  if( data ) {
    // Do stuff after data updates...
    // runs only when `data` is not empty - after first and futher data loading
  }
}, [data]);

It didn't solve this issue though, so you probably want to try using useQuery with skip like this instead, it worked great for me:
( source )

const [skipQuery, setSkipQuery] = useState(true);

let { loading, error, data } = useQuery(QUERY, {
    variables: { ...variables },
    skip: skipQuery,
});

useEffect(() => {
    if (!skipQuery) {
        const onCompleted = () => {};
        const onError = () => {};
        if (onCompleted || onError) {
            if (onCompleted && !loading && !error) {
                //SuccessFunctionHere
                setSkipQuery(true);
            }
            else if (onError && !loading && error) {
                //ErrorFunctionHere
                setSkipQuery(true);
            }
        }
    }
}, [loading, data, error]);

You just trigger it via:

    setSkipQuery(false);

@jackieg017
Copy link

jackieg017 commented Jun 22, 2020

@GunnarPDX Hello! Looking at your solution above. A bit confused about what are onCompleted and onError's purpose?

@jackieg017
Copy link

jackieg017 commented Jun 22, 2020

@ambuznego What are the variables? How should those be used? In your implementation, variables are passed around, but no where are they being fed in, so curious to see the use case of that.

 const imperativelyCallQuery = (variables) => {
    return refetch(variables);
  } 

@ambuznego
Copy link

@jackieg017 The variables to pass to your query, as described here: https://www.apollographql.com/docs/react/data/queries/#refetching

There is a gotcha with this approach... refetch seems to have issues when you remove a variable in future fetches (it keeps the removed prop with the old value -- I think this is a current bug). So for some instances where my variables sometimes contained a certain prop but sometimes they didn't, this didn't work for me and I had to directly use client.query instead which can be thened

@jackieg017
Copy link

jackieg017 commented Jun 22, 2020

@ambuznego Thanks for responding 🎉
My query doesn't need any parameters to execute. So actually, passing { } as variables seems to work for me, like so:
const { data } = await callQuery({}); 🤷‍♀️

@GunnarPDX
Copy link

@jackieg017 I just copy-pasted the example I found, the variables are whatever you want them to be, and onComplete and onError are both optional. This is how I ended up implementing it without the onComplete/onError.

    let { loading, error, data } = useQuery<LookupResults>(SYMBOL_LOOKUP,
        {
            variables: {query: searchQuery},
            skip: skipQuery,
        }
    );

    useEffect(() => {
        if (!skipQuery) {
            if (!loading && !error) {
                setResults(data?.symbolLookup);
                setSkipQuery(true);
            }
            else if (!loading && error) {
                console.log(error);
                setSkipQuery(true);
            }
        }
    }, [loading, data, error]);

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests