ErrorBoundary for Perspective tables

catalog/app/components/Preview/renderers/Perspective/Perspective.tsx

@@ -3,6 +3,7 @@ import * as React from 'react'
 import type { RegularTableElement } from 'regular-table'
 import * as M from '@material-ui/core'
 import * as Lab from '@material-ui/lab'
+import { ErrorBoundary } from 'react-error-boundary'
 
 import * as perspective from 'utils/perspective'
 
@@ -39,7 +40,7 @@ const useTruncatedWarningStyles = M.makeStyles((t) => ({
 interface ToolbarProps {
   className: string
   onLoadMore?: () => void
-  state: perspective.State | null
+  state: perspective.Model
   truncated: boolean
 }
 
@@ -93,28 +94,31 @@ function Toolbar({ className, onLoadMore, state, truncated }: ToolbarProps) {
 }
 
 const useStyles = M.makeStyles((t) => ({
-  root: {
-    display: 'flex',
-    flexDirection: 'column',
-    minHeight: t.spacing(60),
-    overflow: 'hidden',
-    // NOTE: padding is required because perspective-viewer covers resize handle
-    padding: '0 0 8px',
-    resize: 'vertical',
-  },
-  fullHeight: {
-    minHeight: t.spacing(120),
-  },
   meta: {
-    marginBottom: t.spacing(1),
+    marginBottom: t.spacing(2),
   },
   viewer: {
+    background: '#f2f4f6',
     flexGrow: 1,
     zIndex: 1,
   },
   toolbar: {
     marginBottom: t.spacing(1),
   },
+  table: {
+    flexGrow: 1,
+    minHeight: t.spacing(60),
+
+    display: 'flex',
+    flexDirection: 'column',
+    overflow: 'hidden',
+
+    // NOTE: padding is required because perspective-viewer covers resize handle
+    //       and for inset shadows
+    padding: t.spacing(0.5, 0.5, 1),
+    boxShadow: `inset 0 0 ${t.spacing(0.5)}px rgba(0, 0, 0, 0.2)`,
+    resize: 'vertical',
+  },
   warning: {
     marginTop: t.spacing(2),
   },
@@ -124,17 +128,14 @@ export interface PerspectiveProps
   extends React.HTMLAttributes<HTMLDivElement>,
     PerspectiveOptions {
   data: perspective.PerspectiveInput
-  meta?: ParquetMetadata | H5adMetadata | PackageMetadata
   onLoadMore?: () => void
   onRender?: (tableEl: RegularTableElement) => void
   truncated: boolean
 }
 
-export default function Perspective({
-  children,
+function Perspective({
   className,
   data,
-  meta,
   onLoadMore,
   onRender,
   truncated,
@@ -143,36 +144,46 @@ export default function Perspective({
 }: PerspectiveProps) {
   const classes = useStyles()
 
-  const [root, setRoot] = React.useState<HTMLDivElement | null>(null)
+  const [anchorEl, setAnchorEl] = React.useState<HTMLDivElement | null>(null)
+  const state = perspective.use(anchorEl, data, classes.viewer, config, onRender)
+
+  return (
+    <div className={className} {...props}>
+      {state && (
+        <Toolbar
+          className={classes.toolbar}
+          state={state}
+          onLoadMore={onLoadMore}
+          truncated={truncated}
+        />
+      )}
+      <div ref={setAnchorEl} className={classes.table} />
+    </div>
+  )
+}
+
+interface TabularProps extends PerspectiveProps {
+  meta?: ParquetMetadata | H5adMetadata | PackageMetadata
+}
 
-  const attrs = React.useMemo(() => ({ className: classes.viewer }), [classes])
-  const state = perspective.use(root, data, attrs, config, onRender)
+function ErrorFallback() {
+  const classes = useStyles()
+  return (
+    <Lab.Alert className={classes.warning} severity="info" icon={false}>
+      Could not render tabular data
+    </Lab.Alert>
+  )
+}
 
-  if (state instanceof Error) {
-    return (
-      <div className={cx(className, classes.root)} {...props}>
-        {!!meta && <Metadata className={classes.meta} metadata={meta} />}
-        <Lab.Alert className={classes.warning} severity="info" icon={false}>
-          Could not render tabular data
-        </Lab.Alert>
-      </div>
-    )
-  }
+export default function Tabular({ meta, ...props }: TabularProps) {
+  const classes = useStyles()
 
   return (
-    <div
-      className={cx(className, classes.root, classes.fullHeight)}
-      ref={setRoot}
-      {...props}
-    >
-      <Toolbar
-        className={classes.toolbar}
-        state={state}
-        onLoadMore={onLoadMore}
-        truncated={truncated}
-      />
+    <>
       {!!meta && <Metadata className={classes.meta} metadata={meta} />}
-      {children}
-    </div>
+      <ErrorBoundary FallbackComponent={ErrorFallback}>
+        <Perspective {...props} />
+      </ErrorBoundary>
+    </>
   )
 }

catalog/app/utils/perspective.ts

@@ -6,106 +6,155 @@ import perspective from '@finos/perspective'
 import type { Table, TableData, ViewConfig } from '@finos/perspective'
 import type { HTMLPerspectiveViewerElement } from '@finos/perspective-viewer'
 
-import log from 'utils/Logging'
+import Log from 'utils/Logging'
 import { themes } from 'utils/perspective-pollution'
 
-export interface State {
-  rotateThemes: () => void
-  size: number | null
-  toggleConfig: () => void
-}
-
 export type PerspectiveInput = TableData
 
 const worker = perspective.worker()
 
-export function renderViewer(
-  parentNode: HTMLElement,
-  { className }: React.HTMLAttributes<HTMLDivElement>,
-): HTMLPerspectiveViewerElement {
-  const element = document.createElement('perspective-viewer')
-  // NOTE: safari needs `.perspective-viewer-material` instead of custom tagName
-  element.className = cx('perspective-viewer-material', className)
-  parentNode.appendChild(element)
-  return element
+export type Model = {
+  rotateThemes: () => void
+  size: number | null
+  toggleConfig: () => void
 }
 
-export async function renderTable(
-  data: PerspectiveInput,
+async function createModel(
   viewer: HTMLPerspectiveViewerElement,
-) {
-  const table = await worker.table(data)
-  await viewer.load(table)
-  return table
+  table: Table,
+): Promise<Model> {
+  const size = await table.size()
+  return {
+    rotateThemes: async () => {
+      const settings = await viewer?.save()
+      // @ts-expect-error `ViewConfig` type doesn't have `theme`
+      const themeIndex = themes.findIndex((t) => t === settings?.theme)
+      const theme = themeIndex === themes.length - 1 ? themes[0] : themes[themeIndex + 1]
+      viewer?.restore({ theme } as ViewConfig)
+    },
+    size,
+    toggleConfig: () => viewer?.toggleConfig(),
+  }
 }
 
-function usePerspective(
-  container: HTMLDivElement | null,
-  data: PerspectiveInput,
-  attrs: React.HTMLAttributes<HTMLDivElement>,
-  config?: ViewConfig,
-  onRender?: (tableEl: RegularTableElement) => void,
+function useModel(
+  viewer: HTMLPerspectiveViewerElement | null,
+  table: Table | Error | null,
 ) {
-  const [state, setState] = React.useState<State | Error | null>(null)
+  const [model, setModel] = React.useState<Model | null>(null)
+  const [error, setError] = React.useState<Error | null>(null)
+
+  const init = React.useCallback(async (): Promise<Model | null> => {
+    if (!table || !viewer) return null
+    if (table instanceof Error) {
+      throw table
+    }
+    return createModel(viewer, table)
+  }, [viewer, table])
 
   React.useEffect(() => {
-    // NOTE(@fiskus): if you want to refactor, don't try `useRef`, try something different
-    let table: Table | null = null
-    let viewer: HTMLPerspectiveViewerElement | null = null
-
-    async function renderData() {
-      if (!container) return
-
-      try {
-        viewer = renderViewer(container, attrs)
-        table = await renderTable(data, viewer)
-      } catch (e) {
-        const error = e instanceof Error ? e : new Error((e as any).message || `${e}`)
-        setState(error)
-        log.error(error)
-        return
-      }
-
-      const regularTable: RegularTableElement | null =
-        viewer.querySelector('regular-table')
-      if (onRender && regularTable?.addStyleListener) {
-        onRender(regularTable)
-        regularTable.addStyleListener(({ detail }) => onRender(detail))
-      }
-
-      if (config) {
-        await viewer.restore(config)
-      }
-
-      const size = await table.size()
-      setState({
-        rotateThemes: async () => {
-          const settings = await viewer?.save()
-          // @ts-expect-error `ViewConfig` type doesn't have `theme`
-          const themeIndex = themes.findIndex((t) => t === settings?.theme)
-          const theme =
-            themeIndex === themes.length - 1 ? themes[0] : themes[themeIndex + 1]
-          viewer?.restore({ theme } as ViewConfig)
-        },
-        size,
-        toggleConfig: () => viewer?.toggleConfig(),
+    init()
+      .then(setModel)
+      .catch((e) => {
+        setError(e)
+        Log.error(e)
       })
+  }, [init])
+
+  if (error) {
+    throw error
+  }
+
+  return model
+}
+
+function useViewer(anchorEl: HTMLDivElement | null, className: string) {
+  const [viewer, setViewer] = React.useState<HTMLPerspectiveViewerElement | null>(null)
+
+  React.useEffect(() => {
+    if (!anchorEl) return
+
+    const element = document.createElement('perspective-viewer')
+    // NOTE: safari needs `.perspective-viewer-material` instead of custom tagName
+    element.className = cx('perspective-viewer-material', className)
+    anchorEl.appendChild(element)
+
+    setViewer(element)
+
+    return () => {
+      element.parentNode?.removeChild(element)
+      element.delete()
     }
+  }, [anchorEl, className])
 
-    async function disposeTable() {
-      viewer?.parentNode?.removeChild(viewer)
-      await viewer?.delete()
-      await table?.delete()
+  return viewer
+}
+
+function useTable(viewer: HTMLPerspectiveViewerElement | null, data: PerspectiveInput) {
+  const [table, setTable] = React.useState<Table | Error | null>(null)
+  React.useEffect(() => {
+    let tbl: Table | null = null
+
+    async function renderTable() {
+      if (!viewer) return
+
+      tbl = await worker.table(data)
+      await viewer.load(tbl)
+      setTable(tbl)
     }
 
-    renderData()
+    renderTable().catch((e) => {
+      setTable(e instanceof Error ? e : new Error(e.message || `${e}`))
+    })
 
     return () => {
-      disposeTable()
+      tbl?.delete()
+      tbl = null
     }
-  }, [attrs, config, container, data, onRender])
+  }, [data, viewer])
+  return table
+}
+
+function useRestoreConfig(
+  viewer: HTMLPerspectiveViewerElement | null,
+  config?: ViewConfig,
+) {
+  React.useEffect(() => {
+    if (!config || !viewer) return
+    viewer.restore(config)
+  }, [config, viewer])
+}
+
+function useListenOnRender(
+  viewer: HTMLPerspectiveViewerElement | null,
+  onRender?: (tableEl: RegularTableElement) => void,
+) {
+  React.useEffect(() => {
+    if (!viewer) return
+
+    const regularTable: RegularTableElement | null = viewer.querySelector('regular-table')
+
+    if (!onRender || !regularTable?.addStyleListener) return
+
+    onRender(regularTable)
+    regularTable.addStyleListener(({ detail }) => onRender(detail))
+  }, [onRender, viewer])
+}
+
+function usePerspective(
+  anchorEl: HTMLDivElement | null,
+  data: PerspectiveInput,
+  className: string,
+  config?: ViewConfig,
+  onRender?: (tableEl: RegularTableElement) => void,
+) {
+  const viewer = useViewer(anchorEl, className)
+  const table = useTable(viewer, data)
+
+  useRestoreConfig(viewer, config)
+  useListenOnRender(viewer, onRender)
 
-  return state
+  return useModel(viewer, table)
 }
 
 export const use = usePerspective