Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

skip: true should init the observable #6796

Closed
ernieyang09 opened this issue Aug 7, 2020 · 12 comments
Closed

skip: true should init the observable #6796

ernieyang09 opened this issue Aug 7, 2020 · 12 comments

Comments

@ernieyang09
Copy link

Intended outcome:
We can use refetch function even if useQuery with skip: true option.

Actual outcome:
Uncaught (in promise) TypeError: Cannot read property 'refetch' of undefined
at QueryData._this.obsRefetch (QueryData.ts:457)

I'm using Imperative Query function mention here

And I found the issue #6670 and the commit.

I think this line should be remove.

c0ec90d#diff-fd2ae033c9b90f5e2c0f060ff67bf095R230

if you don't init ObservableQuery, you can't use this query(refetch, fetchMore, updateQuery.....)

and I don't see any network request after I remove this line as well.

Versions

npmPackages:
@apollo/client: ^3.1.2 => 3.1.2

@hwillson

@malikalimoekhamedov
Copy link

malikalimoekhamedov commented Aug 9, 2020

Having exactly the same issue.
This started from 3.1.2 and is still present in 3.1.3. Currently rolling back to 3.1.1 for it to work.

Example query (TypeScript):

const { refetch, loading, error } = useQuery<blah, blahVariables>(BLAH, { skip: true });

@ernieyang09
Copy link
Author

ernieyang09 commented Aug 10, 2020

#6752

Sry, just found someone has the same problem in merge request.

Ya, It can easy to make a workaround though.

But I think it still needs to init observable. Otherwise, in what situation can we use with skip:true option.

@axelvaindal
Copy link

@ernieyang09 Do you have an effective workaround for this? I'm having this same issue, but useLazyQuery does not return a promise so it's kinda annoying 😕

@ernieyang09
Copy link
Author

@axelvaindal

We just use skip:false cause the api is for read and usage & payload is small.

But I create this for you.

class Deferred {
  constructor() {
    this.promise = new Promise((resolve, reject) => {
      this.reject = reject;
      this.resolve = resolve;
    });
  }
}

export const useImperativeQuery = (query) => {
  const resultRef = useRef();
  const dfd = useMemo(() => new Deferred(), []);

  const [fn, result] = useLazyQuery(query, {
    onCompleted: () => {
      resultRef.current = result.refetch;
      dfd.resolve(result);
    },
  });

  const imperativelyCallQuery = useCallback(
    (variables) => {
      if (!resultRef.current) {
        fn(variables);
        return dfd.promise;
      } else {
        return resultRef.current(variables);
      }
    },
    [dfd.promise, fn]
  );

  return imperativelyCallQuery;
};

@loxosceles
Copy link

Unfortunately, the solution provided by @ernieyang09 didn't work for me. When the query is run, the variables are not passed in and, as a result, it fails.

@ernieyang09
Copy link
Author

ernieyang09 commented Aug 23, 2020

Yeah, thx for mention.
We don't use variables for query and there's a bug.

change

fn(variables);

to

fn({ variables });

Here's example
https://codesandbox.io/s/determined-cannon-sbn3d

@EllaSharakanski
Copy link

EllaSharakanski commented Aug 24, 2020

Upvote. And I think this bug is new, in 3.0.0 everything worked okay, and now in version 3.1.0 I'm getting this error.

My function is:

export type PromiseQuery<TData, TVariables> = [
  (variables: TVariables) => Promise<ApolloQueryResult<TData>>,
  Pick<QueryResult<TData>, 'data' | 'loading' | 'fetchMore'> & { called: boolean },
]

export const usePromiseQuery = <TData, TVariables>(
  query: DocumentNode,
  options?: Omit<UseQueryOptions<TData, TVariables>, 'skip'>,
): PromiseQuery<TData, TVariables> => {
  const [loading, setLoading] = useState(false)
  const data = useRef<TData | undefined>(undefined)
  const [called, setCalled] = useState(false)
  const isMounted = useIsMounted()
  // useQuery does not update loading/data with skip: true, and called is always true.
  const { refetch, fetchMore } = useQuery<TData, TVariables>(query, {
    ...options,
    skip: true,
  })
  return [
    async (variables: TVariables): Promise<ApolloQueryResult<TData>> => {
      setCalled(true)
      setLoading(true)
      let result: ApolloQueryResult<TData> | undefined
      try {
        // Exception is thrown here! 👎 
        result = await refetch(variables)
      } finally {
        data.current = result?.data
        if (isMounted.current) {
          setLoading(false)
        }
      }
      return result
    },
    { loading, data: data.current, called, fetchMore },
  ]
}

@priley86
Copy link

priley86 commented Sep 2, 2020

Setting skip: false does not seem to resolve the issue above for me either. We have a similar query to the one @EllaSharakanski mentions. Refactoring towards @ernieyang09 may be a potential option for us, but will await the feedback here.

@smclaughry
Copy link

We have also had to roll back to 3.1.1 while we wait for this to be fixed.

@benjamn
Copy link
Member

benjamn commented Sep 10, 2020

PR #6999 by @mu29 looks like it might help with this issue, and is now available for testing in @apollo/[email protected], if folks want to give it a try.

@priley86
Copy link

@apollo/[email protected] resolves the issue here. Thanks!

@ernieyang09
Copy link
Author

Fixed in this release.

Thanks to every one.

https://github.com/apollographql/apollo-client/releases/tag/v3.2.0

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 15, 2023
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

8 participants