MartinMalinda / vue-concurrency

A library for encapsulating asynchronous operations and managing concurrency for Vue and Composition API.
https://vue-concurrency.netlify.app/
MIT License
352 stars 15 forks source link

Question: pinia store containing task? #71

Open shaunc opened 2 years ago

shaunc commented 2 years ago

Newbie question -- reading through your documentation -- in particular regarding use with store.

If I have a multiple components that depend on the same data loaded asynchronously, won't it be better to put the task itself in the store? Doing it the way you suggest will mean that one component will have access to task state, but for others the data just appears magically, or is missing. If they also request via useApiTask, there could be multiple overlapping requests for the same data.

(But perhaps I'm missing something.)

MartinMalinda commented 2 years ago

Good question.

data just appears magically, or is missing

This could happen if the task did not cancel. By default it does cancel though so once you leave a route the logic stops and the state just wont end up in the store.

But in some cases you do want to have some logic not to cancel when user leaves the route or perhaps the same should be used in parallel in multiple different places (getCurrentUserTask for example). Firing the async operation multiple times unecessarily is not the best approach.

So far in my experience it was enough to just have these "global" operations, like fetching current user in the main App/Layout component. I don't assign an entire task to the store, just the last instance.

// App.vue setup
const sessionStore = useSessionStore();
const fetchCurrentUserTask = useTask(function * () {  });
sessionStore.currentUserPromise = fetchCurrentUserTask.perform();

Then elsewhere I can do await sessionStore.currentUserInstance if I need to wait until currentUser loads or I can use some getter for sessionStore.currentUser which checks sessionStore.currentUserInstance.value.

I feel like there's a room for improvement here. I was thinking it would be awesome if there could be some kind of "functional" wrapper for stores, which could basically allow them to have their own setup function. Then the task could just live side by side with the store.

export const useSessionStore = defineStore({
 state: () => ({}),
 getters: {},
 setup(state) {
    // run once the store is imported somewhere for the first time, probably right during the start of the app
   state.myTask = useTask(function * () {});
  }
});

Maybe this could be done via some kind of custom extension/wrapper of Pinia.

One downside I can see here is SSR. The whole state transfer from state to client could cause problems. It would most liely break the task, as it's not just plain state, but a custom object with functions etc.

shaunc commented 2 years ago

Thanks for the explanation and thoughts. What I was envisioning was similar to your setup, but in lieu of changing pinia, have a way to use a task idempotently. So in pinia you'd have the task:

export const useSessionStore = defineStore({
 state: () => {
    const task = useTask(function * () {})
    return { task }
  },
  actions: {
    load: function () {
      this.task.performOnce()
    }
  }   
});

Then in the components:

...
setup () {
  const store = useSessionStore()
  onMounted() {
    store.load()
  }
  ...
}
...

Where performOnce would ensure that, even if load was called in several components, underlying perform would only be called once. (Of course we could also have another refresh action that simply called perform for explicit reload of state.)

MartinMalinda commented 2 years ago

@shaunc sorry for late answer

Your code example certainly looks interesting and it could be great for certain usecases. I don't have a clear answer how to make this work. You can set cancelOnUnmount to false on the Task options. That way you can create a task there without any warnings or errors. But there might be other issues I can't foresee right now.

MartinMalinda commented 2 years ago

https://pinia.vuejs.org/core-concepts/#setup-stores

MartinMalinda commented 9 months ago

I think there's two approaches to approach this:

A) Actions in pinia store can effectively work as a task creator function: https://vue-concurrency.netlify.app/composing-tasks/#task-creators.

const myStore = useMyStore();
const getUserTask = myStore.getUser();
getUserTask.perform('someUserId').then(user => ...); // store returns user and also does whatever it wants with it before

Because in this way the task is created in setup of the component, it's bound to the component instance.

B) Tasks live directly inside a setup store You use https://pinia.vuejs.org/core-concepts/#setup-stores Here the tasks would be created once during the initialization of the store. They are not bound to any particular component. This means they keep running and won't get canceled on route change etc. This might be useful for some global operations, such as fetching a current user / session or fetching locale data.

const sessionStore = useSessionStore();
console.log(sessionStore.currentUserTask.isRunning); // the task and its instances are shared in all components that use the store
sessionStore.currentUserTask.last.then(user => console.log('now the user is available').catch(e => console.error('could not fetch current user');

I haven't used this approach so far. If anyone experiments with this, feel free to reach out.

wongpeiyi commented 4 months ago

Hi @MartinMalinda I believe I'm using your approach B above and it works great, especially coming from ember-concurrency. However I'm getting the following warning:

Task instance has been created in inactive scope. Perhaps youre creating task out of setup?

I created a minimal repro here: https://github.com/wongpeiyi/vue-concurrency-setup-store/commit/730bf1a3aa4d612857c4314f39a662bc55509651 Somewhat new to vue, so let me know if I'm doing anything wrong, or perhaps the warning needs to account for Pinia setup stores?

MartinMalinda commented 4 months ago

@wongpeiyi ah you're right, the log is definitely annoying in this scenario :/

https://github.com/vuejs/pinia/issues/2712

Let's see if there's a way to detect usage in pinia, in that case I could turn off the warning.

If not I have an option to

A) Turn off this warning altogether B) Don't issue this warning if cancelOnUnmount is false. So this option would have to be set on the "pinia tasks"

MartinMalinda commented 4 months ago

I've tried using getCurrentScope and onScopeDispose but I'm getting some infinite reactivity recursions. I'll have to try again sometimes later.