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

remove query option from hooks with two args #10527

Merged
merged 3 commits into from
Feb 15, 2023

Conversation

phryneas
Copy link
Member

@phryneas phryneas commented Feb 7, 2023

Removes the query/mutation/subscription optionfrom hooks that already take that value as their first argument.

These options have not been wired up, but were present in the TS types.

Exception: for useLazyQuery this has been hooked up for a very brief period of time through #10499.
That code has been adjusted to allow changing the query though the execution function, but not via the hook option.

There have not been any tests for this functionality before, so as long as all existing tests keep passing, we should be fine.

Checklist:

  • If this PR contains changes to the library itself (not necessary for e.g. docs updates), please include a changeset (see CONTRIBUTING.md)
  • If this PR is a new feature, please reference an issue where a consensus about the design was reached (not necessary for small changes)
  • Make sure all of the significant new logic is covered by tests

Removes the `query`/`mutation`/`subscription` option
from hooks that already take that value as their first argument.
These options have not been wired up, but were present in the TS types.
@changeset-bot
Copy link

changeset-bot bot commented Feb 7, 2023

🦋 Changeset detected

Latest commit: 27b69ea

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@apollo/client Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@phryneas phryneas requested review from benjamn and alessbell and removed request for benjamn February 7, 2023 11:39
Copy link
Member

@alessbell alessbell left a comment

Choose a reason for hiding this comment

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

Nice fix! LGTM ✅

Copy link
Member

@benjamn benjamn left a comment

Choose a reason for hiding this comment

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

Looks good for release-3.8!

@@ -216,7 +223,6 @@ export interface MutationHookOptions<
TContext = DefaultContext,
TCache extends ApolloCache<any> = ApolloCache<any>,
> extends BaseMutationOptions<TData, TVariables, TContext, TCache> {
mutation?: DocumentNode | TypedDocumentNode<TData, TVariables>;
Copy link
Member

@benjamn benjamn Feb 10, 2023

Choose a reason for hiding this comment

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

I would say the useMutation hook has some important similarities with useLazyQuery, since the caller of the mutation execution function can change the mutation document over time.

However, the options type passed to the mutation execution function is MutationFunctionOptions (defined further above), which still has an optional

mutation?: DocumentNode | TypedDocumentNode<TData, TVariables>;

field. The only options type losing its mutation field is MutationHookOptions.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup, that was the goal here - no mutation field in the hook option, but still a mutation field in the execution function options.
Just to double-check: you just reiterated that here, there was no change for a request, right?

Copy link
Member

@jerelmiller jerelmiller left a comment

Choose a reason for hiding this comment

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

🎉

@alessbell alessbell merged commit 0cc7e2e into release-3.8 Feb 15, 2023
@alessbell alessbell deleted the pr/remove-query-option branch February 15, 2023 19:55
This was referenced Feb 15, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 18, 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

Successfully merging this pull request may close these issues.

4 participants