Merge pull request #25093 from overleaf/td-upgrade-react-error-boundary-second-attempt

Upgrade react-error-boundary to version 5, second attempt

GitOrigin-RevId: 2b88334b66f0ace383211c147279ff88e9f956bb

package-lock.json

@@ -34942,16 +34942,13 @@
       }
     },
     "node_modules/react-error-boundary": {
-      "version": "2.3.2",
-      "resolved": "https://registry.npmjs.org/react-error-boundary/-/react-error-boundary-2.3.2.tgz",
-      "integrity": "sha512-ZMzi7s4pj/6A/6i9RS4tG7g1PdF2Rgr4/7FTQ8sbKHex19uNji0j+xq0OS//c6TUgQRKoL6P51BNNNFmYpRMhw==",
+      "version": "5.0.0",
+      "resolved": "https://registry.npmjs.org/react-error-boundary/-/react-error-boundary-5.0.0.tgz",
+      "integrity": "sha512-tnjAxG+IkpLephNcePNA7v6F/QpWLH8He65+DmedchDwg162JZqx4NmbXj0mlAYVVEd81OW7aFhmbsScYfiAFQ==",
       "dev": true,
+      "license": "MIT",
       "dependencies": {
-        "@babel/runtime": "^7.11.2"
-      },
-      "engines": {
-        "node": ">=10",
-        "npm": ">=6"
+        "@babel/runtime": "^7.12.5"
       },
       "peerDependencies": {
         "react": ">=16.13.1"
@@ -45321,7 +45318,7 @@
         "react-dnd": "^16.0.1",
         "react-dnd-html5-backend": "^16.0.1",
         "react-dom": "^17.0.2",
-        "react-error-boundary": "^2.3.1",
+        "react-error-boundary": "^5.0.0",
         "react-google-recaptcha": "^3.1.0",
         "react-i18next": "^13.3.1",
         "react-linkify": "^1.0.0-alpha",

services/web/frontend/js/features/chat/components/chat-pane.tsx

@@ -114,4 +114,4 @@ function Placeholder() {
   )
 }
 
-export default withErrorBoundary(ChatPane, ChatFallbackError)
+export default withErrorBoundary(ChatPane, () => <ChatFallbackError />)

services/web/frontend/js/features/history/components/diff-view/diff-view.tsx

@@ -5,7 +5,7 @@ import { Diff, DocDiffResponse } from '../../services/types/doc'
 import { useHistoryContext } from '../../context/history-context'
 import { diffDoc } from '../../services/api'
 import { highlightsFromDiffResponse } from '../../utils/highlights-from-diff-response'
-import { useErrorHandler } from 'react-error-boundary'
+import { useErrorBoundary } from 'react-error-boundary'
 import useAsync from '../../../../shared/hooks/use-async'
 import { useTranslation } from 'react-i18next'
 
@@ -14,7 +14,7 @@ function DiffView() {
   const { isLoading, data, runAsync } = useAsync<DocDiffResponse>()
   const { t } = useTranslation()
   const { updateRange, selectedFile } = selection
-  const handleError = useErrorHandler()
+  const { showBoundary } = useErrorBoundary()
 
   useEffect(() => {
     if (!updateRange || !selectedFile?.pathname || loadingFileDiffs) {
@@ -33,7 +33,7 @@ function DiffView() {
         abortController.signal
       )
     )
-      .catch(handleError)
+      .catch(showBoundary)
       .finally(() => {
         abortController = null
       })
@@ -50,7 +50,7 @@ function DiffView() {
     updateRange,
     selectedFile,
     loadingFileDiffs,
-    handleError,
+    showBoundary,
   ])
 
   const diff = useMemo(() => {

services/web/frontend/js/features/history/context/history-context.tsx

@@ -25,7 +25,7 @@ import {
   Update,
 } from '../services/types/update'
 import { Selection } from '../services/types/selection'
-import { useErrorHandler } from 'react-error-boundary'
+import { useErrorBoundary } from 'react-error-boundary'
 import { getUpdateForVersion } from '../utils/history-details'
 import { getHueForUserId } from '@/shared/utils/colors'
 
@@ -99,7 +99,7 @@ function useHistory() {
   )
 
   const updatesAbortControllerRef = useRef<AbortController | null>(null)
-  const handleError = useErrorHandler()
+  const { showBoundary } = useErrorBoundary()
 
   const fetchNextBatchOfUpdates = useCallback(() => {
     // If there is an in-flight request for updates, just let it complete, by
@@ -199,11 +199,11 @@ function useHistory() {
           loadingState: 'ready',
         })
       })
-      .catch(handleError)
+      .catch(showBoundary)
       .finally(() => {
         updatesAbortControllerRef.current = null
       })
-  }, [updatesInfo, projectId, labels, handleError, userHasFullFeature])
+  }, [updatesInfo, projectId, labels, showBoundary, userHasFullFeature])
 
   // Abort in-flight updates request on unmount
   useEffect(() => {
@@ -284,7 +284,7 @@ function useHistory() {
           }
         })
       })
-      .catch(handleError)
+      .catch(showBoundary)
       .finally(() => {
         setLoadingFileDiffs(false)
         abortController = null
@@ -295,7 +295,7 @@ function useHistory() {
         abortController.abort()
       }
     }
-  }, [projectId, fromV, toV, updateForToV, handleError])
+  }, [projectId, fromV, toV, updateForToV, showBoundary])
 
   useEffect(() => {
     // Set update range if there isn't one and updates have loaded

services/web/frontend/js/features/history/context/hooks/use-restore-deleted-file.ts

@@ -4,7 +4,7 @@ import { restoreFile } from '../../services/api'
 import { isFileRemoved } from '../../utils/file-diff'
 import { useHistoryContext } from '../history-context'
 import type { HistoryContextValue } from '../types/history-context-value'
-import { useErrorHandler } from 'react-error-boundary'
+import { useErrorBoundary } from 'react-error-boundary'
 import { useFileTreeData } from '@/shared/context/file-tree-data-context'
 import { findInTree } from '@/features/file-tree/util/find-in-tree'
 import { useCallback, useEffect, useState } from 'react'
@@ -23,7 +23,7 @@ export function useRestoreDeletedFile() {
   const { projectId } = useHistoryContext()
   const { setView } = useLayoutContext()
   const { openDocWithId, openFileWithId } = useEditorManagerContext()
-  const handleError = useErrorHandler()
+  const { showBoundary } = useErrorBoundary()
   const { fileTreeData } = useFileTreeData()
   const [state, setState] = useState<RestorationState>('idle')
   const [restoredFileMetadata, setRestoredFileMetadata] =
@@ -59,14 +59,14 @@ export function useRestoreDeletedFile() {
     if (state === 'waitingForFileTree') {
       const timer = window.setTimeout(() => {
         setState('timedOut')
-        handleError(new Error('timed out'))
+        showBoundary(new Error('timed out'))
       }, 3000)
 
       return () => {
         window.clearTimeout(timer)
       }
     }
-  }, [handleError, state])
+  }, [showBoundary, state])
 
   const restoreDeletedFile = useCallback(
     (selection: HistoryContextValue['selection']) => {
@@ -93,13 +93,13 @@ export function useRestoreDeletedFile() {
             },
             error => {
               setState('error')
-              handleError(error)
+              showBoundary(error)
             }
           )
         }
       }
     },
-    [handleError, projectId]
+    [showBoundary, projectId]
   )
 
   return { restoreDeletedFile, isLoading }

services/web/frontend/js/features/history/context/hooks/use-restore-project.ts

@@ -1,12 +1,12 @@
 import { useCallback, useState } from 'react'
-import { useErrorHandler } from 'react-error-boundary'
+import { useErrorBoundary } from 'react-error-boundary'
 import { restoreProjectToVersion } from '../../services/api'
 import { useLayoutContext } from '@/shared/context/layout-context'
 
 type RestorationState = 'initial' | 'restoring' | 'restored' | 'error'
 
 export const useRestoreProject = () => {
-  const handleError = useErrorHandler()
+  const { showBoundary } = useErrorBoundary()
   const { setView } = useLayoutContext()
 
   const [restorationState, setRestorationState] =
@@ -22,10 +22,10 @@ export const useRestoreProject = () => {
         })
         .catch(err => {
           setRestorationState('error')
-          handleError(err)
+          showBoundary(err)
         })
     },
-    [handleError, setView]
+    [showBoundary, setView]
   )
 
   return {

services/web/frontend/js/features/history/context/hooks/use-restore-selected-file.ts

@@ -3,7 +3,7 @@ import { restoreFileToVersion } from '../../services/api'
 import { isFileRemoved } from '../../utils/file-diff'
 import { useHistoryContext } from '../history-context'
 import type { HistoryContextValue } from '../types/history-context-value'
-import { useErrorHandler } from 'react-error-boundary'
+import { useErrorBoundary } from 'react-error-boundary'
 import { useFileTreeData } from '@/shared/context/file-tree-data-context'
 import { findInTree } from '@/features/file-tree/util/find-in-tree'
 import { useCallback, useEffect, useState } from 'react'
@@ -24,7 +24,7 @@ export function useRestoreSelectedFile() {
   const { projectId } = useHistoryContext()
   const { setView } = useLayoutContext()
   const { openDocWithId, openFileWithId } = useEditorManagerContext()
-  const handleError = useErrorHandler()
+  const { showBoundary } = useErrorBoundary()
   const { fileTreeData } = useFileTreeData()
   const [state, setState] = useState<RestoreState>('idle')
   const [restoredFileMetadata, setRestoredFileMetadata] =
@@ -60,14 +60,14 @@ export function useRestoreSelectedFile() {
     if (state === 'waitingForFileTree') {
       const timer = window.setTimeout(() => {
         setState('timedOut')
-        handleError(new Error('timed out'))
+        showBoundary(new Error('timed out'))
       }, RESTORE_FILE_TIMEOUT)
 
       return () => {
         window.clearTimeout(timer)
       }
     }
-  }, [handleError, state])
+  }, [showBoundary, state])
 
   const restoreSelectedFile = useCallback(
     (selection: HistoryContextValue['selection']) => {
@@ -91,13 +91,13 @@ export function useRestoreSelectedFile() {
             },
             error => {
               setState('error')
-              handleError(error)
+              showBoundary(error)
             }
           )
         }
       }
     },
-    [handleError, projectId]
+    [showBoundary, projectId]
   )
 
   return { restoreSelectedFile, isLoading }

services/web/frontend/js/features/ide-react/components/history-root.tsx

@@ -10,4 +10,6 @@ const HistoryRoot = () => (
   </HistoryProvider>
 )
 
-export default withErrorBoundary(memo(HistoryRoot), ErrorBoundaryFallback)
+export default withErrorBoundary(memo(HistoryRoot), () => (
+  <ErrorBoundaryFallback />
+))

services/web/frontend/js/features/source-editor/components/source-editor.tsx

@@ -16,4 +16,6 @@ function SourceEditor() {
   )
 }
 
-export default withErrorBoundary(memo(SourceEditor), ErrorBoundaryFallback)
+export default withErrorBoundary(memo(SourceEditor), () => (
+  <ErrorBoundaryFallback />
+))

services/web/frontend/js/features/source-editor/hooks/use-codemirror-scope.ts

@@ -33,7 +33,7 @@ import { setSpellCheckLanguage } from '../extensions/spelling'
 import { setKeybindings } from '../extensions/keybindings'
 import { Highlight } from '../../../../../types/highlight'
 import { EditorView } from '@codemirror/view'
-import { useErrorHandler } from 'react-error-boundary'
+import { useErrorBoundary } from 'react-error-boundary'
 import { setVisual } from '../extensions/visual/visual'
 import { useFileTreePathContext } from '@/features/file-tree/contexts/file-tree-path'
 import { useUserSettingsContext } from '@/shared/context/user-settings-context'
@@ -265,7 +265,7 @@ function useCodeMirrorScope(view: EditorView) {
     visual: showVisual,
   })
 
-  const handleError = useErrorHandler()
+  const { showBoundary } = useErrorBoundary()
 
   const handleException = useCallback((exception: any) => {
     captureException(exception, {
@@ -304,7 +304,7 @@ function useCodeMirrorScope(view: EditorView) {
           spelling: spellingRef.current,
           visual: visualRef.current,
           projectFeatures: projectFeaturesRef.current,
-          handleError,
+          showBoundary,
           handleException,
         }),
       })
@@ -335,7 +335,7 @@ function useCodeMirrorScope(view: EditorView) {
     }
     // IMPORTANT: This effect must not depend on anything variable apart from currentDocument,
     // as the editor state is recreated when the effect runs.
-  }, [view, currentDocument, handleError, handleException])
+  }, [view, currentDocument, showBoundary, handleException])
 
   useEffect(() => {
     if (openDocName) {

services/web/frontend/js/infrastructure/error-boundary.jsx

@@ -1,37 +0,0 @@
-import { captureException } from './error-reporter'
-import { ErrorBoundary } from 'react-error-boundary'
-
-function errorHandler(error, componentStack) {
-  captureException(error, {
-    extra: {
-      componentStack,
-    },
-    tags: {
-      handler: 'react-error-boundary',
-    },
-  })
-}
-
-function DefaultFallbackComponent() {
-  return <></>
-}
-
-function withErrorBoundary(WrappedComponent, FallbackComponent) {
-  function ErrorBoundaryWrapper(props) {
-    return (
-      <ErrorBoundary
-        FallbackComponent={FallbackComponent || DefaultFallbackComponent}
-        onError={errorHandler}
-      >
-        <WrappedComponent {...props} />
-      </ErrorBoundary>
-    )
-  }
-  ErrorBoundaryWrapper.propTypes = WrappedComponent.propTypes
-  ErrorBoundaryWrapper.displayName = `WithErrorBoundaryWrapper${
-    WrappedComponent.displayName || WrappedComponent.name || 'Component'
-  }`
-  return ErrorBoundaryWrapper
-}
-
-export default withErrorBoundary

services/web/frontend/js/infrastructure/error-boundary.tsx

@@ -0,0 +1,31 @@
+import { captureException } from './error-reporter'
+import { withErrorBoundary as rebWithErrorBoundary } from 'react-error-boundary'
+import { ComponentType, ErrorInfo } from 'react'
+import { FallbackProps } from 'react-error-boundary/dist/declarations/src/types'
+
+function errorHandler(error: Error, errorInfo: ErrorInfo) {
+  captureException(error, {
+    extra: {
+      componentStack: errorInfo.componentStack,
+    },
+    tags: {
+      handler: 'react-error-boundary',
+    },
+  })
+}
+
+function DefaultFallbackComponent() {
+  return <></>
+}
+
+function withErrorBoundary(
+  WrappedComponent: ComponentType<any>,
+  FallbackComponent?: ComponentType<FallbackProps>
+) {
+  return rebWithErrorBoundary(WrappedComponent, {
+    onError: errorHandler,
+    FallbackComponent: FallbackComponent || DefaultFallbackComponent,
+  })
+}
+
+export default withErrorBoundary

services/web/package.json

@@ -333,7 +333,7 @@
     "react-dnd": "^16.0.1",
     "react-dnd-html5-backend": "^16.0.1",
     "react-dom": "^17.0.2",
-    "react-error-boundary": "^2.3.1",
+    "react-error-boundary": "^5.0.0",
     "react-google-recaptcha": "^3.1.0",
     "react-i18next": "^13.3.1",
     "react-linkify": "^1.0.0-alpha",

services/web/test/frontend/features/history/components/change-list.spec.tsx

@@ -1,4 +1,4 @@
-import { useState } from 'react'
+import { useState, FC } from 'react'
 import ToggleSwitch from '../../../../../frontend/js/features/history/components/change-list/toggle-switch'
 import ChangeList from '../../../../../frontend/js/features/history/components/change-list/change-list'
 import {
@@ -10,13 +10,14 @@ import { HistoryProvider } from '../../../../../frontend/js/features/history/con
 import { updates } from '../fixtures/updates'
 import { labels } from '../fixtures/labels'
 import { formatTime, relativeDate } from '@/features/utils/format-date'
+import { withTestContainerErrorBoundary } from '../../../helpers/error-boundary'
 
-const mountWithEditorProviders = (
-  component: React.ReactNode,
-  scope: Record<string, unknown> = {},
-  props: Record<string, unknown> = {}
-) => {
-  cy.mount(
+const TestContainerWithoutErrorBoundary: FC<{
+  component: React.ReactNode
+  scope: Record<string, unknown>
+  props: Record<string, unknown>
+}> = ({ component, scope, props }) => {
+  return (
     <EditorProviders scope={scope} {...props}>
       <HistoryProvider>
         <div style={{ display: 'flex', justifyContent: 'center' }}>
@@ -27,6 +28,18 @@ const mountWithEditorProviders = (
   )
 }
 
+const TestContainer = withTestContainerErrorBoundary(
+  TestContainerWithoutErrorBoundary
+)
+
+const mountWithEditorProviders = (
+  component: React.ReactNode,
+  scope: Record<string, unknown> = {},
+  props: Record<string, unknown> = {}
+) => {
+  cy.mount(<TestContainer component={component} scope={scope} props={props} />)
+}
+
 describe('change list (Bootstrap 5)', function () {
   const scope = {
     ui: { view: 'history', pdfLayout: 'sideBySide', chatOpen: true },

services/web/test/frontend/features/history/components/toolbar.spec.tsx

@@ -3,6 +3,28 @@ import { HistoryProvider } from '../../../../../frontend/js/features/history/con
 import { HistoryContextValue } from '../../../../../frontend/js/features/history/context/types/history-context-value'
 import { Diff } from '../../../../../frontend/js/features/history/services/types/doc'
 import { EditorProviders } from '../../../helpers/editor-providers'
+import { FC } from 'react'
+import { withTestContainerErrorBoundary } from '../../../helpers/error-boundary'
+
+const TestContainerWithoutErrorBoundary: FC<{
+  scope: Record<string, unknown>
+  diff: Diff
+  selection: HistoryContextValue['selection']
+}> = ({ scope, diff, selection }) => {
+  return (
+    <EditorProviders scope={scope}>
+      <HistoryProvider>
+        <div className="history-react">
+          <Toolbar diff={diff} selection={selection} />
+        </div>
+      </HistoryProvider>
+    </EditorProviders>
+  )
+}
+
+const TestContainer = withTestContainerErrorBoundary(
+  TestContainerWithoutErrorBoundary
+)
 
 describe('history toolbar', function () {
   const editorProvidersScope = {
@@ -58,13 +80,11 @@ describe('history toolbar', function () {
     }
 
     cy.mount(
-      <EditorProviders scope={editorProvidersScope}>
-        <HistoryProvider>
-          <div className="history-react">
-            <Toolbar diff={diff} selection={selection} />
-          </div>
-        </HistoryProvider>
-      </EditorProviders>
+      <TestContainer
+        scope={editorProvidersScope}
+        diff={diff}
+        selection={selection}
+      />
     )
 
     cy.get('.history-react-toolbar').within(() => {
@@ -108,13 +128,11 @@ describe('history toolbar', function () {
     }
 
     cy.mount(
-      <EditorProviders scope={editorProvidersScope}>
-        <HistoryProvider>
-          <div className="history-react">
-            <Toolbar diff={diff} selection={selection} />
-          </div>
-        </HistoryProvider>
-      </EditorProviders>
+      <TestContainer
+        scope={editorProvidersScope}
+        diff={diff}
+        selection={selection}
+      />
     )
 
     cy.get('.history-react-toolbar').within(() => {

services/web/test/frontend/features/source-editor/helpers/test-container.tsx

@@ -1,12 +1,18 @@
-import { FC, ComponentProps, Suspense } from 'react'
+import { FC, ComponentProps, PropsWithChildren, Suspense } from 'react'
+import { withTestContainerErrorBoundary } from '../../../helpers/error-boundary'
 
 const style = { width: 785, height: 785 }
 
-export const TestContainer: FC<ComponentProps<'div'>> = ({
-  children,
-  ...rest
-}) => (
+const TestContainerWithoutErrorBoundary: FC<
+  PropsWithChildren<ComponentProps<'div'>>
+> = ({ children, ...rest }) => (
   <div style={style} {...rest}>
     <Suspense fallback={null}>{children}</Suspense>
   </div>
 )
+
+// react-error-boundary version 5 requires an error boundary when using
+// useErrorBoundary, which we do in several components
+export const TestContainer = withTestContainerErrorBoundary(
+  TestContainerWithoutErrorBoundary
+)

services/web/test/frontend/helpers/error-boundary.tsx

@@ -0,0 +1,12 @@
+import { ComponentType, FC } from 'react'
+import withErrorBoundary from '@/infrastructure/error-boundary'
+
+const FallbackComponent: FC = () => {
+  return <>An error occurred within the test container</>
+}
+
+export const withTestContainerErrorBoundary = function <T>(
+  Component: ComponentType<T>
+) {
+  return withErrorBoundary(Component, FallbackComponent)
+}