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.

Sign up for GitHub

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

feat(suspense): return tuple for useSuspenseQuery and useSuspenseInfiniteQuery #5822

Closed
wants to merge 3 commits into from
Closed

feat(suspense): return tuple for useSuspenseQuery and useSuspenseInfiniteQuery #5822

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ac49f4f
feat(suspense): return tuple for useSuspenseQuery and useSuspenseInfi…
TkDodo Aug 1, 2023
375a436
docs
TkDodo Aug 1, 2023
2c36aec
Merge branch 'beta' into feature/suspense-query-tuple
TkDodo Aug 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/react/guides/suspense.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ You can also use the dedicated `useSuspenseQuery` hook to enable suspense mode f
```tsx
import { useSuspenseQuery } from '@tanstack/react-query'

const { data } = useSuspenseQuery({ queryKey, queryFn })
const [data, query] = useSuspenseQuery({ queryKey, queryFn })
```

This has the same effect as setting the `suspense` option to `true` in the query config, but it works better in TypeScript, because `data` is guaranteed to be defined (as errors and loading states are handled by Suspense- and ErrorBoundaries).
Expand Down
12 changes: 7 additions & 5 deletions docs/react/reference/useSuspenseInfiniteQuery.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ title: useSuspenseInfiniteQuery
---

```tsx
const result = useSuspenseInfiniteQuery(options)
const [data, query] = useSuspenseInfiniteQuery(options)
```

**Options**
Expand All @@ -17,7 +17,9 @@ The same as for [useInfiniteQuery](../reference/useInfiniteQuery), except for:

**Returns**

Same object as [useInfiniteQuery](../reference/useInfiniteQuery), except for:
- `isPlaceholderData` is missing
- `status` is always `success`
- the derived flags are set accordingly.
A tuple of `[data, query]`, where:
- `data` is the query data
- `query` is the same query object as returned by [useInfiniteQuery](../reference/useInfiniteQuery), except for:
- `isPlaceholderData` is missing
- `status` is always `success`
- the derived flags are set accordingly.
12 changes: 7 additions & 5 deletions docs/react/reference/useSuspenseQuery.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ title: useSuspenseQuery
---

```tsx
const result = useSuspenseQuery(options)
const [data, query] = useSuspenseQuery(options)
```

**Options**
Expand All @@ -17,7 +17,9 @@ The same as for [useQuery](../reference/useQuery), except for:

**Returns**

Same object as [useQuery](../reference/useQuery), except for:
- `isPlaceholderData` is missing
- `status` is always `success`
- the derived flags are set accordingly.
A tuple of `[data, query]`, where:
- `data` is the query data
- `query` is the same query object as returned by [useQuery](../reference/useQuery), except for:
- `isPlaceholderData` is missing
- `status` is always `success`
- the derived flags are set accordingly.
11 changes: 3 additions & 8 deletions examples/react/nextjs-suspense-streaming/src/app/page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,9 @@ function getBaseURL() {
return 'http://localhost:3000'
}
const baseUrl = getBaseURL()
function useWaitQuery(props: { wait: number }) {
const query = useSuspenseQuery({

function MyComponent(props: { wait: number }) {
const [data] = useSuspenseQuery({
queryKey: ['wait', props.wait],
queryFn: async () => {
const path = `/api/wait?wait=${props.wait}`
Expand All @@ -31,12 +32,6 @@ function useWaitQuery(props: { wait: number }) {
},
})

return [query.data as string, query] as const
}

function MyComponent(props: { wait: number }) {
const [data] = useWaitQuery(props)

return <div>result: {data}</div>
}

Expand Down
14 changes: 6 additions & 8 deletions examples/react/suspense/src/components/Project.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import Spinner from './Spinner'
import { fetchProject } from '../queries'

export default function Project({ activeProject, setActiveProject }) {
const { data, isFetching } = useSuspenseQuery({
const [project, { isFetching }] = useSuspenseQuery({
queryKey: ['project', activeProject],
queryFn: () => fetchProject(activeProject),
})
Expand All @@ -18,13 +18,11 @@ export default function Project({ activeProject, setActiveProject }) {
<h1>
{activeProject} {isFetching ? <Spinner /> : null}
</h1>
{data ? (
<div>
<p>forks: {data.forks_count}</p>
<p>stars: {data.stargazers_count}</p>
<p>watchers: {data.watchers}</p>
</div>
) : null}
<div>
<p>forks: {project.forks_count}</p>
<p>stars: {project.stargazers_count}</p>
<p>watchers: {project.watchers}</p>
</div>
<br />
<br />
</div>
Expand Down
4 changes: 2 additions & 2 deletions examples/react/suspense/src/components/Projects.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,15 @@ import { fetchProjects, fetchProject } from '../queries'

export default function Projects({ setActiveProject }) {
const queryClient = useQueryClient()
const { data, isFetching } = useSuspenseQuery({
const [projects, { isFetching }] = useSuspenseQuery({
queryKey: ['projects'],
queryFn: fetchProjects,
})

return (
<div>
<h1>Projects {isFetching ? <Spinner /> : null}</h1>
{data.map((project) => (
{projects.map((project) => (
<p key={project.name}>
<Button
onClick={() => {
Expand Down
6 changes: 4 additions & 2 deletions packages/react-query/src/useSuspenseInfiniteQuery.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ export function useSuspenseInfiniteQuery<
TPageParam
>,
queryClient?: QueryClient,
): UseSuspenseInfiniteQueryResult<TData, TError> {
return useBaseQuery(
): [TData, UseSuspenseInfiniteQueryResult<TData, TError>] {
const query = useBaseQuery(
{
...options,
enabled: true,
Expand All @@ -44,4 +44,6 @@ export function useSuspenseInfiniteQuery<
InfiniteQueryObserver as typeof QueryObserver,
queryClient,
) as InfiniteQueryObserverSuccessResult<TData, TError>

return [query.data, query]
Copy link
Contributor

@manudeli manudeli Aug 1, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess you want to make library user can rename guaranteed TData easily by using tuple returning like useState?
but I want to mention there is some trade offs.

  1. making difference of interface with useQuery will raise barrier to entry a bit for useSuspenseQuery user.
  2. it is expected to cause a BREAKING CHANGE if we add the enabled option in minor version so that TData cannot be guaranteed.
Suggested change
return [query.data, query]
return query

Could I know why you want to make this change more?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is expected to cause a BREAKING CHANGE if we add the enabled option in minor version so that TData cannot be guaranteed.

not breaking if introduced enabled is true by default (and thus data should always be defined)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we add enabled (which we likely won't), we would do so that not specifying enabled will still result in guaranteed TData, and only if you add it (and make it false), it will be undefined. We do the same with initialData.

This change is mainly about developer ergonomics. With a normal query, you usually want access to data, error, isLoading etc. because you need to handle those states. With useSuspenseQuery, you mainly want access to data, and not the rest of the query.

The entry barrier should be fine, as this is a new hook. Also, TypeScript helps :)

Copy link
Contributor

@manudeli manudeli Aug 1, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is expected to cause a BREAKING CHANGE if we add the enabled option in minor version so that TData cannot be guaranteed.

not breaking if introduced enabled is true by default (and thus data should always be defined)

first of all, I don't think enabled option has been determined to be always true yet. if then, I think we shouldn't make returning type as tuple with hugging chance to make BREAKING CHANGE If enabled can accept a boolean

Also we also have below way that useQuery users are familiar with already too. Isn't it better If we want to access data directly?

const {data: post, ...others} = useSuspenseQuery({
  queryKey: ['post', postId],
  queryFn: () => fetchPost(postId),
})

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to make BREAKING CHANGE If enabled can accept a boolean

not sure what enabled has to do here:

const [data] = useSuspenseQuery({
  queryKey: ['foo'],
  queryFn: () => Promise.resolve('foo')
})

data will be of type string here, while:

const [data] = useSuspenseQuery({
  queryKey: ['foo'],
  queryFn: () => Promise.resolve('foo'),
  enabled: someCondition
})

data will be of type string | undefined

Also we also have below way that useQuery users are familiar with already too.

Implementation detail, but object spread has the disadvantage that it triggers getters on the result object, which interferes with tracked queries, so I usually advise against that: https://tkdodo.eu/blog/react-query-render-optimizations#tracked-queries

Copy link
Contributor

@manudeli manudeli Aug 1, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I worried about same point. if we make enabled option making ReturnType<typeof useSuspenseQuery>[0] can have type TData | undefined

image image

All this cases cannot have advantage of isSuccess flag's type narrowing too.
post: string | undefined

but in this below case
image

we can have advantage of isSuccess flag's type narrowing
post: string

Copy link
Contributor

@manudeli manudeli Aug 1, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@juliusmarminge anyway, I am a big fan of trpc! I am so glad to meet you. I know https://trpc.io/docs/client/react/suspense this one. but I want to keep useSuspenseQuery's return type as just object like useQuery's returning type.

and you can also check situation when we make a @tanstack/react-query's new feature, useSuspenseQuery accepting enabled option in useSuspenseQuery of @suspensive/react-query. so I just don't want to make this useSuspenseQuery meet BREAKING CHANGE because of type narrowing problem.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The isSuccess check is a good point. However, I really doubt that we will add enabled to useSuspenseQuery. enabled has two use-cases:

  1. dependent queries. With suspense, queries are always run in serial, because react suspends on the first query, then runs and continues on the second query. so our dependent queries example literally becomes:
// Get the user
const [user] = useSuspenseQuery({
  queryKey: ['user', email],
  queryFn: getUserByEmail,
})

const userId = user.id

// Then get the user's projects
const [projects] = useQuery({
  queryKey: ['projects', userId],
  queryFn: getProjectsByUser,
})

there is no enabled needed at all for this.

  1. disabling a query from running until a condition is met, like user-input

suspense evolves a lot component composition, so the lazy queries example could become:

function Todos() {
  const [filter, setFilter] = React.useState('')

  return (
      <div>
        // 🚀 applying the filter will enable and execute the query
        <FiltersForm onApply={setFilter} />
        {filter && <TodosTable filter={filter} />
      </div>
  )
}

function TodosTable({ filter }) {
    const [data] = useSuspenseQuery({
      queryKey: ['todos', filter],
      queryFn: () => fetchTodos(filter),
  })
}

Copy link
Contributor

@manudeli manudeli Aug 1, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understood why you don't care enabled. but yet I don't think returning tuple is good way to give better DX.

  1. Quite different interface with useQuery. Should we make useQuery return tuple too? It will make definitely type narrowing problem
  2. Alternative way is quite good too. (Are there any downsides to these ways?)

Alternative way is also good to rename it too.

it's same with useQuery. it's easy. what do tuple have good thing more?

const postQuery = useSuspenseQuery({
  queryKey: ['post', postId],
  queryFn: () => fetchPost(postId),
})

const post = postQuery.data // TData
const {data: post} = useSuspenseQuery({
  queryKey: ['post', postId],
  queryFn: () => fetchPost(postId),
})

post // TData

We can keep simmilar shape with useQuery

const postQuery = useQuery({
  queryKey: ['post', postId],
  queryFn: () => fetchPost(postId),
})

if(postQuery.isSuccess){
  const post = postQuery.data // TData
}

I think it's easier to have a similar shape if it has similar functions.

Are there any downsides of object returning way?

}
6 changes: 4 additions & 2 deletions packages/react-query/src/useSuspenseQuery.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ export function useSuspenseQuery<
>(
options: UseSuspenseQueryOptions<TQueryFnData, TError, TData, TQueryKey>,
queryClient?: QueryClient,
): UseSuspenseQueryResult<TData, TError> {
return useBaseQuery(
): [TData, UseSuspenseQueryResult<TData, TError>] {
const query = useBaseQuery(
{
...options,
enabled: true,
Expand All @@ -23,4 +23,6 @@ export function useSuspenseQuery<
QueryObserver,
queryClient,
) as UseSuspenseQueryResult<TData, TError>

return [query.data, query]
}