nuxt / nuxt

The Intuitive Vue Framework.
https://nuxt.com
MIT License
54.26k stars 4.96k forks source link

Should `useAsyncData` emit warn in dev mode when returning `false` or `''` values? #28096

Closed nicolaspayot closed 2 months ago

nicolaspayot commented 2 months ago

Environment

Reproduction

https://stackblitz.com/edit/nuxt-starter-nhpzcs?file=app.vue

Describe the bug

When returning false or '' from useAsyncData, we get the following warn in dev mode:

 WARN  [nuxt] useAsyncData must return a truthy value (for example, it should not be undefined, null or empty string) or the request may be duplicated on the client side.

The request is indeed duplicated on the client side when returning null or undefined but it is not when returning false or '', as you can see on the reproduction link. Is there anything else that's preventing false/'' to be returned form useAsyncData? Should this warn be displayed only when returning null/undefined?

(I'll happily fix/update this behaviour if it's confirmed)

Additional context

No response

Logs

No response

stackblitz[bot] commented 2 months ago

Fix this issue in StackBlitz Codeflow Start a new pull request in StackBlitz Codeflow.

P4sca1 commented 2 months ago

@danielroe This change introduced the warning for every non-truthy value. It should check explicitly for null | undefined. I propose to only check for undefined. See https://github.com/nuxt/nuxt/discussions/28107.

manniL commented 2 months ago

PR welcome to check only for undefined, see also https://github.com/nuxt/nuxt/pull/27718

danielroe commented 2 months ago

What is your use-case for returning false or ''?

nicolaspayot commented 2 months ago

For example, we have this function from a Pinia store:

    async function isLogged(): Promise<boolean> {
        if (!loggedIn.value) {
            try {
                user.value = await $fetch<User>('/api/user');
                loggedIn.value = true;
            } catch (error: unknown) {
                if (error instanceof FetchError) {
                    if (error.status !== 401 && error.status !== 403) {
                        console.error('No user found', error);
                    }
                }
            }
        }
        return loggedIn.value;
    }

that we use this way in a plugin:

    await useAsyncData('isLogged', () => isLogged(), {dedupe: 'defer'});

I need to wrap it like this if I want to avoid the warn:

    await useAsyncData(
        'isLogged',
        async () => {
            return {isLogged: await isLogged()};
        },
        {dedupe: 'defer'},
    );
danielroe commented 2 months ago

Ah, that is exactly what we are trying to warn about. useAsyncData is not for dispatching side effects. There are situations when it might run at a different time than you are expecting.

nicolaspayot commented 2 months ago

Are you referencing the states assignations in the store through useAsyncData?

But even without this, I assume we could easily find some use-cases as such:

const {data: isLogged} = await useAsyncData(() => $fetch<boolean>('/api/user/logged'));

And then do something depending on isLogged value in a template or a script setup. Is this something we should avoid as well?

P4sca1 commented 2 months ago

What is your use-case for returning false or ''?

A use case for explicitly returning null is to indicate that something does not exist.

// fetchMyPost returns Post | null
const { data: post } = await useAsyncData(() => fetchMyPost(route.params.id))
nicolaspayot commented 2 months ago

Wouldn't you rather return a 404 Not Found for this one?

P4sca1 commented 2 months ago

Depends on the use case. Sometimes you still want to render the page and just don't show the loaded value or indicate that it does not exist.

fabianwohlfart commented 2 months ago

I also encountered it and do not fully understand why I am not allowed to return 0 or false. If I want to fetch the count of posts of category X and display something special for 0 posts I have to go the detour of throwing an error? Would appreciate it very much to get more insights, as "must be truthy" is not so telling for me, something with timing, any examples, use-cases?

Also useFetch docs does not state this (directly), so I was even more confused and searched my code for an abandoned useAsyncFetch until I remembered that useFetch is a wrapper

phoenix-ru commented 1 month ago

Also discovered the same problem out-of-nowhere on this code after doing some dependency updates.

app.vue Source: https://github.com/sidebase/nuxt-auth/blob/de1dca620a88b1cec2b408bbe27b67aacd72c355/playground-authjs/app.vue#L10

const { data: token } = await useFetch('/api/token', { headers })

server/api/token.get.ts https://github.com/sidebase/nuxt-auth/blob/de1dca620a88b1cec2b408bbe27b67aacd72c355/playground-authjs/server/api/token.get.ts#L4

export default eventHandler(event => getToken({ event }))

The endpoint returns null when a user is unauthenticated. What is strange here is that $fetch actually overwrites null to undefined. I couldn't pinpoint the exact cause of it yet, but the behaviour is very consistent. In development it spams logs with

 WARN  [nuxt] useAsyncData must return a value (it should not be undefined) or the request may be duplicated on the client side.

I had to do a manual $fetch return override to work around this: https://github.com/sidebase/nuxt-auth/pull/849/commits/1e2b5a8729695e78244c503b78aabdac00325a14

P.S. Will try to create a reproduction for it