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

feat(Cell Suspense): Allow Cells to not Suspend #9106

Merged
merged 4 commits into from
Aug 31, 2023
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
24 changes: 13 additions & 11 deletions packages/web/src/components/cell/cellTypes.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ export interface CreateCellProps<CellProps, CellVariables> {
displayName?: string
}

export type SuperSuccessProps = React.PropsWithChildren<
export type SuspendingSuccessProps = React.PropsWithChildren<
Record<string, unknown>
> & {
queryRef: QueryReference<DataObject> // from useBackgroundQuery
Expand Down Expand Up @@ -208,18 +208,20 @@ export interface SuspenseCellQueryResult<
networkStatus?: NetworkStatus
called: boolean // can we assume if we have a queryRef its called?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just updates comments here based on conversation with Lenz, Tobbe and Tom

// Stuff not here:
// observable: ObservableQuery<TData, TVariables>
// previousData?: TData, May not be relevant anymore.
// Stuff not here compared to useQuery:
// observable: ObservableQuery<TData, TVariables> // Lenz: internal implementation detail, should not be required anymore
// previousData?: TData, // emulating suspense, not required in the new Suspense model

// ObservableQueryFields 👇
// subscribeToMore ~ returned from useSuspenseQuery. What would users use this for?
// updateQuery
// refetch
// reobserve
// variables <~ variables passed to the query. Startup club have reported using this, but why?
// fetchMore
// startPolling <~ Apollo team are not ready to expose Polling yet
// subscribeToMore ~ returned from useSuspenseQuery but not useReadQuery. Apollo team **may** expose from useReadQuery.
// updateQuery <~ May not be necessary in the Suspense model
// refetch ~ <~ refetch signature is different in useQuery vs useSuspenseQuery. Apollo team need an internal discussion.
// reobserve <~ avoid
// variables <~ variables passed to the query, useful if you updated the variables using updateQuery or refetch. Apollo team need an internal discussion.

// Polling: Apollo team are not ready to expose Polling yet. Unlikely to be shipped in the next few months.
// But possible to re-implement this in a different way using setInternal or client.startPolling
// startPolling
// stopPolling
// ~~~
}
38 changes: 28 additions & 10 deletions packages/web/src/components/cell/createSuspendingCell.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { CellErrorBoundary, FallbackProps } from './CellErrorBoundary'
import {
CreateCellProps,
DataObject,
SuperSuccessProps,
SuspendingSuccessProps,
SuspenseCellQueryResult,
} from './cellTypes'
import { isDataEmpty } from './isCellEmpty'
Expand Down Expand Up @@ -44,13 +44,13 @@ export function createSuspendingCell<
}),
afterQuery = (data) => ({ ...data }),
isEmpty = isDataEmpty,
Loading = () => <>Loading...</>,
Loading,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note we don't default the Loading component anymore!

Failure,
Empty,
Success,
displayName = 'Cell',
} = createCellProps
function SuperSuccess(props: SuperSuccessProps) {
function SuspendingSuccess(props: SuspendingSuccessProps) {
const { queryRef, suspenseQueryResult, userProps } = props
const { data, networkStatus } = useReadQuery<DataObject>(queryRef)
const afterQueryData = afterQuery(data as DataObject)
Expand Down Expand Up @@ -79,7 +79,7 @@ export function createSuspendingCell<
)
}

SuperSuccess.displayName = displayName
SuspendingSuccess.displayName = displayName

// @NOTE: Note that we are returning a HoC here!
return (props: CellProps) => {
Expand Down Expand Up @@ -125,17 +125,35 @@ export function createSuspendingCell<
)
}

return (
<CellErrorBoundary renderFallback={FailureComponent}>
const wrapInSuspenseIfLoadingPresent = (
suspendingSuccessElement: React.ReactNode,
LoadingComponent: typeof Loading
) => {
if (!LoadingComponent) {
return suspendingSuccessElement
}

return (
<Suspense
fallback={<Loading {...props} queryResult={suspenseQueryResult} />}
fallback={
<LoadingComponent {...props} queryResult={suspenseQueryResult} />
}
>
<SuperSuccess
{suspendingSuccessElement}
</Suspense>
)
}

return (
<CellErrorBoundary renderFallback={FailureComponent}>
{wrapInSuspenseIfLoadingPresent(
<SuspendingSuccess
userProps={props}
queryRef={queryRef as QueryReference<DataObject>}
suspenseQueryResult={suspenseQueryResult}
/>
</Suspense>
/>,
Loading
)}
</CellErrorBoundary>
)
}
Expand Down