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

docs: extend description of handler param for useAsyncData composable #23389

Conversation

MaxKostenko
Copy link
Contributor

@MaxKostenko MaxKostenko commented Sep 24, 2023

πŸ”— Linked issue

resolves #23388

❓ Type of change

  • πŸ“– Documentation (updates to the documentation, readme or JSdoc annotations)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

Some developers use useAsyncData composable for fulfilling pinia store. As a result, the handler returns nothing (undefined) that leads to API request duplication on the client side. I extended the handler param's description to emphasize this feature.

πŸ“ Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

@stackblitz
Copy link

stackblitz bot commented Sep 24, 2023

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@nuxt-studio
Copy link

nuxt-studio bot commented Sep 24, 2023

βœ… Live Preview ready!

Name Edit Preview Latest Commit
Nuxt Docs Edit on Studio β†—οΈŽ View Live Preview e9bdb47

@Atinux
Copy link
Member

Atinux commented Sep 25, 2023

Interesting, thanks for the feedback, why using useAsyncData in that case and not directly using an await to fill your store wrapped with an if statement to check if not already filled?

@MaxKostenko
Copy link
Contributor Author

MaxKostenko commented Sep 25, 2023

First of all, it is not my personal feedback it is one of the questions that people asked at least twice this week in our community.
If you open the official pinia guide you will see:

<script setup>
const store = useStore()
const { data } = await useAsyncData('user', () => store.fetchUser())
</script>

That is correct if fetchUser returns data, but not all developers understand that it must return something.
A lot of people still look at the useAsyncData like full analog asyncData hook from Nuxt2

@posva
Copy link
Collaborator

posva commented Sep 25, 2023

I think that example in the pinia docs is a bit misleading. I will have to think about a different example that makes sense and use that instead. Maybe, not use useAsyncData() at all in that example

@danielroe danielroe merged commit 9da77db into nuxt:main Sep 28, 2023
4 checks passed
This was referenced Sep 28, 2023
@vushe
Copy link

vushe commented Oct 6, 2023

It's quite tricky to understand, how the fetching data from store is supposed to work.

why using useAsyncData in that case and not directly using an await to fill your store wrapped with an if statement to check if not already filled?

Probably because you may want to refresh data and check loading state, for example. useAsyncData() is rather a compact and an elegant way of fetching data, isn't it? Another reason is that it handles adding responses to the Nuxt payload so they can be passed from server to client without re-fetching the data on client side when the page hydrates.

So yes, not a surprise if people prefer fetching from store, having side effects and still utilize useAsyncData() for rendering component. Even more, if you load data directly to component, what's the point in store?

Also when you're rushing to migrate your app from Nuxt 2 to Nuxt 3, you may want to copy approaches used previously (though asyncData hook in Nuxt 2 was supposed to return as well).

I can't say how many people need it, but probably documentation on both Nuxt and Pinia require more clarifications about data fetching from store. If fetching data from store the way I described above isn't recommended, it could be explicitly explained.

@MaxKostenko
Copy link
Contributor Author

MaxKostenko commented Oct 8, 2023

@vushe Nuxt 2 was based on vuex which is always global, so any state set on the server side with vuex was transited to client. So it was ok to fill a store there.

I guess store usage should look like this:

export const useUserStore = defineStore('user', () => {
    const user = ref();
    
    async function getUser() {
         if(user.value) return user;
         
         return await fetchUser();
    }
    
    async function fetchUser() {
         user.value = await $fecth('/api/user');
         return user;
    }

    return {
        user,
        getUser,
        fetchUser
    }
})

<script setup>
const userStore = useUserStore();
await userStore.getUser();
</script>

But I agree this way causes some questions:
Will it block routing like useAsyncData or not?
Depending on the answer we have another question. How do we achieve the opposite behavior?
And the main question is: Why Nuxt3 has too many ways to do the same thing? (For example useFetch doesn't fit any project if it is bigger than a demo IMHO)

In the same time, people who just start with Nuxt very often do something like this:

<script setup>
const name = ref()
const email = ref()

async function saveCustomer() {
       await useFetch(() => `/updateCustomer?name=${unref(name)}&email=${email}`)
}
</script> 

I know that documentation recommends using $fetch. But such code will cause effect that UI will send request after each input event

@vushe
Copy link

vushe commented Oct 9, 2023

Thanks @MaxKostenko! What you wrote is very close to how I understood this, including thoughts about useFetch (which is imho more relevant as an addition on top of the framework, e.g. in a library).

When we fetch from script setup, it is already on client, correct? Just one call then anyway. What if user data is something that should be loaded from route middleware? Then we either need useAsyncData() or we have to manually block second request as mentioned earlier. Is there another way?

In addition, if we elaborate your first example more with try-catch, we can expect known issues.

So this is what I mean when writing, it's a challenge to understand, how fetching is supposed to work from store. Not mentioning problems that e.g. i18n brings when it can't suddenly detect a context from Promise.all().

Yes, if it's a demo, you don't care from where you load data or where you store them. However, a developer of a solid application would be definitely happy to have a clear example instead of guessing.

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.

Docs: explicitly describe that useAsyncData callback must return a value
5 participants