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

Allow useLazyQuery(query, { defaultOptions }) to benefit from defaultOptions.variables #9762

Merged
merged 5 commits into from
May 26, 2022

Conversation

benjamn
Copy link
Member

@benjamn benjamn commented May 24, 2022

After implementing PR #9758, I noticed that useLazyQuery was unable to see/use any defaultOptions.variables passed to useLazyQuery(query, { defaultOptions: { variables }}) (a feature introduced by PR #9563), because the defaultOptions only mattered when the ObservableQuery was first created with client.watchQuery (see #9665), and not when the useLazyQuery execution function is later called. This PR fixes that oversight, allowing useLazyQuery to benefit from options.defaultOptions.variables, and also ensures global client.defaultOptions.watchQuery.variables are merged at the same time.

src/react/hooks/useQuery.ts Outdated Show resolved Hide resolved
@benjamn benjamn requested a review from hwillson May 24, 2022 22:58
@benjamn benjamn changed the title Allow useLazyQuery to benefit from options.defaultOptions.variables Allow useLazyQuery(query, { defaultOptions }) to benefit from defaultOptions.variables and client.defaultOptions.watchQuery.variables May 24, 2022
@benjamn benjamn changed the title Allow useLazyQuery(query, { defaultOptions }) to benefit from defaultOptions.variables and client.defaultOptions.watchQuery.variables Allow useLazyQuery(query, { defaultOptions }) to benefit from defaultOptions.variables May 24, 2022
This makes it easier to see that we pass equivalent/consistent variables
both when initially calling client.watchQuery and when calling
this.observable.reobserve.
@benjamn benjamn force-pushed the fix-useLazyQuery-defaultOptions-merging branch from f80c7fd to 3e59b3d Compare May 25, 2022 18:53
Copy link
Member

@hwillson hwillson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great - thanks @benjamn!

@benjamn benjamn merged commit 1b8449a into main May 26, 2022
@benjamn benjamn deleted the fix-useLazyQuery-defaultOptions-merging branch May 26, 2022 14:54
@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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants