fix(react): fix throw null (#151)

* fix(react): fix throw null

* Create grumpy-turtles-tickle.md

* test(react): add test case for ErrorBoundary catching null thrown

* chore(react): update changeset

* Update packages/react/src/utils/toTest.tsx

Co-authored-by: tommy(Chung-il) <[email protected]>

* chore: update

---------

Co-authored-by: tommy(Chung-il) <[email protected]>

.changeset/grumpy-turtles-tickle.md

@@ -0,0 +1,5 @@
+---
+"@suspensive/react": patch
+---
+
+fix(react): ErrorBoundary should catch null thrown by children

packages/react/src/ErrorBoundary.spec.tsx

@@ -1,7 +1,7 @@
 import { act } from '@testing-library/react'
 import { ComponentProps, ComponentRef, createRef } from 'react'
 import { createRoot } from 'react-dom/client'
-import { ERROR_MESSAGE, FALLBACK, MS_100, TEXT, ThrowError } from './utils/toTest'
+import { ERROR_MESSAGE, FALLBACK, MS_100, TEXT, ThrowError, ThrowNull } from './utils/toTest'
 import { ErrorBoundary } from '.'
 
 let container = document.createElement('div')
@@ -60,6 +60,23 @@ describe('ErrorBoundary', () => {
     expect(container.textContent).not.toBe(TEXT)
   })
 
+  it('should catch it even if thrown null', () => {
+    const onError = vi.fn()
+    vi.useFakeTimers()
+    renderErrorBoundary({
+      onError,
+      fallback: <>{FALLBACK}</>,
+      children: <ThrowNull after={MS_100}>{TEXT}</ThrowNull>,
+    })
+    expect(container.textContent).toBe(TEXT)
+    expect(container.textContent).not.toBe(FALLBACK)
+    expect(onError).toHaveBeenCalledTimes(0)
+    act(() => vi.advanceTimersByTime(MS_100))
+    expect(container.textContent).toBe(FALLBACK)
+    expect(container.textContent).not.toBe(TEXT)
+    expect(onError).toHaveBeenCalledTimes(1)
+  })
+
   it('should be reset by items of resetKeys, and call onReset', () => {
     const onReset = vi.fn()
     vi.useFakeTimers()

packages/react/src/ErrorBoundary.tsx

@@ -47,26 +47,32 @@ type ErrorBoundaryProps = PropsWithChildren<{
   fallback: NonNullable<ReactNode> | FunctionComponent<ErrorBoundaryFallbackProps>
 }>
 
-type ErrorBoundaryState = {
-  error: Error | null
-}
+type ErrorBoundaryState<TError extends Error = Error> =
+  | {
+      isError: true
+      error: TError
+    }
+  | {
+      isError: false
+      error: null
+    }
 
 const initialState: ErrorBoundaryState = {
+  isError: false,
   error: null,
 }
 
 class BaseErrorBoundary extends Component<ErrorBoundaryProps, ErrorBoundaryState> {
-  static getDerivedStateFromError(error: Error) {
-    return { error }
+  static getDerivedStateFromError(error: Error): ErrorBoundaryState {
+    return { isError: true, error }
   }
 
   state = initialState
 
   componentDidUpdate(prevProps: ErrorBoundaryProps, prevState: ErrorBoundaryState) {
-    const { error } = this.state
+    const { isError } = this.state
     const { resetKeys } = this.props
-
-    if (error !== null && prevState.error !== null && hasResetKeysChanged(prevProps.resetKeys, resetKeys)) {
+    if (isError && prevState.isError && hasResetKeysChanged(prevProps.resetKeys, resetKeys)) {
       this.resetErrorBoundary()
     }
   }
@@ -86,10 +92,10 @@ class BaseErrorBoundary extends Component<ErrorBoundaryProps, ErrorBoundaryState
   }
 
   render() {
-    const { error } = this.state
+    const { isError, error } = this.state
     const { children, fallback } = this.props
 
-    if (error !== null) {
+    if (isError) {
       if (typeof fallback === 'function') {
         return createElement(fallback, {
           error,

packages/react/src/utils/toTest.tsx

@@ -29,6 +29,20 @@ export const ThrowError = ({ message, after, children }: ThrowErrorProps) => {
   }, [after])
   return <>{children}</>
 }
+
+type ThrowNullProps = PropsWithChildren<{ after: number }>
+export const ThrowNull = ({ after, children }: ThrowNullProps) => {
+  const [isNeedError, setIsNeedError] = useState(throwErrorIsNeed.current)
+  if (isNeedError) {
+    throw null
+  }
+  useEffect(() => {
+    const timerId = setTimeout(() => setIsNeedError(true), after)
+    return () => clearTimeout(timerId)
+  }, [after])
+  return <>{children}</>
+}
+
 ThrowError.reset = () => {
   throwErrorIsNeed.current = false
 }