feat(vue): refactor composable functions

yurks commented 2 weeks ago

Summary

This includes https://github.com/urql-graphql/urql/pull/3612, but fixed reactivity loosing here.

In addition, a little refactoring for use* composable functions for reducing code duplication and adding more test cases.

Set of changes

Remove wrapping args in reactive in useQuery.ts and useSubscription.ts
Add createRequestWithArgs(), useClientState() and useRequestState() utils to avoid code duplication.
Refactor useQuery(), useSubscription() and useMutation() to use shared utils mentioned above.
Add isReadonly() check before changing isPaused variable, as it could be readonly.
~~Remove onBeforeUnmount and unwatch handlers, as watcher automatically stopped when the owner component is unmounted~~
Remove flush: 'pre' in watcher options, as this is default setting
Rename unref() to unwrap() to avoid confusions with vue function
Reuse then() handler for useQuery() for cleaner code and avoid duplications
Cover of reactive arguments usage with test cases

changeset-bot[bot] commented 2 weeks ago

🦋 Changeset detected

Latest commit: 24a3889738f46750b2ef6af632ee0ba8cebd7ebe

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

This PR includes changesets to release 1 package

| Name | Type | | --------- | ----- | | @urql/vue | 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

negezor commented 1 week ago

This is a really good refactoring. The only thing that bothers me is why the effects in useClientHandle() were initially manually stopped; I couldn’t find the reason (or I didn’t look well).

yurks commented 1 week ago

Thanks @negezor ☀️

why the effects in useClientHandle() were initially manually stopped

The reason of having useClientHandle() and manually stopping watchers I found is here https://github.com/urql-graphql/urql/pull/3610#issuecomment-2178843530. And the reason it was removed in current PR - I consider this as excessive concern for rare use cases and attempt to bypass vue limitations, which should be completely on users responsibility imho.

But I can agree this kind of removal is overkill for mine "little refactoring", so I'll revert this functionality here. I guess we can found an optimal solution in scope of that PR.

negezor commented 6 days ago

@kitten could you please review the PR?

urql-graphql / urql