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

Allow useTask to be used outside of setup #1

Closed MartinMalinda closed 3 years ago

MartinMalinda commented 4 years ago

This could be useful for testing.

It seems the only blocker for this is the usage of onUnmounted() which would cause an error if used outside of setup. It could be conditionally turned off so that useTask() can be used anywhere.

 const mockTask = useTask(function*() {
      return "success";
    }, { cancelOnUnmount: false });
  const wrapper = mount(SaveButton, {
    propsData: {
      task,
    },
  });

  task.perform();
  expect(wrapper.find("button")).toBeDisabled();
 });
pnaylor commented 4 years ago

I was hoping to be able to use Tasks in composables, seems like it could be very helpful.

MartinMalinda commented 4 years ago

@pnaylor a code example would be useful here for me to understand this usage 🙏

so far I could get away with declaring tasks in the root of setup() and then I performed them later inside other hooks if needed

pnaylor commented 4 years ago

My thought was something like a getCollectionTask in this use-collection composable:

import { readonly, ref, Ref } from 'vue'
import { DB, FirebaseDocument } from '@/firebase-config'

/*
    Query for documents from a DB collection

    initialize with name/path for collection to reference
    use helpers to query and switch collections
*/

const debug = false

type QueryOptions = {
    query?: {
        field: string
        opStr: firebase.firestore.WhereFilterOp
        value: string
    } // query string, see firebase documentation
    orderBy?: string // order results, see firebase documentation
    limit?: number // number of objects to return
}

export default function (collectionName?: string) {
    // state
    const docs: Ref<FirebaseDocument[]> = ref([]) // query results
    const error: Ref<Error | null> = ref(null)
    const isLoading = ref(false)

    let collection = collectionName ? collectionName : ''

    // functions

    const setCollection = (collectionName: string) => {
        if (debug) console.log('setCollection triggered', collectionName)
        collection = collectionName
    }

    const getCollection = (
        { query, limit, orderBy }: QueryOptions = {},
        callback?: Function,
    ) => {
        if (debug)
            console.log('getCollection triggered', { query, limit, orderBy })
        isLoading.value = true
        docs.value = []
        error.value = null

        let theQuery = query
            ? DB.collection(collection).where(
                  query.field,
                  query.opStr,
                  query.value,
              )
            : DB.collection(collection)

        theQuery = limit ? theQuery.limit(limit) : theQuery
        theQuery = orderBy ? theQuery.orderBy(orderBy) : theQuery

        theQuery
            .get()
            .then((querySnapshot) => {
                const resultArray: FirebaseDocument[] = []
                if (debug) console.log('got collection docs: ')

                querySnapshot.forEach((doc) => {
                    if (debug) console.log(doc.data())
                    resultArray.push({ id: doc.id, ...doc.data() })
                })
                docs.value = resultArray
                error.value = null
                if (callback) {
                    callback()
                }
            })
            .catch((error) => {
                if (debug) console.log('Error getCollection: ', error)
                error.value = error
            })
            .finally(() => {
                isLoading.value = false
            })
    }

    return {
        // state
        docs: readonly(docs),
        isLoading: readonly(isLoading),
        error: readonly(error),

        // functions
        getCollection,
        setCollection,
    }
}

If it were declared in a component setup, would it only be available when that component is mounted? I am pretty new with Vue still, so maybe I am not thinking of this in the best way yet either. Appreciate the feedback.

MartinMalinda commented 4 years ago

@pnaylor I'm not sure which library is this from, but yes this function is not very compatible with vue-concurrency because it returns its own refs.

If it only returned the theQuery.get() which is a Promise then you can yield (await) that Promise in a Task.

const task = useTask(function * () {
 const someFirebaseData = yield getCollection('foo');
});

So it would be the other way around - Task would be the top level construct and you'd use the firebase utils inside.

I'm not 100% sure if it's the case here, but if you're dealing with streams which is common with Firebase that's a pattern that's hardly compatible with tasks in vue-concurrency. Streams continuously yield values. Tasks can have many values too but the trigger for getting a next value is explicit via perform() (user action and other triggers often happening on the client), while for Streams (websockets) the trigger is kind of outside our control (data changing on the server) and we only listen to it.

Observables and other subscription based things are better fit for this.

Hope this helps!

MartinMalinda commented 4 years ago

It seems like useTask can be used outside of setup in Vue 3, although it produces an onUnmounted is called when there is no active component warning.

pnaylor commented 4 years ago

Interesting, I guess you would just have to deal with the cleanup manually then?

Was also wondering if defining a task in a component would mean it can only be reused between related components within the same page and not between pages, as it would only exist while the given page is mounted? Unless it is declared in the main App component I guess?

Appreciate the feedback and the great library!

MartinMalinda commented 4 years ago

@pnaylor normally vue-concurrency does task.cancelAll when the component is unmounted. Which is usually what you want to happen. But I can imagine that if you would import a task directly from somewhere - not a function that returns a task, but rather a task directly - that in that scenario maybe you don't want the task to cancel if the component is unmounted.

You could create a task right away, then perform it from anywhere and import wherever needed. That gives you shared state. In theory you could then use tasks as a source of long-term state. But I'm not sure if it's a good idea. I can imagine some scenarios where it is useful. But I would still stick to passing tasks via props or using other state management solutions (Pinia, VueX, XState) on the side when it makes sense.

If you'd submit a form and left the page and returned, the form might still be in the loading state because the request is long pending for example. Usually it's better UX to cancel and try again.

refayathaque commented 3 years ago

This looks like a very promising library that I can use to solve some concurrency issues we're having @MartinMalinda ! Thank you so much for your thought and effort behind it :) Our application is using the Options API so I'm having a hard time refactoring to Composition API to be able to use this, is there anything you can propose that would allow me to leverage this library but not change my Options API-based code?

rudolfbyker commented 3 years ago

This would very useful for using Tasks inside Vuex. Yes, Vuex is a state management system, and you could store a promise there, store its value when it resolves, store its error when it rejects, etc. etc. But all of that boilerplate has to be written for every asynchronous thing you do from within Vuex. vue-concurrency's Tasks already does all of this. All that is needed is to disable the automatic cancellation, since Vuex is global.

MartinMalinda commented 3 years ago

This should now be released in 2.1.0. I haven't done thorough tests on how tasks behave outside of components but in theory, there should be no problems at all.

useTask() can just be called anywhere. You can also do useTask(function * () {}, { cancelOnUnmount: false }) but that actually makes sense only if you call it in a component in those rare cases when you don't wanna abort on unmounting.

rudolfbyker commented 3 years ago

Does this mean that this module no longer depends on the composition API at all?

MartinMalinda commented 3 years ago

@rudolfbyker no, it still stands on top of composition api. It's just that just like ref and computed can be used outside of the setup function, so can tasks now

dschmidt commented 2 years ago

This issue is still referenced in the docs, which say this would still be open :)

Unfortunately pasting the url somehow doesn't work in my phone where I'm currently reading the docs... 🙄 but it's ln the testing section

MartinMalinda commented 2 years ago

@dschmidt Thanks for the reminder. I'll try to check the testing workflows with cancelOnUnmount: false and update that docs page and then close this issue 👍

https://vue-concurrency.netlify.app/testing/