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

How to access state variables of a Task/Instance inside the script tag? #64

Closed BenJackGill closed 2 years ago

BenJackGill commented 2 years ago

Thanks for building this! Looks like a great solution for async calls.

But I'm having trouble implementing it.

Here is a simple Vue component using vue-concurrency:

    <template>
      <q-page class="q-ma-lg">
        <div>
          <div v-if="task.isRunning">Loading...</div>
          <div v-else-if="task.isError">{{ task.last?.error.message }}</div>
          <div v-else>
            {{ task.last?.value }}
          </div>
        </div>
      </q-page>
    </template>

    <script setup lang="ts">
    import { timeout, useTask } from 'vue-concurrency';
    const task = useTask(function* () {
      yield timeout(1000);
      if (Math.random() < 0.5) {
        // lets say the API is flaky and errors out often:
        throw new Error('Ruh oh. Something went wrong.');
      } else {
        return 'tada';
      }
    });
    const instance = task.perform();
    // Now I try to access some of the varibles
    console.log(instance.isError); // runs immediatley, always returns "false"
    console.log(instance.error); // runs immediatley, always returns "null"
    console.log(instance.value); // runs immediatley, always returns "null"
    console.log(task.isError); // runs immediatley, always returns "false"
    console.log(task.last?.isError); // runs immediatley, always returns "false"
    console.log(task.last?.error); // runs immediatley, always returns "null"
    console.log(task.last?.value); // runs immediatley, always returns "null"
    </script>

This component makes a fake API call. Pauses for one second. Then will either throw an Error or return the "tada" string.

The current code works in the template, it shows "Loading..." and then either "tada" or the error message.

But I want to access the task and instance variables inside the script tag, and at the moment they all run immediatley without waiting for the async call to execute.

I thought I could fix this by using await task.perform(). Doing so gives me the task.last?.value variable. But the template stops working (becomes blank) and the error variables don't print to the console.

So how do I correctly access the state values of a vue-concurrency Task/Instance inside the script tag?

MartinMalinda commented 2 years ago

It is not the purpose of the task to turn your code from asynchronous to synchronous.

task.perform() will trigger the task and the task indeed runs asynchronously in the background. The same way as if you'd had an async function and called it someFunction().

Oftentimes you don't need to work with the task like this in the <script>. You just perform it and then you rely on reactivity in the template, to render errors, spinners and so on.

await directly in setup() could work but that turns your component into async component and I don't havre much experience with that. At that point you'd probably have to start using Suspense and that's a different approach probably not too compatible with vue concurrency.

If you want to wait for task to finish in the script you can see an example here: https://vue-concurrency.netlify.app/composing-tasks/#async-await-example (async await) or here: https://vue-concurrency.netlify.app/examples/store/ (promises)

BenJackGill commented 2 years ago

Ok thanks I understand better now.

A Task creates one or more Instances, and those Instances are asynchronous. Therefore we need to handle Instances like any other async code (await, try/catch, then/catch, etc).

I've added some code examples below that helped wrap my head around all this, incase it anyone else needs it.

These code examples replace everything below the line // Now I try to access some of the varibles of the opening post. The catch errors have TypeScript types and avoid EsLint errors where possible:

Option 1 - then/catch promises:

instance
  .then((response) => {
    console.log(response); // tada
    console.log(instance.value); // tada
  })
  .catch((err: Error) => {
    console.log(err.message); // 'Ruh oh. Something went wrong.'
    console.log(instance.isError); // true
    console.log(instance.error); // the Error object
    // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
    console.log(instance.error.message); // 'Ruh oh. Something went wrong.'
  });

Option 2 - try/catch using async wrapper:

const wrapper = async () => {
  try {
    await instance;
    console.log(instance.value); // tada
  } catch (err) {
    if (err instanceof Error) {
      console.log(err.message); // 'Ruh oh. Something went wrong.'
    }
    console.log(instance.isError); // true
    console.log(instance.error); // the Error object
    // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
    console.log(instance.error.message); // 'Ruh oh. Something went wrong.'
  }
};
void wrapper();

Option 3 - IIFE:

(async () => {
  await instance;
  console.log(instance.value); // tada
})().catch((err: Error) => {
  console.log(err.message); // 'Ruh oh. Something went wrong.'
  console.log(instance.isError); // true
  console.log(instance.error); // the Error object
  // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
  console.log(instance.error.message); // 'Ruh oh. Something went wrong.'
});

And actually the original code can be modified because we are not calling the Instance multiple times.

A streamlined version could chain the useTask() with perform() which would then return a single Instance (as opposed to the original code which returned a Task and later assigned the Instance to a variable).

That would save some lines but also require us to update the template and remove references to the task:

<template>
  <q-page class="q-ma-lg">
    <div>
      <div v-if="instance.isRunning">Loading...</div>
      <div v-else-if="instance.isError">{{ instance.error.message }}</div>
      <div v-else>
        {{ instance.value }}
      </div>
    </div>
  </q-page>
</template>

<script setup lang="ts">
import { timeout, useTask } from 'vue-concurrency';
const instance = useTask(function* () {
  yield timeout(1000);
  if (Math.random() < 0.5) {
    // lets say the API is flaky and errors out often:
    throw new Error('Ruh oh. Something went wrong.');
  } else {
    return 'tada';
  }
}).perform();
</script>
MartinMalinda commented 2 years ago

This looks good!

Although I'd still try to minimze the usage of the await and then (using tasks as Promises). In my own code I end up doing this here and then but in majority of cases I just define a task and pass it to the template.

In a lot of cases you can have that logic you're now awaiting directly inside the task.

That would save some lines but also require us to update the template and remove references to the task:

Overall it's still beter to pass the whole task to the template.

You can use task.last to refer to the last instance within the task.

<template>
  <q-page class="q-ma-lg">
    <div>
      <div v-if="task.last.isRunning">Loading...</div>
      <div v-else-if="task.last.isError">{{ task.last.error.message }}</div>
      <div v-else>
        {{ task.last.value }}
      </div>
    </div>
  </q-page>
</template>

Now task.last can blow up if the task wasn't performed. So in Vue 3 I usually do task.last?.value instead.

There's also some shortcuts like:

task.isRunning which is a shortcut to task.last.isRunning or even task.isError. So in practice I also use these. But task.last.isRunning is more explicit and less magical and makes newcomers to the codebase understand quicker what's happening.

MartinMalinda commented 2 years ago

BTW - any PRs for documentation improvements are welcome. Maybe there could be a section for "Awaiting tasks"

BenJackGill commented 2 years ago

I would love to help with the docs, because I think this library has great potential, but I'm still trying to get it working on my own app.

I almost have it, but working with Task Creator arguments and state is tricky.

For example, I have followed the Task Creators pattern from the docs like so:

src/composables/tasks.ts

/* eslint-disable @typescript-eslint/no-unsafe-call */
/* eslint-disable @typescript-eslint/no-unsafe-return */
/* eslint-disable @typescript-eslint/no-unsafe-assignment */
import { timeout, useTask } from 'vue-concurrency';

export const useReturnTextTask = (text: string) => {
  return useTask(function* () {
    // Simulate an API call that takes a string argument and returns a string
    yield timeout(1000);
    return text;
  });
};

And I am using it inside a component like so:

src/components/example.vue

<template>
  <input v-model="textInput" type="text" />
  <button @click="handleClick">Click to Perform Task</button>
  <div v-if="returnTextTask.last?.isError">
    {{ returnTextTask.last?.error.message }}
  </div>
  <div v-else>
    {{ returnTextTask.last?.value }}
  </div>
</template>

<script setup lang="ts">
/* eslint-disable @typescript-eslint/no-unsafe-member-access */
/* eslint-disable @typescript-eslint/no-unsafe-assignment */
/* eslint-disable @typescript-eslint/no-unsafe-call */
import { useReturnTextTask } from 'src/composables/test';
import { ref } from 'vue';

const textInput = ref<string>();

const handleClick = async () => {
  if (textInput.value) {
    const returnTextTask = useReturnTextTask(textInput.value);
    const returnTextInstance = returnTextTask.perform();
    await returnTextInstance;
    if (returnTextTask.last?.isSuccessful) {
      console.log(returnTextTask.last?.value);
      console.log(returnTextInstance.value);
    }
  }
};
</script>

Now a problem arises because useReturnTextTask has a required text arugment, so I am using it inside if (textInput.value).

But doing that causes all returnTextTask references in the template to have the error Cannot find name 'returnTextTask'. ts(2304).

I assume this is because the Task is not always defined when it is inside if (textInput.value).

Do you know how to overcome this?

I have also tried using declaring returnTextTask outside handleClick using something like let returnTextTask: Task<string, []>; but that causes runtime errors because it's declared with an undefined value (TypeError: Cannot read properties of undefined (reading 'last')).

BenJackGill commented 2 years ago

Ok so this was fixed by passing in refs and checking for undefined inside the Task generator, like so:

/* eslint-disable @typescript-eslint/no-unsafe-call */
/* eslint-disable @typescript-eslint/no-unsafe-return */
/* eslint-disable @typescript-eslint/no-unsafe-assignment */
import { Ref } from 'vue';
import { timeout, useTask } from 'vue-concurrency';

export const useReturnTextTask = (text: Ref<string | undefined>) => {
  return useTask(function* () {
    if (text.value) {
      console.log('inside Task', text);
      yield timeout(1000);
      return text;
    } else {
      throw new Error('Text required.');
    }
  });
};

And moving the useReturnTextTask() to the top level like so:

<template>
  <input v-model="textInput" type="text" />
  <button @click="handleClick">Click to Perform Task</button>
  <div v-if="returnTextTask.last?.isError">
    {{ returnTextTask.last?.error.message }}
  </div>
  <div v-else>
    {{ returnTextTask.last?.value }}
  </div>
</template>

<script setup lang="ts">
/* eslint-disable @typescript-eslint/no-unsafe-member-access */
/* eslint-disable @typescript-eslint/no-unsafe-assignment */
/* eslint-disable @typescript-eslint/no-unsafe-call */
import { useReturnTextTask } from 'src/composables/test';
import { ref } from 'vue';

const textInput = ref<string>();

const returnTextTask = useReturnTextTask(textInput);

const handleClick = async () => {
  const returnTextInstance = returnTextTask.perform();
  await returnTextInstance;
  if (returnTextTask.last?.isSuccessful) {
    console.log('returnTextTask.last?.value', returnTextTask.last?.value);
    console.log('returnTextInstance.value', returnTextInstance.value);
  }
};
</script>
MartinMalinda commented 2 years ago

I think this issue is more related to JS and composition api in general rather than vue concurrency. I'd go for this approach:

export const useReturnTextTask = () => {
  return useTask(function* (signal, text: string) {
    // Simulate an API call that takes a string argument and returns a string
    yield timeout(1000);
    return text;
  });
};

// later

textTask.perform('foo'); // this will get passed as 2nd param

The approach with ref is OK also but it's more simple and explicit to just pass the value directly.

You could still pass some params to useReturnTextTask() but those would rather be some generic configuration options rather than a direct value used for each task instance.

const task = useReturnTextTask({ timeout: 1000, endpoint: '...' }); // etc
task.perform('ahoy'); // the actual input value that is being used. On each perform, therefore for each instance, it can be different
MartinMalinda commented 2 years ago

BTW

 await returnTextInstance;
  if (returnTextTask.last?.isSuccessful) {
    console.log('returnTextTask.last?.value', returnTextTask.last?.value);
    console.log('returnTextInstance.value', returnTextInstance.value);
  }

There's no need for the if here as in that case the task will always be successful. If it's not successful it throws an error and the code execution stops.

So the proper handling here would be try catch: https://vue-concurrency.netlify.app/handling-errors/#promises-and-async-await

BenJackGill commented 2 years ago

Ok this is making a lot more sense now.

Thanks for sticking with me.

I now have this real world example:

src/composables/auth.ts

export const useSignInWithEmailAndPasswordTask = () => {
  const router = useRouter();
  return useTask(function* (
    signal,
    email: string,
    password: string,
    routeLocation?: RouteLocationRaw
  ) {
    const userCredential: UserCredential = yield signInWithEmailAndPassword(
      auth,
      email,
      password
    );
    if (routeLocation) {
      yield router.push(routeLocation);
    }
    return userCredential;
  });
};

src/components/login.vue

<script setup lang="ts">
import { ref } from 'vue';
import { useSignInWithEmailAndPasswordTask } from 'src/composables/auth';

const email = ref<string>();
const password = ref<string>();

const signInTask = useSignInWithEmailAndPasswordTask();

const handleSubmit = () => {
  if (email.value && password.value) {
    void signInTask.perform(email.value, password.value, {
      name: 'dashboard',
    });
  }
};
</script>

But this brings up a few more questions:

1) Is the use of void appropriate for signInTask.perform() because we are not doing anything with the retured userCredential?

Without void Eslint gives this error:

Promises must be awaited, end with a call to .catch, end with a call to .then with a rejection handler or be explicitly marked as ignored with the 'void' operator. eslint(@typescript-eslint/no-floating-promises)

2) When explicily typing intermediate yielded value should they be typed with a Promise (Promise<UserCredential>) or as the promise value (UserCredential)? I have found that both work, but I'm not sure which is best practise.

And if you have any other tips to improve this pattern I would love to hear it!

MartinMalinda commented 2 years ago
  1. void seems appropriate here, although I've never done that. Probably I'm running older version of typescript-eslint or use some kind of different preset.
  2. Typing the resolved value seems correct as that's the end value you get after yield (or await). You'd get Promise<UserCredential> if you didn't yield.

Otherwise the code looks good!