Skip to content

Commit

Permalink
Await component (#11936)
Browse files Browse the repository at this point in the history
This PR adds an `<Await />` component, that allows to trigger `Suspense` ___inside the component___

```tsx
const {promise} = useQuery({queryKey: ['data'], queryFn: fetchData})

// Will trigger the suspense within this component
// And once the promise resolves, call the children with the data
// unlike regular approach like `return isLoading ? <Loader /> : <div>{data}</div>`
// it doesn't lead to waterfall of loaders and tell React to keep the tree suspended
// So we always have a single loader, which improves the UX
<Await promise={promise}>
{(data) => <div>{data}</div>}
</Await>
```
  • Loading branch information
MrFlashAccount authored Dec 25, 2024
1 parent 6fe253f commit 0c41e8d
Show file tree
Hide file tree
Showing 10 changed files with 310 additions and 62 deletions.
8 changes: 4 additions & 4 deletions app/common/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,12 @@
"lint": "eslint ./src --cache --max-warnings=0"
},
"peerDependencies": {
"@tanstack/query-core": "5.54.1",
"@tanstack/vue-query": ">= 5.54.0 < 5.56.0"
"@tanstack/query-core": "5.59.20",
"@tanstack/vue-query": "5.59.20"
},
"dependencies": {
"@tanstack/query-persist-client-core": "^5.54.0",
"@tanstack/vue-query": ">= 5.54.0 < 5.56.0",
"@tanstack/query-persist-client-core": "5.59.20",
"@tanstack/vue-query": "5.59.20",
"lib0": "^0.2.85",
"react": "^18.3.1",
"vitest": "^1.3.1",
Expand Down
7 changes: 7 additions & 0 deletions app/common/src/queryClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,13 @@ export function createQueryClient<TStorageValue = string>(
networkMode: 'always',
refetchOnReconnect: 'always',
staleTime: DEFAULT_QUERY_STALE_TIME_MS,
// This allows to prefetch queries in the render phase. Enables returning
// a promise from the `useQuery` hook, which is useful for the `Await` component,
// which needs to prefetch the query in the render phase to be able to display
// the error boundary/suspense fallback.
// @see [experimental_prefetchInRender](https://tanstack.com/query/latest/docs/framework/react/guides/suspense#using-usequerypromise-and-reactuse-experimental)
// eslint-disable-next-line camelcase
experimental_prefetchInRender: true,
retry: (failureCount, error: unknown) => {
const statusesToIgnore = [403, 404]
const errorStatus =
Expand Down
6 changes: 3 additions & 3 deletions app/gui/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,8 @@
"@sentry/vite-plugin": "^2.22.7",
"@stripe/react-stripe-js": "^2.7.1",
"@stripe/stripe-js": "^3.5.0",
"@tanstack/react-query": "5.55.0",
"@tanstack/vue-query": ">= 5.54.0 < 5.56.0",
"@tanstack/react-query": "5.59.20",
"@tanstack/vue-query": "5.59.20",
"@vueuse/core": "^10.4.1",
"@vueuse/gesture": "^2.0.0",
"ag-grid-community": "^32.3.3",
Expand Down Expand Up @@ -155,7 +155,7 @@
"@storybook/test": "^8.4.2",
"@storybook/vue3": "^8.4.2",
"@storybook/vue3-vite": "^8.4.2",
"@tanstack/react-query-devtools": "5.45.1",
"@tanstack/react-query-devtools": "5.59.20",
"@testing-library/jest-dom": "6.6.3",
"@testing-library/react": "16.0.1",
"@testing-library/react-hooks": "8.0.1",
Expand Down
158 changes: 158 additions & 0 deletions app/gui/src/dashboard/components/Await.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,158 @@
/**
* @file
*
* Await a promise and render the children when the promise is resolved.
*/
import { type ReactNode } from 'react'

import invariant from 'tiny-invariant'
import { ErrorBoundary, type ErrorBoundaryProps } from './ErrorBoundary'
import { Suspense, type SuspenseProps } from './Suspense'

/**
* Props for the {@link Await} component.
*/
export interface AwaitProps<PromiseType>
extends Omit<SuspenseProps, 'children'>,
Omit<ErrorBoundaryProps, 'children'> {
/**
* Promise to await.
*
* ___The promise instance ***must be stable***, otherwise this will lock the UI into the loading state___
*/
readonly promise: Promise<PromiseType>
readonly children: ReactNode | ((value: PromiseType) => ReactNode)
}

/**
* State of the promise.
*/
export type PromiseState<T> =
| {
readonly status: 'error'
readonly data?: never
readonly error: unknown
}
| {
readonly status: 'pending'
readonly data?: never
readonly error?: never
}
| {
readonly status: 'success'
readonly data: T
readonly error?: never
}

/**
* Awaits a promise and render the children when the promise resolves.
* Works well with React Query, as it returns a cached promise from the useQuery hook.
* Useful to trigger Suspense ***inside*** the component, rather than ***outside*** of it.
* @example
* const {promise} = useQuery({queryKey: ['data'], queryFn: fetchData})
*
* <Await promise={promise}>
* {(data) => <div>{data}</div>}
* </Await>
*/
export function Await<PromiseType>(props: AwaitProps<PromiseType>) {
const {
promise,
children,
FallbackComponent,
fallback,
loaderProps,
onBeforeFallbackShown,
onError,
onReset,
resetKeys,
subtitle,
title,
} = props

return (
<ErrorBoundary
FallbackComponent={FallbackComponent}
onError={onError}
onBeforeFallbackShown={onBeforeFallbackShown}
onReset={onReset}
resetKeys={resetKeys}
subtitle={subtitle}
title={title}
>
<Suspense fallback={fallback} loaderProps={loaderProps}>
<AwaitInternal promise={promise} children={children} />
</Suspense>
</ErrorBoundary>
)
}

const PRIVATE_AWAIT_PROMISE_STATE = Symbol('PRIVATE_AWAIT_PROMISE_STATE_REF')

/**
* Internal implementation of the {@link Await} component.
*
* This component throws the promise and trigger the Suspense boundary
* inside the {@link Await} component.
* @throws {Promise} - The promise that is being awaited by Suspense.
* @throws {unknown} - The error that is being thrown by the promise. Triggers error boundary inside the {@link Await} component.
*/
function AwaitInternal<PromiseType>(props: AwaitProps<PromiseType>) {
const { promise, children } = props

/**
* Define the promise state on the promise.
*/
const definePromiseState = (
promiseToDefineOn: Promise<PromiseType>,
promiseState: PromiseState<PromiseType>,
) => {
// @ts-expect-error: we know that the promise state is not defined in the type but it's fine,
// because it's a private and scoped to the component.
promiseToDefineOn[PRIVATE_AWAIT_PROMISE_STATE] = promiseState
}

// We need to define the promise state, only once.
// We don't want to use refs on state, because it scopes the state to the component.
// But we might use multiple Await components with the same promise.
if (!(PRIVATE_AWAIT_PROMISE_STATE in promise)) {
definePromiseState(promise, { status: 'pending' })

// This breaks the chain of promises, but it's fine,
// because this is suppsed to the last in the chain.
// and the error will be thrown in the render phase
// to trigger the error boundary.
void promise.then((data) => {
definePromiseState(promise, { status: 'success', data })
})
void promise.catch((error) => {
definePromiseState(promise, { status: 'error', error })
})
}

// This should never happen, as the promise state is defined above.
// But we need to check it, because the promise state is not defined in the type.
// And we want to make TypeScript happy.
invariant(
PRIVATE_AWAIT_PROMISE_STATE in promise,
'Promise state is not defined. This should never happen.',
)

const promiseState =
// This is safe, as we defined the promise state above.
// and it always present in the promise object.
// eslint-disable-next-line no-restricted-syntax
promise[PRIVATE_AWAIT_PROMISE_STATE] as PromiseState<PromiseType>

if (promiseState.status === 'pending') {
// Throwing a promise is the valid way to trigger Suspense
// eslint-disable-next-line @typescript-eslint/only-throw-error
throw promise
}

if (promiseState.status === 'error') {
throw promiseState.error
}

return typeof children === 'function' ? children(promiseState.data) : children
}
40 changes: 26 additions & 14 deletions app/gui/src/dashboard/components/ErrorBoundary.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import * as result from '#/components/Result'
import { useEventCallback } from '#/hooks/eventCallbackHooks'
import * as errorUtils from '#/utilities/error'
import { OfflineError } from '#/utilities/HttpClient'
import type { FallbackProps } from 'react-error-boundary'
import SvgMask from './SvgMask'

// =====================
Expand All @@ -30,20 +31,28 @@ export interface OnBeforeFallbackShownArgs {
}

/** Props for an {@link ErrorBoundary}. */
export interface ErrorBoundaryProps
extends Readonly<React.PropsWithChildren>,
Readonly<
Pick<
errorBoundary.ErrorBoundaryProps,
'FallbackComponent' | 'onError' | 'onReset' | 'resetKeys'
>
> {
/** Called before the fallback is shown. */
readonly onBeforeFallbackShown?: (
args: OnBeforeFallbackShownArgs,
) => React.ReactNode | null | undefined
readonly title?: string
readonly subtitle?: string
export interface ErrorBoundaryProps extends Readonly<React.PropsWithChildren> {
/** Keys to reset the error boundary. Use it to declaratively reset the error boundary. */
readonly resetKeys?: errorBoundary.ErrorBoundaryProps['resetKeys'] | undefined
/** Fallback component to show when there is an error. */
// This is a Component, and supposed to be capitalized according to the react conventions.
// eslint-disable-next-line @typescript-eslint/naming-convention
readonly FallbackComponent?: React.ComponentType<FallbackProps> | undefined
/** Called when there is an error. */
readonly onError?: errorBoundary.ErrorBoundaryProps['onError'] | undefined
/** Called when the error boundary is reset. */
readonly onReset?: errorBoundary.ErrorBoundaryProps['onReset'] | undefined
/**
* Called before the fallback is shown, can return a React node to render instead of the fallback.
* Alternatively, you can use the error boundary api to reset the error boundary based on the error.
*/
readonly onBeforeFallbackShown?:
| ((args: OnBeforeFallbackShownArgs) => React.ReactNode | null | undefined)
| undefined
/** Title to show when there is an error. */
readonly title?: string | undefined
/** Subtitle to show when there is an error. */
readonly subtitle?: string | undefined
}

/**
Expand All @@ -59,13 +68,15 @@ export function ErrorBoundary(props: ErrorBoundaryProps) {
onBeforeFallbackShown = () => null,
title,
subtitle,
resetKeys,
...rest
} = props

return (
<reactQuery.QueryErrorResetBoundary>
{({ reset }) => (
<errorBoundary.ErrorBoundary
{...(resetKeys != null ? { resetKeys } : {})}
FallbackComponent={(fallbackProps) => {
const displayMessage = errorUtils.extractDisplayMessage(fallbackProps.error)

Expand Down Expand Up @@ -142,6 +153,7 @@ export function ErrorDisplay(props: ErrorDisplayProps): React.JSX.Element {
status={finalStatus}
title={finalTitle}
subtitle={finalSubtitle}
testId="error-display"
>
<ariaComponents.ButtonGroup align="center">
<ariaComponents.Button
Expand Down
7 changes: 5 additions & 2 deletions app/gui/src/dashboard/components/Result.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import Success from '#/assets/check_mark.svg'
import Error from '#/assets/cross.svg'

import { tv, type VariantProps } from '#/utilities/tailwindVariants'
import type { TestIdProps } from './AriaComponents'
import { Text } from './AriaComponents/Text'
import * as loader from './Loader'
import SvgMask from './SvgMask'
Expand Down Expand Up @@ -93,7 +94,10 @@ interface StatusIcon {
// ==============

/** Props for a {@link Result}. */
export interface ResultProps extends React.PropsWithChildren, VariantProps<typeof RESULT_STYLES> {
export interface ResultProps
extends React.PropsWithChildren,
VariantProps<typeof RESULT_STYLES>,
TestIdProps {
readonly className?: string
readonly title?: React.JSX.Element | string
readonly subtitle?: React.JSX.Element | string
Expand All @@ -103,7 +107,6 @@ export interface ResultProps extends React.PropsWithChildren, VariantProps<typeo
*/
readonly status?: React.ReactElement | Status
readonly icon?: string | false
readonly testId?: string
}

/** Display the result of an operation. */
Expand Down
5 changes: 3 additions & 2 deletions app/gui/src/dashboard/components/Suspense.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,9 @@ import * as React from 'react'
import * as loader from './Loader'

/** Props for {@link Suspense} component. */
export interface SuspenseProps extends React.SuspenseProps {
readonly loaderProps?: loader.LoaderProps
export interface SuspenseProps extends React.PropsWithChildren {
readonly fallback?: React.ReactNode | undefined
readonly loaderProps?: loader.LoaderProps | undefined
}

/**
Expand Down
68 changes: 68 additions & 0 deletions app/gui/src/dashboard/components/__tests__/Await.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
import { act, render, screen } from '@testing-library/react'
import { describe, vi } from 'vitest'
import { Await } from '../Await'

describe('<Await />', (it) => {
it('should the suspense boundary before promise is resolved, then show the children once promise is resolved', async ({
expect,
}) => {
const promise = Promise.resolve('Hello')
render(<Await promise={promise}>{(value) => <div>{value}</div>}</Await>)

expect(screen.queryByText('Hello')).not.toBeInTheDocument()
expect(screen.getByTestId('spinner')).toBeInTheDocument()

await act(() => promise)

expect(screen.getByText('Hello')).toBeInTheDocument()
expect(screen.queryByTestId('spinner')).not.toBeInTheDocument()
})

// This test is SUPPOSED to throw an error,
// Because the only way to test the error boundary is to throw an error during the render phase.
// But currently, vitest fails when promise is rejected with ⎯⎯⎯⎯ Unhandled Rejection ⎯⎯⎯⎯⎯ output,
// and it causes the test to fail on CI.
// We do not want to catch the error before we render the component,
// because in that case, the error boundary will not be triggered.
// This can be avoided by setting `dangerouslyIgnoreUnhandledErrors` to true in the vitest config,
// but it's unsafe to do for all tests, and there's no way to do it for a single test.
// We skip this test for now on CI, until we find a way to fix it.
it.skipIf(process.env.CI)(
'should show the fallback if the promise is rejected',
async ({ expect }) => {
// Suppress the error message from the console caused by React Error Boundary
vi.spyOn(console, 'error').mockImplementation(() => {})

const promise = Promise.reject(new Error('💣'))

render(<Await promise={promise}>{() => <>Hello</>}</Await>)

expect(screen.getByTestId('spinner')).toBeInTheDocument()

await act(() => promise.catch(() => {}))

expect(screen.queryByText('Hello')).not.toBeInTheDocument()
expect(screen.queryByTestId('spinner')).not.toBeInTheDocument()
expect(screen.getByTestId('error-display')).toBeInTheDocument()
// eslint-disable-next-line no-restricted-properties
expect(console.error).toHaveBeenCalled()
},
)

it('should not display the Suspense boundary of the second Await if the first Await already resolved', async ({
expect,
}) => {
const promise = Promise.resolve('Hello')
const { unmount } = render(<Await promise={promise}>{(value) => <div>{value}</div>}</Await>)

await act(() => promise)

expect(screen.getByText('Hello')).toBeInTheDocument()

unmount()

render(<Await promise={promise}>{(value) => <div>{value}</div>}</Await>)

expect(screen.getByText('Hello')).toBeInTheDocument()
})
})
Loading

0 comments on commit 0c41e8d

Please sign in to comment.