No only Error for FallbackProps

[viclafouch]

I'm using React-query, and we can type our error. But there is a conflit with react-error-boundary since the type is not an Error type, but a custom. How can we deal with that ?

• react-error-boundary version: 3.1.0
• node version: 12.19.1
• npm version: 6.14.10

Relevant code or config

const OrderDetailsViews = (
  props: RouteComponentProps<TParams>
): React.ReactElement => {
  const { match } = props
  const { t } = useTranslation()
  const orderId = Number(match.params.id)
  const handleError = useErrorHandler()

  const { data: order, error, refetch: refetchOrders } = useQuery<
    FindOneOrder,
    ApiError
  >(['order-details', orderId], () => api.orders.findOneComplete(orderId), {
    onError: err => handleError(err) <-- ERROR TYPE
  })
}

Suggested solution: We can pass our type like : handleError<ApiError>(err)

OR

interface FallbackProps<T extends Error> { <-- We can pass something else
    error: T;
    resetErrorBoundary: (...args: Array<unknown>) => void;
}

[kentcdodds]

That suggested solution sounds fine to me 👍

[tommarien]

What is the need for it to be an error ? and not unknown like in node?

try {} catch(e: unknown){}

Also see #36 and #25

[tommarien]

Is it ok if i try to change the error type to unknown in a pull request, in the next coming days? Something in the order of this

import React, { useCallback } from "react";

function useErrorHandler(givenError?: unknown): (error: unknown) => void {
  const [error, setError] = React.useState<unknown>(givenError ?? null);
  if (error !== null) throw error;

  const onError = useCallback(
    (err: unknown) => {
      setError(err ?? null);
    },
    []
  );

  return onError;
}

Would you guys accept a pull containing this change ?

[kentcdodds]

I'm not sure why the useCallback is necessary... And this would break the useErrorHandler(error) API. Why couldn't the change just be:

function useErrorHandler(
  givenError?: unknown,
): React.Dispatch<React.SetStateAction<unknown>> {
  const [error, setError] = React.useState<unknown>(null)
  if (givenError) throw givenError
  if (error) throw error
  return setError
}

[kentcdodds]

In fact, I think it could be further simplified to this:

function useErrorHandler(givenError?: unknown) {
  const [error, setError] = React.useState<unknown>(null)
  if (givenError) throw givenError
  if (error) throw error
  return setError
}

The return type can be inferred without any trouble.

[tommarien]

Should try to explain why i did things ;)

1. Explicit return type, as it is a boundary method i included this change (which could be seen as a contract breach, but the end result is the same only closed

// inferred type
React.Dispatch<React.SetStateAction<unknown>>

// preferred type
(error:unknown) => void

2. Removed duplication between given error and state error, moved the givenerror (i presumed its something like initialError)

function useErrorHandler(givenError?: unknown) {
  const [error, setError] = React.useState<unknown>(null)
  if (givenError) throw givenError
  if (error) throw error
}

ends up in:

function useErrorHandler(givenError?: unknown) {
  const [error, setError] = React.useState<unknown>(givenError)
  if (error) throw error
}

3. The callback/ ?? null shenanigan
   In javascript you could technically throw all falsy values like null, undefined, but also 0, false and '', consider following scenario

const onError = useErrorHandler();

useEffect(()=>{
  function async fetchy() {
    promise.catch(onError);
  }
  fetchy();
}, [onError])

What would happen if the promise was rejected with ''. In the change i made i ensure everything except undefined and null would be rethrown

Sorry should have given explaination in the first place. For me the minimum change you did is also fine but just wanted to give my angle on it ;)

[kentcdodds]

1. Explicit return type

I guess that's ok

2. Removed duplication

That's not duplication. The suggested change would actually change the behavior. The givenError could be changed by the calling code and we want to capture that.

3. The callback/ ?? null shenanigan

I guess that makes sense too.

With that, here's a good implementation:

function useErrorHandler(givenError?: unknown): (error: unknown) => void {
  const [error, setError] = React.useState<unknown>(null)
  if (givenError != null) throw givenError
  if (error != null) throw error
  return setError
}

[kentcdodds]

And I would consider this a bug fix

[tommarien]

@kentcdodds thanks for the assist and discussion ;), will try to wip up a pull as soon as possible

[github-actions]

🎉 This issue has been resolved in version 3.1.3 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀