query icon indicating copy to clipboard operation
query copied to clipboard

`suspense()` does not respect the setting of `useErrorBoundary`

Open Mini-ghost opened this issue 2 years ago • 5 comments

Describe the bug

When we use Nuxt3 with vue-query, we will write code like this:

import { useQuery } from '@tanstack/vue-query';
import { onServerPrefetch } from 'vue';

const fetcher = async () => await $fetch('/api/500');

const { data, error, suspense } = useQuery({
  queryKey: ['TEST'],
  queryFn: fetcher,
});

onServerPrefetch(async () => {
  await suspense();
});

When the api on the server side throws an error, the client shows the error in full screen

截圖 2023-09-07 下午11 18 40

To avoid this issue, we need add catch after suspense() throw any error

onServerPrefetch(async () => {
  await suspense().catch(() => {});
});

According the useErrorBoundary description in TSDoc, the default behavior will not thrown error to the error boundary.

export interface QueryObserverOptions<TQueryFnData = unknown, TError = unknown, TData = TQueryFnData, TQueryData = TQueryFnData, TQueryKey extends QueryKey = QueryKey> extends QueryOptions<TQueryFnData, TError, TQueryData, TQueryKey> {
    /**
     * Whether errors should be thrown instead of setting the `error` property.
     * If set to `true` or `suspense` is `true`, all errors will be thrown to the error boundary.
     * If set to `false` and `suspense` is `false`, errors are returned as state.
     * If set to a function, it will be passed the error and the query, and it should return a boolean indicating whether to show the error in an error boundary (`true`) or return the error as state (`false`).
     * Defaults to `false`.
     */
    useErrorBoundary?: UseErrorBoundary<TQueryFnData, TError, TQueryData, TQueryKey>;
}

Should we need to respect this define in vue-query?

Your minimal, reproducible example

https://stackblitz.com/edit/github-bgz1jd-5nhtyt?file=app.vue

Steps to reproduce

  1. Go to https://stackblitz.com/edit/github-bgz1jd-5nhtyt?file=app.vue
  2. It will shows the error in full screen.

Expected behavior

  1. When useErrorBoundary is undefined or false. suspense() will not throw any error.
  2. When useErrorBoundary is true. suspense() will throw all error to the error boundary.

How often does this bug happen?

Every time

Screenshots or Videos

No response

Platform

System:
    OS: macOS 11.6
    CPU: (8) arm64 Apple M1
    Memory: 90.47 MB / 16.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 18.16.0 - ~/.nvm/versions/node/v18.16.0/bin/node
    npm: 9.5.1 - ~/.nvm/versions/node/v18.16.0/bin/npm
  Browsers:
    Firefox: 116.0.2
    Safari: 14.1.2
    Safari Technology Preview: 15.4
  npmPackages:
    vue: ^3.3.4 => 3.3.4 

Tanstack Query adapter

vue-query

TanStack Query version

v4.35.2

TypeScript version

v5.2.2

Additional context

There have a discussions mention same issue:

  • https://github.com/TanStack/query/discussions/5688

Mini-ghost avatar Sep 07 '23 16:09 Mini-ghost

Well, actually we are in-line with description.

If set to true or suspense is true

So there is an or and we are actually inside suspense. Not the flag, but vue-query does not handle the flag, instead returning suspense Promise which is a replacement for the flag.

But since react-query actually separates suspense to separate hooks, we could change this behavior in v5.

DamianOsipiuk avatar Sep 11 '23 19:09 DamianOsipiuk

Thank you for your response. Can I open a PR for this feature in the beta branch?

Mini-ghost avatar Sep 12 '23 09:09 Mini-ghost

Yup. Would be ideal to also add a test failing for current behavior. Might be tricky since we cannot use component rendering due to Vue2/Vue3 incompatibility.

DamianOsipiuk avatar Sep 12 '23 13:09 DamianOsipiuk

I've implemented a composables API called useSuspenseQuery in my own Nuxt project, and it works wonderfully, functioning similarly to useQuery but is more SSR-friendly.

Initially, we had to write the code this way:

<script setup lang="ts">
import { onServerPrefetch } from 'vue'
import { useQuery } from '@tanstack/vue-query'

const { data, suspense } = useQuery({
  queryKey: ['TODO'],
  queryFn: async () => {
    const res = await fetch('https://jsonplaceholder.typicode.com/todos/1')
    return res.json()
  },
})

onServerPrefetch(async () => {
  await suspense()
})

Developers needed to determine if enabled on the server side to decide whether to execute await suspense().

<script setup lang="ts">
import { onServerPrefetch } from 'vue'
import { useQuery } from '@tanstack/vue-query'

const { data, suspense } = useQuery({
  queryKey: ['TODO'],
  queryFn: async () => {
    const res = await fetch('https://jsonplaceholder.typicode.com/todos/1')
    return res.json()
  },
  enabled: process.client,
})

if(process.client) {
  onServerPrefetch(async () => {
    await suspense()
  })
}

useSuspenseQuery resolves this issue and simplifies usage. When using useSuspenseQuery in Nuxt, developers don’t need to write extra code to achieve SSR-friendly effects, as shown below:

<script setup lang="ts">
import { useSuspenseQuery } from '@tanstack/vue-query'

const { data } = useSuspenseQuery({
  queryKey: ['TODO'],
  queryFn: async () => {
    const res = await fetch('https://jsonplaceholder.typicode.com/todos/1')
    return res.json()
  },
})
<script setup lang="ts">
import { useSuspenseQuery } from '@tanstack/vue-query'

const { data } = useSuspenseQuery({
  queryKey: ['TODO'],
  queryFn: async () => {
    const res = await fetch('https://jsonplaceholder.typicode.com/todos/1')
    return res.json()
  },
  enabled: process.server,
})

This approach simplifies the code. If you need to wait for data loading, just add await before useServerSuspenseQuery.

<script setup lang="ts">
import { useSuspenseQuery } from '@tanstack/vue-query'

const { data } = await useSuspenseQuery({
  queryKey: ['TODO'],
  queryFn: async () => {
    const res = await fetch('https://jsonplaceholder.typicode.com/todos/1')
    return res.json()
  },
  enabled: process.server,
})

// Do something with data

However, useSuspenseQuery is currently coupled with Nuxt, like using useQuery in Nuxt middleware. To make useSuspenseQuery work, it has to rely on Nuxt Hooks.

We can consider some trade-offs:

  1. useSuspenseQuery not depending on Nuxt Hooks might pose issues in middleware, which we could explicitly mention in the documentation.
  2. Design nuxt-query specifically for Nuxt, where currently only useSuspenseQuery needs to be extended, and others can be inherited directly, making the implementation fully reliant on Nuxt Hooks.

Regarding the second point, there's already a Nuxt Module, @hebilicious/vue-query-nuxt, which we could consider implementing useSuspenseQuery in.

I look forward to hearing your thoughts. Thank you.

Mini-ghost avatar Dec 28 '23 01:12 Mini-ghost

@hebilicious How do you feel about this idea? It would be great to hear your opinion!

Mini-ghost avatar Dec 28 '23 01:12 Mini-ghost

@DamianOsipiuk what's the status here please? Seems like your suggestion didn't make it into v5?

TkDodo avatar Feb 17 '24 18:02 TkDodo

@Mini-ghost Sorry for the late answer, I would happily add this to vue-query-nuxt. If you could open a PR with your useSuspenseQuery implementation example I will take a look.

Hebilicious avatar Feb 20 '24 19:02 Hebilicious