fix(vue-query): Make use mutate reactive to argument changes #4339

matthewhausman · 2022-10-18T19:40:04Z

Definitely need someone to help verify this implementation as I am not intimately familiar with how tanstack query is used professionally, only generally familiar.

codesandbox-ci · 2022-10-18T20:42:49Z

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 830ecfc:

Sandbox	Source
@tanstack/query-example-react-basic-typescript	Configuration
@tanstack/query-example-solid-basic-typescript	Configuration
@tanstack/query-example-vue-basic	Configuration

DamianOsipiuk · 2022-10-19T22:16:29Z

So, from my point of view, there are a lot of unnecessary changes.
How i would see this implemented is to add something like this in types.ts:

// A Vue version of MutationObserverOptions from "@tanstack/query-core"
// Accept refs as options
export type VueMutationObserverOptions<
  TData = unknown,
  TError = unknown,
  TVariables = void,
  TContext = unknown,
> = {
  [Property in keyof MutationObserverOptions<
    TData,
    TError,
    TVariables,
    TContext
  >]: MaybeRefDeep<
    MutationObserverOptions<TData, TError, TVariables, TContext>[Property]
  >
}

and then use it in https://github.com/TanStack/query/blob/main/packages/vue-query/src/useMutation.ts#L23

This will also require change in
https://github.com/TanStack/query/blob/main/packages/vue-query/src/useMutation.ts#L168
and
https://github.com/TanStack/query/blob/main/packages/vue-query/src/useMutation.ts#L183

to use non reffed version of options which is currently at https://github.com/TanStack/query/blob/main/packages/vue-query/src/useMutation.ts#L23

matthewhausman · 2022-10-19T23:07:23Z

@DamianOsipiuk thanks for the feedback gonna start on that now

matthewhausman · 2022-10-20T00:42:33Z

@DamianOsipiuk let me know what ya think! I'd like if functions could also be accepted as refs by tanstack query if possible but what do you think? (hence the change to the deep ref function)

DamianOsipiuk · 2022-10-22T18:40:58Z

packages/vue-query/src/__tests__/useMutation.test.ts

  test('should return error when request fails', async () => {
    const mutation = useMutation(errorMutator)
-
-    mutation.mutate()


I do not think changes in this file are relevant

Ok will take another look - for some reason or another passing no args was failing the tests.

Any idea why these changes would cause something as such?

Actually no idea.
Does it work on main branch?
What TS version do you have installed?

that test is running on your cloud test suite - not my local so not sure if my ts version is relevant

export type MutateFunction<
TData = unknown,
TError = unknown,
TVariables = void,
TContext = unknown,

= (
variables: TVariables,
options?: MutateOptions<TData, TError, TVariables, TContext>,
) => Promise

it seems completely reasonable to get an error for not passing args - variables is not optional. Maybe I somehow removed a Partial somewhere... pretty strange

packages/vue-query/src/useMutation.ts

… potentially

matthewhausman · 2022-10-22T21:57:58Z

I have a separate concern with the current typing of the arguments - I see the most popular way to create reactive arguments to be to make a single options object, so const options = ref({ mutationKey: ..., mutationFn: ... })

currently, we do not allow for such an argument to be passed. I generally think that we should adjust the type signature to something like what I have just committed

export type UseMutationOptions<TData, TError, TVariables, TContext> =
WithQueryClientKey<
MutationObserverOptions<TData, TError, TVariables, TContext>

The mutation options above are all non-reactive

export type VueMutationObserverOptions<
TData = unknown,
TError = unknown,
TVariables = void,
TContext = unknown,

= {
[Property in keyof UseMutationOptions<
TData,
TError,
TVariables,
TContext
]: MaybeRefDeep<
UseMutationOptions<TData, TError, TVariables, TContext>[Property]

}

the Vue options use maybe deep ref on each key....

options: MaybeRef<VueMutationObserverOptions<TData, TError, TVariables, TContext>>,

the object itself is allowed to be reactive

matthewhausman · 2022-10-22T22:12:31Z

Lastly I have tried all of the prettier formatting commands defined in package.json but still getting errors in the workflow tests somehow, any idea how to fix that?

DamianOsipiuk · 2022-10-22T22:17:39Z

Lastly I have tried all of the prettier formatting commands defined in package.json but still getting errors in the workflow tests somehow, any idea how to fix that?

yeah, i think the write script was not updated after the switch to pnpm.

Change pnpm run prettier -- --write to pnpm run prettier --write and then run it and it should work

matthewhausman · 2022-10-22T22:39:24Z

worked like a charm thanks!!!!!

codecov-commenter · 2022-10-22T23:08:17Z

Codecov Report

Base: 96.36% // Head: 92.28% // Decreases project coverage by -4.07% ⚠️

Coverage data is based on head (830ecfc) compared to base (eab6e2c).
Patch has no changes to coverable lines.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4339      +/-   ##
==========================================
- Coverage   96.36%   92.28%   -4.08%     
==========================================
  Files          45       87      +42     
  Lines        2281     3563    +1282     
  Branches      640      904     +264     
==========================================
+ Hits         2198     3288    +1090     
- Misses         80      259     +179     
- Partials        3       16      +13

Impacted Files	Coverage Δ
src/react/Hydrate.tsx
src/core/onlineManager.ts
src/core/retryer.ts
src/core/queriesObserver.ts
src/react/reactBatchedUpdates.ts
src/react/useMutation.ts
src/core/mutation.ts
src/devtools/useLocalStorage.ts
src/core/infiniteQueryObserver.ts
src/devtools/Explorer.tsx
... and 122 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

matthewhausman · 2022-10-22T23:13:02Z

Woooo 🎉🎉🎉🎉🎉🎉🎉 test passed! lmk if you want to zoom or something and go over before we move forward. Thank you so much @DamianOsipiuk for the help it means so much to help with my first open source contribution :)

DamianOsipiuk · 2022-10-22T23:16:42Z

Thanks for the contribution @matthewhausman 🚀

TkDodo · 2022-10-23T05:56:50Z

@allcontributors add @matthewhausman for code

allcontributors · 2022-10-23T05:56:59Z

@TkDodo

I've put up a pull request to add @matthewhausman! 🎉

matthewhausman added 5 commits October 18, 2022 15:09

update

de920b4

Update utils.ts

381b422

Update types.ts

a7f9195

potentail fix to parseArgs typing

20a3ba9

change name of maybeRefArgs

cf78d2f

DamianOsipiuk added the package: vue-query label Oct 18, 2022

matthewhausman added 6 commits October 18, 2022 16:14

Update useMutation.ts

af9d5a2

update

927f5f3

revert some stuff

b5ec820

Update types.ts

f4d0ebf

Update types.ts

972af36

update

046452b

matthewhausman changed the title ~~Make use mutate reactive to argument changes~~ feat(vue-query): Make use mutate reactive to argument changes Oct 18, 2022

matthewhausman changed the title ~~feat(vue-query): Make use mutate reactive to argument changes~~ fix(vue-query): Make use mutate reactive to argument changes Oct 18, 2022

matthewhausman added 3 commits October 18, 2022 16:59

Update types.ts

17731e1

Update types.ts

cc669dd

Update useMutation.ts

918755f

matthewhausman added 4 commits October 19, 2022 18:46

update

e826d02

update

c390809

Update useMutation.ts

5079492

Update useMutation.ts

857e2ce

matthewhausman added 6 commits October 19, 2022 19:13

PR feedback

6a9035b

Update useMutation.ts

7118264

Update types.ts

95864b2

Update useMutation.ts

0a06a18

Update types.ts

097de89

Update types.ts

a4ccbc4

matthewhausman added 2 commits October 19, 2022 19:39

Update useMutation.ts

4976e2e

Update types.ts

ccf20b2

matthewhausman added 6 commits October 19, 2022 20:50

Update useMutation.ts

98904f8

update

41be28a

update fn names

8a19aa1

change subscribe to const

e5fee68

Update types.ts

3c88803

Update useMutation.ts

34ced5c

DamianOsipiuk reviewed Oct 22, 2022

View reviewed changes

matthewhausman added 3 commits October 22, 2022 16:30

wrong arg passed in useMutation, unnecessary changes in testing files…

fab0f73

… potentially

update

e6b1641

update

fab13aa

matthewhausman added 2 commits October 22, 2022 18:06

Update useMutation.test.ts

7f96936

Update useMutation.test.ts

412ab66

update

f879965

Update useMutation.test.ts

9684698

Update useMutation.test.ts

830ecfc

DamianOsipiuk merged commit 915d5e9 into TanStack:main Oct 22, 2022

allcontributors bot mentioned this pull request Oct 23, 2022

add matthewhausman as a contributor for code #4362

Merged

Uh oh!

fix(vue-query): Make use mutate reactive to argument changes #4339

fix(vue-query): Make use mutate reactive to argument changes #4339

Uh oh!

Conversation

matthewhausman commented Oct 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codesandbox-ci bot commented Oct 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DamianOsipiuk commented Oct 19, 2022

Uh oh!

matthewhausman commented Oct 19, 2022

Uh oh!

matthewhausman commented Oct 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DamianOsipiuk Oct 22, 2022

Choose a reason for hiding this comment

Uh oh!

matthewhausman Oct 22, 2022

Choose a reason for hiding this comment

Uh oh!

matthewhausman Oct 22, 2022

Choose a reason for hiding this comment

Uh oh!

DamianOsipiuk Oct 22, 2022

Choose a reason for hiding this comment

Uh oh!

matthewhausman Oct 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matthewhausman Oct 22, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

matthewhausman commented Oct 22, 2022

Uh oh!

matthewhausman commented Oct 22, 2022

Uh oh!

DamianOsipiuk commented Oct 22, 2022

Uh oh!

matthewhausman commented Oct 22, 2022

Uh oh!

codecov-commenter commented Oct 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

matthewhausman commented Oct 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DamianOsipiuk commented Oct 22, 2022

Uh oh!

TkDodo commented Oct 23, 2022

Uh oh!

allcontributors bot commented Oct 23, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

matthewhausman commented Oct 18, 2022 •

edited

Loading

codesandbox-ci bot commented Oct 18, 2022 •

edited

Loading

matthewhausman commented Oct 20, 2022 •

edited

Loading

matthewhausman Oct 22, 2022 •

edited

Loading

codecov-commenter commented Oct 22, 2022 •

edited

Loading

matthewhausman commented Oct 22, 2022 •

edited

Loading