Skip to content

Conversation

@ant32t
Copy link

@ant32t ant32t commented Jul 5, 2022

I would like to be able to use effectScope

const scope = effectScope()
scope.run(() => {
  const result = useQuery(["todos"], fetchTodoList);
});

@vercel
Copy link

vercel bot commented Jul 5, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
vue-query ✅ Ready (Inspect) Visit Preview Sep 12, 2022 at 4:33PM (UTC)

@codesandbox-ci
Copy link

codesandbox-ci bot commented Jul 5, 2022

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 4048311:

Sandbox Source
DamianOsipiuk/vue-query: basic Configuration
DamianOsipiuk/vue-query: 2.x-basic Configuration
DamianOsipiuk/vue-query: nuxt-simple Configuration

@sonarqubecloud
Copy link

sonarqubecloud bot commented Jul 5, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@DamianOsipiuk
Copy link
Owner

Hi,
I do not think this will work for all the cases, as it relies on the contextSharing feature, which is opt-in.

Would you mind explaining what is the issue you are encountering without this fix? Ex. you are getting some kind of error?

Minimal reproduction example would be helpful to narrow down the issue and find more general solution.

@ant32t
Copy link
Author

ant32t commented Jul 12, 2022

This example doesn't prove the need for effect scopes but demonstrates that it doesn't work.

<script setup lang="ts">
import {effectScope, EffectScope, onScopeDispose, ref, watch, watchEffect} from "vue";
import {useQuery} from "vue-query";

const sources = ref(5);
const newData = ref<string>();

setTimeout(() => sources.value = 1, 1000);

let scope: EffectScope | undefined;

watch(sources, () => {
  scope?.stop();
  scope = effectScope();
  scope.run(() => {
    const { data } = useQuery(["todos"], () => "hello world");
    watchEffect(() => newData.value = data.value)
  });
});

onScopeDispose(() => scope?.stop());
</script>

<template>
  <h1>{{ newData }}</h1>
</template>

here is the first error

Uncaught (in promise) Error: vue-query hooks can only be used inside setup() function.
    useQueryClient vue-query.js:3238
    useBaseQuery vue-query.js:3839
    useQuery vue-query.js:3866
    setup App.vue:16

For the most part using watch, computed etc is fine to do in an effectScope and the error check could be changed to the following.

  if (!getCurrentScope()) {
    throw new Error(
      "vue-query hooks can only be used inside setup() function."
    );
  }

But this results in the following error

Uncaught (in promise) Error: No 'queryClient' found in Vue context, use 'VueQueryPlugin' to properly initialize the library.
    useQueryClient vue-query.js:3241
    useBaseQuery vue-query.js:3837
    useQuery vue-query.js:3864
    setup App.vue:16

Inject assumes that the injected key is provided somewhere in the parent chain. My understanding is that useEffect doesn't have a parent scope meaning it won't find the provided data.

I'm wondering if it would be ok to store the queryClients in a global object instead of using vue's inject provide?

@DamianOsipiuk
Copy link
Owner

So, skipping the fact that i do not see/understand the real use-case of enclosing useQuery in an effectScope it definitely works, when run synchronously inside setup function. Ex:

let query;
const scope = effectScope();
scope.run(() => {
  query = reactive(useQuery(["posts"], fetcher));
});

return { query };

It does not work, when useQuery is called in some callback function that is run after setup function finished running, ex watch.
And that is expected behavior.
You are correctly getting the error, which states that you are not running useQuery in setup, cause you are running it in callback that is invoked after setup.

useQuery is tied to the component instance and registers listeners on the queryCache to be notified about the changes on the queryKey.
In case you need to switch, which query you are listening to dynamically, you do not need to dispose the useQuery, just change the queryKey via ref/computed and observer will do the rest for you.

In case you want to hack your way to make useQuery be usable outside of the component scope, I think you would be better off to build your solution on top of react-query/core, as it will be much more flexible for you.

@ant32t
Copy link
Author

ant32t commented Jul 15, 2022

I agree that this pull request is not elegant and I have not presented a valid example where it is needed. From my perspective if people use effect scopes they might want to use useQuery inside of them. #179 appears to implement most of what is needed for using Vue Query in effect scopes and also mentions Pinia. I haven't worked with Pina so I'm unsure if people using both Pina and Vue Query would benefit from being able to use useQuery inside of a detached effect scope.

In our project we have an array of refs that sometimes changes based on user input or a change on the backend. When that happens we create a new effect scope and setup a few new watchers and computed values and stop the old effect scope. I would love to use useQuery inside of it. It works great if I use the hack in this pull request.

@ant32t
Copy link
Author

ant32t commented Jul 19, 2022

Here's a different approach of getting the query client from within effect scope outside of setup.
ant32t@e5ba77e

@DamianOsipiuk
Copy link
Owner

So, i have thought about this and i guess that this can cause problem when you will have multiple applications instances on the page, but you do not want to share the instance between those.

I think what we could do to eventually support this is to add ability to pass queryClient to useQuery as an option.
Then internally instead of use useQueryClient, just use provided client from options.

This way you would be able to useQueryClient in setup and pass it to useQuery in the created scope.

Ofc we would need to do the same modification in all composables.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@ant32
Copy link

ant32 commented Sep 13, 2022

@DamianOsipiuk I updated the pull to allow passing in the query client. I simply searched for places useQueryClient was used and updated them to first look for queryClient in the options. I did a small test in our code base and it works great.

I was considering renaming type WithQueryClientKey to WithQueryClient.

Another thought I had was to merge queryClientKey and queryClient and inside of useQueryClient just check if id was an Object then return with that object otherwise continue as before and get the query client using vue inject.

@codecov
Copy link

codecov bot commented Sep 15, 2022

Codecov Report

Base: 100.00% // Head: 100.00% // No change to project coverage 👍

Coverage data is based on head (4048311) compared to base (111167c).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff            @@
##              beta      #203   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           14        14           
  Lines          377       378    +1     
  Branches        64        69    +5     
=========================================
+ Hits           377       378    +1     
Impacted Files Coverage Δ
src/vuejs/useBaseQuery.ts 100.00% <100.00%> (ø)
src/vuejs/useIsFetching.ts 100.00% <100.00%> (ø)
src/vuejs/useIsMutating.ts 100.00% <100.00%> (ø)
src/vuejs/useMutation.ts 100.00% <100.00%> (ø)
src/vuejs/useQueries.ts 100.00% <100.00%> (ø)

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.

@DamianOsipiuk DamianOsipiuk merged commit 1d2aec9 into DamianOsipiuk:beta Sep 15, 2022
@DamianOsipiuk
Copy link
Owner

@ant32t Thanks for the contribution! 🚀

@github-actions
Copy link

🎉 This PR is included in version 2.0.0-beta.11 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants