Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature request: hooks for data fetch #189

Closed
jbaubree opened this issue Feb 9, 2023 · 5 comments · Fixed by #190
Closed

Feature request: hooks for data fetch #189

jbaubree opened this issue Feb 9, 2023 · 5 comments · Fixed by #190

Comments

@jbaubree
Copy link
Contributor

jbaubree commented Feb 9, 2023

Hi @logaretm,

First of all, this library is great job.

Clear and concise description of the problem

When we use fetching APIs like fetch, axios or villus, we need almost all the time to work with data, specially setting mapping response data.
Currently, the only solution i found to do that with villus is using watcher and i find this not optimize:

const { data, isDone } = useQuery({
  query: myQuery,
  variables: ...
})
watch(isDone, (value), {
  if (value && data.value)
    mapAndSetMyData(data.value)
})

Suggested solution

Add hooks like onSuccess, onError like this is setup on Apollo

I think this is a small feature that will help a lot.
What do you think about it ?

@jbaubree
Copy link
Contributor Author

jbaubree commented Feb 9, 2023

Oh, i didn't see then function was provided, this is a solution. Feel free to close this issue if you think onSuccess and onError is too much for this library but i think it's important to have a specific hooks for error and success

@jbaubree
Copy link
Contributor Author

Update: onFulfilled function in then is only triggered on first call so it does not work with computed variables etc...

@jbaubree
Copy link
Contributor Author

jbaubree commented Jun 7, 2023

@logaretm Do you think we can do the same thing for mutations ?

@logaretm
Copy link
Owner

logaretm commented Jun 7, 2023

@jbaubree Usually mutations are executed explicitly, so I don't think it is as common as queries but I don't see the harm. Feel free to PR it, and I guess you ran into needing it recently, would love to hear about it as well, use-case wise.

@jbaubree
Copy link
Contributor Author

jbaubree commented Jun 8, 2023

@logaretm There are several reasons why I think it is necessary.
Let's take a simple example:
We are in a login page.

const { execute: signIn } = useMutation<{ signIn: { token: string } }>(SIGN_IN)

async function onSubmit() {
  signIn({ publicAddress, signature }).then(({data, error}) => {
    if (data) <-- Here we don't know if mutation is a success and i find data/error conditions are pains
      //...
    if (error)
      //...
  }
}

Second reason, i find code more readable like this:

const { execute: signIn } = useMutation<{ signIn: { token: string } }>(SIGN_IN, {
  onData(data) {
    setToken(data.signIn.token) <--- We are sure that data exists here
    router.push({ name: 'Root' })
  },
  onError(err) {
    ...
  },
})

async function onSubmit() {
  signIn({ publicAddress, signature })
}

If you confirm that you are ok with that, i will PR it when i will get some free time

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants