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

Merge mutation variables from base options and function invocation #6927

Merged
merged 4 commits into from
Sep 14, 2020
Merged

Merge mutation variables from base options and function invocation #6927

merged 4 commits into from
Sep 14, 2020

Conversation

amannn
Copy link
Contributor

@amannn amannn commented Aug 28, 2020

Checklist:

  • 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

Fixes #6619

/**
* Merges the provided objects shallowly and removes
* all properties with an `undefined` value
*/
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added this comment for clarity.

},
result: { data: expectedData },
},
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test actually detected the behaviour for HOCs. Since this is now fixed, I'v adapted the test.

I also added another test specifically for the hooks version.

@amannn
Copy link
Contributor Author

amannn commented Sep 14, 2020

@hwillson would you consider this PR? From the discussion in #6619 this sounds like it should be fixed, right?

@hwillson
Copy link
Member

@amannn we're definitely considering this PR, thanks very much for working on it! We'll post back with an update shortly.

@amannn
Copy link
Contributor Author

amannn commented Sep 14, 2020

@hwillson That's great, thanks!

@benjamn benjamn changed the base branch from main to release-3.2 September 14, 2020 17:33
@benjamn benjamn added this to the Post 3.0 milestone Sep 14, 2020
@benjamn benjamn changed the base branch from release-3.2 to release-3.3 September 14, 2020 18:55
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.

LGTM with a few changes! Thanks for getting this started, and especially for adding tests, @amannn. These changes will be available to test in the first @apollo/client@3.3.0 beta release (see #7015).

@benjamn benjamn merged commit 0960b8e into apollographql:release-3.3 Sep 14, 2020
@amannn
Copy link
Contributor Author

amannn commented Sep 15, 2020

@benjamn Thank you for your help!

@amannn amannn changed the title fix: Merge mutation variables from base options and function invocation Merge mutation variables from base options and function invocation Oct 8, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 1, 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.

Variables set on mutate function override one set on useMutation hook
3 participants