Skip to content

Commit

Permalink
feat(POC): add error boundary to withSuspense (#10824)
Browse files Browse the repository at this point in the history
* feat: add error boundary to suspense

* chore: Update withSuspense hook to use ErrorFallback prop

* chore: fix sentry mock

* chore: Fix sentry mock

* add react error-boundary and track sentry stacktrace on error

* fix: sentry mock

* add test patch with query rejections

* turn off persisted queries for now

* use explicit empty fallback for all home view sections

* Revert "use explicit empty fallback for all home view sections"

This reverts commit d0395d6.

* add explicit empty fallback for home view sections

* Revert "add explicit empty fallback for home view sections"

This reverts commit 20f464a.

* pls let this be the last time

* add error boundary for home view section screen

* add error boundary to new works for you page

* add fallback for artwork error page

* add fallback for activity screen

* add fallback for browse similar works

* add temp strict suspense hook to track progress easier

* explicit fallback for similar works

* explicit fallback for profile failure

* explicit fallback for NewWorksFromGalleriesYouFollow

* explicit fallback for RecentlyViewArtworks

* explicit fallback for SavedSearchFilterArtistSeries

* remove log

* explicit fallback for SavedSearchFilterPriceRange

* explicit fallback for AlertPriceRangeScreen

* explicit fallback for MyCollectionArtworkEdit

* explicit fallback for MyCollectionCollectedArtistsPrivacy

* remove log

* explicit fallback for ConfirmationScreen

* explicit fallback for SubmitArtworkFormEdit

* explicit fallback for MyCollectionArtworkFormDeleteArtworkModal

* delete unstrict suspense hook

* revert to old name

* remove logs

* Revert "turn off persisted queries for now"

This reverts commit fbed79a.

* move fetch patch to a test dir

* more consistent worksForYou fallback

* more consistent error for activity screen

* remove missed change

* add better api for no fallback

* pass new explicit param everywhere, fix types in withSuspense

* make withSuspense take a typed object for better api

* make selection of spinner fallback explicit

* add some docs to the interface

* render retry button when using LoadFailureView

---------

Co-authored-by: brainbicycle <brian.beckerle@artsymail.com>
  • Loading branch information
gkartalis and brainbicycle authored Oct 8, 2024
1 parent 8caed6b commit 23c860d
Show file tree
Hide file tree
Showing 38 changed files with 694 additions and 369 deletions.
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@
"ordinal": "1.0.3",
"query-string": "4.3.4",
"react": "18.2.0",
"react-error-boundary": "4.0.13",
"react-fps": "1.0.6",
"react-native": "0.73.9",
"react-native-blob-util": "0.19.9",
Expand Down
9 changes: 5 additions & 4 deletions src/app/Components/WorksForYouArtworks.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,8 @@ interface WorksForYouArtworksQRProps {
onlyAtAuction?: boolean
}

export const WorksForYouArtworksQR: React.FC<WorksForYouArtworksQRProps> = withSuspense(
({ version, onlyAtAuction = false, maxWorksPerArtist = 3 }) => {
export const WorksForYouArtworksQR: React.FC<WorksForYouArtworksQRProps> = withSuspense({
Component: ({ version, onlyAtAuction = false, maxWorksPerArtist = 3 }) => {
const data = useLazyLoadQuery<WorksForYouArtworksQuery>(
newWorksForYouArtworksQuery,
{
Expand All @@ -145,5 +145,6 @@ export const WorksForYouArtworksQR: React.FC<WorksForYouArtworksQRProps> = withS

return <WorksForYouArtworks viewer={data.viewer} />
},
() => <NewWorksForYouPlaceholder />
)
LoadingFallback: () => <NewWorksForYouPlaceholder />,
ErrorFallback: () => <SimpleMessage m={2}>Nothing yet. Please check back later.</SimpleMessage>,
})
11 changes: 7 additions & 4 deletions src/app/Scenes/Activity/ActivityItemScreen.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ interface ActivityItemScreenQueryRendererProps {
}

export const ActivityItemScreenQueryRenderer: FC<ActivityItemScreenQueryRendererProps> =
withSuspense(
({ notificationID }) => {
withSuspense({
Component: ({ notificationID }) => {
const data = useLazyLoadQuery<ActivityItemScreenQuery>(ActivityItemQuery, {
internalID: notificationID,
})
Expand Down Expand Up @@ -65,8 +65,11 @@ export const ActivityItemScreenQueryRenderer: FC<ActivityItemScreenQueryRenderer
return null
}
},
() => <Placeholder />
)
LoadingFallback: () => <Placeholder />,
ErrorFallback: (fallbackProps) => {
return <ActivityErrorScreen headerTitle="Activity" error={fallbackProps.error} />
},
})

const ActivityItemQuery = graphql`
query ActivityItemScreenQuery($internalID: String!) {
Expand Down
6 changes: 3 additions & 3 deletions src/app/Scenes/Activity/components/ActivityErrorScreen.tsx
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
import { Screen } from "@artsy/palette-mobile"
import { LoadFailureView } from "app/Components/LoadFailureView"
import { Screen, SimpleMessage } from "@artsy/palette-mobile"
import { goBack } from "app/system/navigation/navigate"

interface ActivityErrorScreenProps {
headerTitle: string
error?: Error
}

export const ActivityErrorScreen: React.FC<ActivityErrorScreenProps> = ({ headerTitle }) => {
return (
<Screen>
<Screen.Header title={headerTitle} onBack={goBack} />
<LoadFailureView justifyContent="center" mb="100px" />
<SimpleMessage m={2}>Something went wrong. Please check back later.</SimpleMessage>
</Screen>
)
}
22 changes: 17 additions & 5 deletions src/app/Scenes/Artwork/Components/ArtworkError.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { Flex, Join, Spacer, Spinner, Text } from "@artsy/palette-mobile"
import { ArtworkErrorQuery } from "__generated__/ArtworkErrorQuery.graphql"
import { ArtworkErrorRecentlyViewed_homePage$key } from "__generated__/ArtworkErrorRecentlyViewed_homePage.graphql"
import { FancyModalHeader } from "app/Components/FancyModal/FancyModalHeader"
import { LoadFailureView } from "app/Components/LoadFailureView"
import { ArtworkModuleRailFragmentContainer } from "app/Scenes/Home/Components/ArtworkModuleRail"
import { ArtworkRecommendationsRail } from "app/Scenes/Home/Components/ArtworkRecommendationsRail"
import { NewWorksForYouRail } from "app/Scenes/Home/Components/NewWorksForYouRail"
Expand Down Expand Up @@ -75,8 +76,8 @@ export const ArtworkError: React.FC<ArtworkErrorProps> = ({ homePage, me, viewer
)
}

export const ArtworkErrorScreen: React.FC<{}> = withSuspense(
() => {
export const ArtworkErrorScreen: React.FC<{}> = withSuspense({
Component: () => {
const worksForYouRecommendationsModel = useExperimentVariant(
RECOMMENDATION_MODEL_EXPERIMENT_NAME
)
Expand All @@ -100,12 +101,23 @@ export const ArtworkErrorScreen: React.FC<{}> = withSuspense(
}
return <ArtworkError homePage={data.homePage} me={data.me} viewer={data.viewer} />
},
() => (
LoadingFallback: () => (
<Flex flex={1} alignItems="center" justifyContent="center" testID="placeholder">
<Spinner />
</Flex>
)
)
),
ErrorFallback: (fallbackProps) => {
return (
<LoadFailureView
onRetry={fallbackProps.resetErrorBoundary}
useSafeArea={false}
error={fallbackProps.error}
showBackButton={true}
trackErrorBoundary={false}
/>
)
},
})

const recentlyViewedFragment = graphql`
fragment ArtworkErrorRecentlyViewed_homePage on HomePage {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,17 +50,26 @@ const BrowseSimilarWorks: React.FC<{ artwork: BrowseSimilarWorks_artwork$key }>

const artworkAlert = computeArtworkAlertProps(artwork)

if (
!artworkAlert ||
!artworkAlert.entity ||
!artworkAlert.attributes ||
!artworkAlert.aggregations
) {
return null
}

const params: BrowseSimilarWorksProps = {
aggregations: artworkAlert.aggregations!,
attributes: artworkAlert.attributes!,
entity: artworkAlert.entity!,
aggregations: artworkAlert.aggregations,
attributes: artworkAlert.attributes,
entity: artworkAlert.entity,
}

return <BrowseSimilarWorksContent params={params} />
}

export const BrowseSimilarWorksQueryRenderer: React.FC<{ artworkID: string }> = withSuspense(
(props) => {
export const BrowseSimilarWorksQueryRenderer: React.FC<{ artworkID: string }> = withSuspense({
Component: (props) => {
const data = useLazyLoadQuery<BrowseSimilarWorksQuery>(SimilarWorksQuery, {
artworkID: props.artworkID,
})
Expand All @@ -69,10 +78,13 @@ export const BrowseSimilarWorksQueryRenderer: React.FC<{ artworkID: string }> =
return <BrowseSimilarWorksErrorState />
}

return <BrowseSimilarWorks artwork={data.artwork!} />
return <BrowseSimilarWorks artwork={data.artwork} />
},
LoadingFallback: BrowseSimilarWorksPlaceholder,
ErrorFallback: () => {
return <BrowseSimilarWorksErrorState />
},
BrowseSimilarWorksPlaceholder
)
})

const similarWorksFragment = graphql`
fragment BrowseSimilarWorks_artwork on Artwork {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,8 @@ const SimilarArtworksPlaceholder: React.FC = () => {
return <GenericGridPlaceholder width={screen.width - space(4)} />
}

const SimilarArtworksContainer: React.FC<{ attributes: SearchCriteriaAttributes }> = withSuspense(
({ attributes }) => {
const SimilarArtworksContainer: React.FC<{ attributes: SearchCriteriaAttributes }> = withSuspense({
Component: ({ attributes }) => {
const screen = useScreenDimensions()
const { space } = useTheme()

Expand Down Expand Up @@ -125,5 +125,12 @@ const SimilarArtworksContainer: React.FC<{ attributes: SearchCriteriaAttributes
</>
)
},
SimilarArtworksPlaceholder
)
LoadingFallback: SimilarArtworksPlaceholder,
ErrorFallback: () => {
return (
<SimpleMessage>
There aren’t any works available that meet the criteria at this time.
</SimpleMessage>
)
},
})
11 changes: 6 additions & 5 deletions src/app/Scenes/HomeView/Sections/HomeViewSectionActivity.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import {
import { useHomeViewTracking } from "app/Scenes/HomeView/useHomeViewTracking"
import { navigate } from "app/system/navigation/navigate"
import { extractNodes } from "app/utils/extractNodes"
import { withSuspense } from "app/utils/hooks/withSuspense"
import { NoFallback, withSuspense } from "app/utils/hooks/withSuspense"
import { useMemoizedRandom } from "app/utils/placeholders"
import { times } from "lodash"
import { FlatList } from "react-native"
Expand Down Expand Up @@ -192,8 +192,8 @@ const HomeViewSectionActivityPlaceholder: React.FC<FlexProps> = (flexProps) => {
)
}

export const HomeViewSectionActivityQueryRenderer: React.FC<SectionSharedProps> = withSuspense(
({ sectionID, index, ...flexProps }) => {
export const HomeViewSectionActivityQueryRenderer: React.FC<SectionSharedProps> = withSuspense({
Component: ({ sectionID, index, ...flexProps }) => {
const data = useLazyLoadQuery<HomeViewSectionActivityQuery>(homeViewSectionActivityQuery, {
id: sectionID,
})
Expand All @@ -204,5 +204,6 @@ export const HomeViewSectionActivityQueryRenderer: React.FC<SectionSharedProps>

return <HomeViewSectionActivity section={data.homeView.section} index={index} {...flexProps} />
},
HomeViewSectionActivityPlaceholder
)
LoadingFallback: HomeViewSectionActivityPlaceholder,
ErrorFallback: NoFallback,
})
11 changes: 6 additions & 5 deletions src/app/Scenes/HomeView/Sections/HomeViewSectionArticles.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import { HOME_VIEW_SECTIONS_SEPARATOR_HEIGHT } from "app/Scenes/HomeView/HomeVie
import { SectionSharedProps } from "app/Scenes/HomeView/Sections/Section"
import { useHomeViewTracking } from "app/Scenes/HomeView/useHomeViewTracking"
import { navigate } from "app/system/navigation/navigate"
import { withSuspense } from "app/utils/hooks/withSuspense"
import { NoFallback, withSuspense } from "app/utils/hooks/withSuspense"
import { useMemoizedRandom } from "app/utils/placeholders"
import { times } from "lodash"
import { graphql, useFragment, useLazyLoadQuery } from "react-relay"
Expand Down Expand Up @@ -147,8 +147,8 @@ const HomeViewSectionArticlesPlaceholder: React.FC<FlexProps> = (flexProps) => {
)
}

export const HomeViewSectionArticlesQueryRenderer: React.FC<SectionSharedProps> = withSuspense(
({ sectionID, index, ...flexProps }) => {
export const HomeViewSectionArticlesQueryRenderer: React.FC<SectionSharedProps> = withSuspense({
Component: ({ sectionID, index, ...flexProps }) => {
const data = useLazyLoadQuery<HomeViewSectionArticlesQuery>(
homeViewSectionArticlesQuery,
{
Expand All @@ -167,5 +167,6 @@ export const HomeViewSectionArticlesQueryRenderer: React.FC<SectionSharedProps>

return <HomeViewSectionArticles section={data.homeView.section} index={index} {...flexProps} />
},
HomeViewSectionArticlesPlaceholder
)
LoadingFallback: HomeViewSectionArticlesPlaceholder,
ErrorFallback: NoFallback,
})
47 changes: 27 additions & 20 deletions src/app/Scenes/HomeView/Sections/HomeViewSectionArticlesCards.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import { SectionSharedProps } from "app/Scenes/HomeView/Sections/Section"
import { useHomeViewTracking } from "app/Scenes/HomeView/useHomeViewTracking"
import { navigate } from "app/system/navigation/navigate"
import { extractNodes } from "app/utils/extractNodes"
import { withSuspense } from "app/utils/hooks/withSuspense"
import { NoFallback, withSuspense } from "app/utils/hooks/withSuspense"
import { ExtractNodeType } from "app/utils/relayHelpers"
import { times } from "lodash"
import { graphql, useFragment, useLazyLoadQuery } from "react-relay"
Expand Down Expand Up @@ -181,26 +181,33 @@ const homeViewSectionArticlesCardsQuery = graphql`
`

export const HomeViewSectionArticlesCardsQueryRenderer: React.FC<SectionSharedProps> = withSuspense(
({ sectionID, index, ...flexProps }) => {
const data = useLazyLoadQuery<HomeViewSectionArticlesCardsQuery>(
homeViewSectionArticlesCardsQuery,
{
id: sectionID,
},
{
networkCacheConfig: {
force: false,
{
Component: ({ sectionID, index, ...flexProps }) => {
const data = useLazyLoadQuery<HomeViewSectionArticlesCardsQuery>(
homeViewSectionArticlesCardsQuery,
{
id: sectionID,
},
}
)
{
networkCacheConfig: {
force: false,
},
}
)

if (!data.homeView.section) {
return null
}
if (!data.homeView.section) {
return null
}

return (
<HomeViewSectionArticlesCards section={data.homeView.section} index={index} {...flexProps} />
)
},
HomeViewSectionArticlesCardsPlaceholder
return (
<HomeViewSectionArticlesCards
section={data.homeView.section}
index={index}
{...flexProps}
/>
)
},
LoadingFallback: HomeViewSectionArticlesCardsPlaceholder,
ErrorFallback: NoFallback,
}
)
11 changes: 6 additions & 5 deletions src/app/Scenes/HomeView/Sections/HomeViewSectionArtists.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import {
import { useHomeViewTracking } from "app/Scenes/HomeView/useHomeViewTracking"
import { navigate } from "app/system/navigation/navigate"
import { extractNodes } from "app/utils/extractNodes"
import { withSuspense } from "app/utils/hooks/withSuspense"
import { NoFallback, withSuspense } from "app/utils/hooks/withSuspense"
import { useMemoizedRandom } from "app/utils/placeholders"
import { ExtractNodeType } from "app/utils/relayHelpers"
import { times } from "lodash"
Expand Down Expand Up @@ -252,8 +252,8 @@ const HomeViewSectionArtistsPlaceholder: React.FC<FlexProps> = (flexProps) => {
)
}

export const HomeViewSectionArtistsQueryRenderer: React.FC<SectionSharedProps> = withSuspense(
({ sectionID, index, ...flexProps }) => {
export const HomeViewSectionArtistsQueryRenderer: React.FC<SectionSharedProps> = withSuspense({
Component: ({ sectionID, index, ...flexProps }) => {
const data = useLazyLoadQuery<HomeViewSectionArtistsMainQuery>(homeViewSectionArtistsQuery, {
id: sectionID,
})
Expand All @@ -270,5 +270,6 @@ export const HomeViewSectionArtistsQueryRenderer: React.FC<SectionSharedProps> =
/>
)
},
HomeViewSectionArtistsPlaceholder
)
LoadingFallback: HomeViewSectionArtistsPlaceholder,
ErrorFallback: NoFallback,
})
11 changes: 6 additions & 5 deletions src/app/Scenes/HomeView/Sections/HomeViewSectionArtworks.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import { useHomeViewTracking } from "app/Scenes/HomeView/useHomeViewTracking"
import { navigate } from "app/system/navigation/navigate"
import { extractNodes } from "app/utils/extractNodes"
import { useFeatureFlag } from "app/utils/hooks/useFeatureFlag"
import { withSuspense } from "app/utils/hooks/withSuspense"
import { NoFallback, withSuspense } from "app/utils/hooks/withSuspense"
import { useMemoizedRandom } from "app/utils/placeholders"
import { times } from "lodash"
import { graphql, useFragment, useLazyLoadQuery } from "react-relay"
Expand Down Expand Up @@ -195,8 +195,8 @@ const HomeViewSectionArtworksPlaceholder: React.FC<FlexProps> = (flexProps) => {
)
}

export const HomeViewSectionArtworksQueryRenderer: React.FC<SectionSharedProps> = withSuspense(
({ sectionID, index, ...flexProps }) => {
export const HomeViewSectionArtworksQueryRenderer: React.FC<SectionSharedProps> = withSuspense({
Component: ({ sectionID, index, ...flexProps }) => {
const data = useLazyLoadQuery<HomeViewSectionArtworksQuery>(homeViewSectionArtworksQuery, {
id: sectionID,
})
Expand All @@ -207,5 +207,6 @@ export const HomeViewSectionArtworksQueryRenderer: React.FC<SectionSharedProps>

return <HomeViewSectionArtworks section={data.homeView.section} index={index} {...flexProps} />
},
HomeViewSectionArtworksPlaceholder
)
LoadingFallback: HomeViewSectionArtworksPlaceholder,
ErrorFallback: NoFallback,
})
Loading

0 comments on commit 23c860d

Please sign in to comment.