feat(react): add useErrorBoundaryFallbackProps

[manudeli]

related with #200

Overview

I added useErrorBoundaryFallbackProps to make useErrorBoundary don't have role in fallback of <ErrorBoundary/>

Cause of adding useErrorBoundaryFallbackProps

1. TypeScriptly, existing useErrorBoundary().isError can be true in children of <ErrorBoundary/>, but actually runtimely it's not.

   const Example = withErrorBoundary(() => {
     const errorBoundary = useErrorBoundary()
     errorBoundary.isError // there cannot be error, but isError can be true TypeScriptly
   }, {
     fallback: () => <></>
   })

2. TypeScriptly, existing useErrorBoundary().isError can be false in fallback of <ErrorBoundary/>, but actually runtimely there must have error in fallback of <ErrorBoundary/>

   const Example = withErrorBoundary(() => {
     
   }, {
     fallback: function ErrorBoundaryFallback() => {
       const errorBoundary = useErrorBoundary()
       errorBoundary.isError // there must be error, but isError can be false TypeScriptly
       return <></>
     }
   })

3. There could be useErrorBoundary().reset in children of <ErrorBoundary/>, but it didn't work in children because there is no error to reset in children of <ErrorBoundary/>.

   const Example = withErrorBoundary(() => {
     const errorBoundary = useErrorBoundary()
     errorBoundary.reset // there is reset method but this is not working for children of `<ErrorBoundary/>`
   }, {
     fallback: () => <></>
   })

Solution

I think both children / fallback of <ErrorBoundary/> should have separate hook for each section. so I made useErrorBoundaryFallbackProps separate from useErrorBoundary

• useErrorBoundary(hook for children of <ErrorBoundary/>)
  • everytime no error
  • no reset function
  • can setError
• useErrorBoundaryFallbackProps(hook for fallback of <ErrorBoundary/>)
  • everytime have error
  • reset function working

PR Checklist

• I have written documents and tests, if needed.

[changeset-bot]

🦋 Changeset detected

Latest commit: 5d72fb0

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages

Name	Type
@suspensive/react	Minor
@suspensive/react-query	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
docs	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Oct 18, 2023 9:41am
docs-legacy	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Oct 18, 2023 9:41am
visualization	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Oct 18, 2023 9:41am

[codecov]

Codecov Report

Merging #216 (5d72fb0) into main (33f6f07) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              main      #216   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           21        21           
  Lines          829       842   +13     
  Branches       146       149    +3     
=========================================
+ Hits           829       842   +13     

Components	Coverage Δ
@suspensive/react	100.00% <100.00%> (ø)
@suspensive/react-query	∅ <ø> (∅)

[minsoo-web]

I think this hook is very convenient, but how about provide props type guide like this?

import { ErrorBoundaryFallbackProps } from '@suspensive/react'

const Nested = ({ reset, error }: ErrorBoundaryFallbackProps) => {
  return (
    <>
      <button onClick={reset}>reset</button>
      {error.message}
    </>
  )
}

This is exactly same, but can't avoid props drilling.
So, my suggestion is, before provide example that can avoid props drilling case, first show simple example.

---

And How about add Errorboundary component's visualization page and link it?

hundred words are worth a glance. In korea 백문이불여일견

[manudeli]

I think this hook is very convenient, but how about provide props type guide like this?

import { ErrorBoundaryFallbackProps } from '@suspensive/react'

const Nested = ({ reset, error }: ErrorBoundaryFallbackProps) => {
  return (
    <>
      <button onClick={reset}>reset</button>
      {error.message}
    </>
  )
}

This is exactly same, but can't avoid props drilling.
So, my suggestion is, before provide example that can avoid props drilling case, first show simple example.

---

And How about add Errorboundary component's visualization page and link it?

hundred words are worth a glance. In korea 백문이불여일견

@minsoo-web could you add suggestion code or Pull Request to this branch please? I want to know what your intend exactly

[minsoo-web]

@minsoo-web could you add suggestion code or Pull Request to this branch please? I want to know what your intend exactly

I made PR for this case #221
Please review this PR ! 🙇‍♂️

[SEOKKAMONI]

I referenced to #221, how about this for document commonization?

[minsoo-web]

LGTM 👍

[okinawaa]

If it's for the purpose of providing typing

Wouldn't it be better for the developer experience to define not only the type of props but also the return?

Suggested change

	import type { ErrorBoundaryFallbackProps } from '@suspensive/react'
	const ErrorBoundaryFallback = ({ reset, error }: ErrorBoundaryFallbackProps) => (
	<>
	<button onClick={reset}>reset</button>
	{error.message}
	</>
	)
	const Example = () => (
	<ErrorBoundary fallback={ErrorBoundaryFallback}>
	<ErrorAfter4s />
	</ErrorBoundary>
	)
	import type { ErrorBoundaryFallbackType } from '@suspensive/react'
	const ErrorBoundaryFallback: ErrorBoundaryFallbackType = ({ reset, error }) => (
	<> // The type of return value is also certain
	<button onClick={reset}>reset</button>
	{error.message}
	</>
	)
	const Example = () => (
	<ErrorBoundary fallback={ErrorBoundaryFallback}>
	<ErrorAfter4s />
	</ErrorBoundary>
	)

I inspried by nextjs GetServerSideProps

[manudeli]

Wouldn't it be better for the developer experience to define not only the type of props but also the return?

I think that's a good idea too. However, even if we do not provide the return type, TypeScript will create a type error if it does not match the fallback type of ErrorBoundary.

I think we need to consider whether providing a return type is right choice. Because the return type may be different from the actual thing returned (this video explain it well). If it is as essential as ErrorBoundaryFallbackProps, it would be a good idea to add it as a minor version feature.

Let's consider it after merging this