fix(react): prevent rendering fallback if not shouldCatch error (#1410)

Co-authored-by: lucas0530 <23375098+lucas0530@users.noreply.github.com>
Co-authored-by: HYUNGU KANG <69349293+linegu@users.noreply.github.com>

.changeset/friendly-dolphins-reflect.md

@@ -0,0 +1,5 @@
+---
+"@suspensive/react": patch
+---
+
+fix(react): prevent rendering fallback if not shouldCatch error

examples/visualization/src/app/layout.tsx

@@ -31,6 +31,11 @@ export default function RootLayout({ children }: { children: React.ReactNode })
                 <li>
                   <Link href="/react/ErrorBoundary/shouldCatch">{`<ErrorBoundary/>`} shouldCatch prop</Link>
                 </li>
+                <li>
+                  <Link href="/react/ErrorBoundary/shouldCatch/renderPhase">
+                    {`<ErrorBoundary/>`} shouldCatch prop render phase
+                  </Link>
+                </li>
                 <li>
                   <Link href="/react/zodSearchParams">zod: no param</Link>
                 </li>

examples/visualization/src/app/react/ErrorBoundary/shouldCatch/renderPhase/page.tsx

@@ -0,0 +1,22 @@
+'use client'
+
+import { ErrorBoundary } from '@suspensive/react'
+import { Throw } from '~/components/Throw'
+
+export default function page() {
+  return (
+    <ErrorBoundary fallback={() => <>root fallback</>}>
+      <ErrorBoundary
+        shouldCatch={(error) => error.message !== 'children error message'}
+        fallback={() => {
+          console.log("child ErrorBoundary's fallback")
+          return <>child ErrorBoundary's fallback</>
+        }}
+      >
+        <Throw.Error message="children error message" after={2000}>
+          before throw Error
+        </Throw.Error>
+      </ErrorBoundary>
+    </ErrorBoundary>
+  )
+}

examples/visualization/src/components/Throw.tsx

@@ -0,0 +1,13 @@
+import { type PropsWithChildren, useState } from 'react'
+import { useTimeout } from '~/hooks'
+
+export const Throw = {
+  Error: ({ message, after = 0, children }: PropsWithChildren<{ message: string; after?: number }>) => {
+    const [isNeedThrow, setIsNeedThrow] = useState(after === 0)
+    if (isNeedThrow) {
+      throw new Error(message)
+    }
+    useTimeout(() => setIsNeedThrow(true), after)
+    return <>{children}</>
+  },
+}

examples/visualization/src/hooks/index.ts

@@ -0,0 +1 @@
+export { useTimeout } from './useTimeout'

examples/visualization/src/hooks/useTimeout.ts

@@ -0,0 +1,11 @@
+import { useCallback, useEffect, useRef } from 'react'
+
+export const useTimeout = (fn: () => void, ms: number) => {
+  const fnRef = useRef(fn)
+  fnRef.current = fn
+  const fnPreserved = useCallback(() => fnRef.current(), [])
+  useEffect(() => {
+    const id = setTimeout(fnPreserved, ms)
+    return () => clearTimeout(id)
+  }, [fnPreserved, ms])
+}

packages/react/src/ErrorBoundary.spec.tsx

@@ -187,6 +187,26 @@ describe('<ErrorBoundary/>', () => {
     await waitFor(() => expect(screen.queryByText(`${ERROR_MESSAGE} of Parent`)).toBeInTheDocument())
   })
 
+  it('should re-throw error if not shouldCatch error in children without rendering fallback', async () => {
+    const onErrorParent = vi.fn()
+    const onErrorChild = vi.fn()
+    const Fallback = vi.fn()
+
+    render(
+      <ErrorBoundary fallback={({ error }) => <>{error.message} of Parent</>} onError={onErrorParent}>
+        <ErrorBoundary shouldCatch={false} fallback={Fallback} onError={onErrorChild}>
+          {createElement(() => {
+            throw new Error(ERROR_MESSAGE)
+          })}
+        </ErrorBoundary>
+      </ErrorBoundary>
+    )
+
+    expect(onErrorChild).toBeCalledTimes(0)
+    expect(onErrorParent).toBeCalledTimes(1)
+    await waitFor(() => expect(screen.queryByText(`${ERROR_MESSAGE} of Parent`)).toBeInTheDocument())
+  })
+
   it.each([
     {
       childCalledTimes: 0,

packages/react/src/ErrorBoundary.tsx

@@ -118,13 +118,6 @@ class BaseErrorBoundary extends Component<ErrorBoundaryProps, ErrorBoundaryState
     if (error instanceof SuspensiveError) {
       throw error
     }
-    const { shouldCatch = true } = this.props
-    const isCatch = Array.isArray(shouldCatch)
-      ? shouldCatch.some((shouldCatch) => checkErrorBoundary(shouldCatch, error))
-      : checkErrorBoundary(shouldCatch, error)
-    if (!isCatch) {
-      throw error
-    }
     this.props.onError?.(error, info)
   }
 
@@ -134,12 +127,19 @@ class BaseErrorBoundary extends Component<ErrorBoundaryProps, ErrorBoundaryState
   }
 
   render() {
-    const { children, fallback } = this.props
+    const { children, fallback, shouldCatch = true } = this.props
     const { isError, error } = this.state
 
     let childrenOrFallback = children
 
     if (isError) {
+      const isCatch = Array.isArray(shouldCatch)
+        ? shouldCatch.some((shouldCatch) => checkErrorBoundary(shouldCatch, error))
+        : checkErrorBoundary(shouldCatch, error)
+      if (!isCatch) {
+        throw error
+      }
+
       if (typeof fallback === 'undefined') {
         if (process.env.NODE_ENV === 'development') {
           console.error('ErrorBoundary of @suspensive/react requires a defined fallback')