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

Await component #11936

Merged
merged 4 commits into from
Dec 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
Loading