Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: fix #189 remove the redundant uncaught error and error stack trace from the console #191

Closed
wants to merge 7 commits into from
Closed

fix: fix #189 remove the redundant uncaught error and error stack trace from the console #191

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
1cb275b
fix: fix #189 remove the redundant uncaught error and error stack tra…
benhuangbmj Aug 5, 2024
08bb7fc
change: wrap the event listener attaching in the lifecycle methods
benhuangbmj Aug 14, 2024
bcc460f
feat: add suppressLogging to the prop type
benhuangbmj Aug 14, 2024
195a724
refactor: rename isShowBoundary to _suppressLogging
benhuangbmj Aug 14, 2024
a14b929
change: add the suppressLogging prop and the handleSuppressLogging fu…
benhuangbmj Aug 25, 2024
0e6bd3b
change: remove the suppress-logging logic from ErrorBoundary.ts
benhuangbmj Aug 25, 2024
abca669
change: wrap the suppress-logging logic in the showBoundary function
benhuangbmj Aug 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions src/ErrorBoundary.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,13 @@ const initialState: ErrorBoundaryState = {
error: null,
};

function handleSuppressLogging(event: ErrorEvent) {
if (event.error._suppressLogging) event.preventDefault();
}
function handleRestoreLogging(event: ErrorEvent) {
if (event.error._suppressLogging) event.error._suppressLogging = false;
}

export class ErrorBoundary extends Component<
ErrorBoundaryProps,
ErrorBoundaryState
Expand All @@ -29,6 +36,9 @@ export class ErrorBoundary extends Component<
this.state = initialState;
}

static defaultProps = {
suppressLogging: false,
bvaughn marked this conversation as resolved.
Show resolved Hide resolved
};
static getDerivedStateFromError(error: Error) {
return { didCatch: true, error };
}
Expand All @@ -46,8 +56,30 @@ export class ErrorBoundary extends Component<
}
}

componentDidMount() {
window.addEventListener("error", handleSuppressLogging);
benhuangbmj marked this conversation as resolved.
Show resolved Hide resolved
}

componentWillUnmount() {
window.removeEventListener("error", handleSuppressLogging);
window.removeEventListener("error", handleRestoreLogging);
}

shouldComponentUpdate(
nextProps: ErrorBoundaryProps,
nextState: ErrorBoundaryState
) {
if (!this.props.suppressLogging && nextState.didCatch) {
window.removeEventListener("error", handleSuppressLogging);
window.addEventListener("error", handleRestoreLogging);
}
benhuangbmj marked this conversation as resolved.
Show resolved Hide resolved
return true;
}

componentDidCatch(error: Error, info: ErrorInfo) {
this.props.onError?.(error, info);
window.removeEventListener("error", handleRestoreLogging);
window.addEventListener("error", handleSuppressLogging);
}

componentDidUpdate(
Expand Down
1 change: 1 addition & 0 deletions src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ type ErrorBoundarySharedProps = PropsWithChildren<{
| { reason: "keys"; prev: any[] | undefined; next: any[] | undefined }
) => void;
resetKeys?: any[];
suppressLogging?: boolean;
}>;

export type ErrorBoundaryPropsWithComponent = ErrorBoundarySharedProps & {
Expand Down
39 changes: 35 additions & 4 deletions src/useErrorBoundary.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,45 @@ import { useContext, useMemo, useState } from "react";
import { assertErrorBoundaryContext } from "./assertErrorBoundaryContext";
import { ErrorBoundaryContext } from "./ErrorBoundaryContext";

type StateError<TError> =
| (TError & { _suppressLogging: true })
| (Error & { _suppressLogging: true });
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like this intentionally breaks the generics type for the hook. I guess you had to do this, to support the non-object case in suppressLoggingForError but if that's the case we should either remove the generic parameter from suppressLoggingForError (since it's not a guarantee) or maybe change it to require extending Error (so then the non-object case in suppressLoggingForError would be unsupported from a types perspective at least)?

function suppressLoggingForError<ErrorType extends Error>(error: ErrorType): StateError<ErrorType> {

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am sorry, but I don't quite get this one. Why removing the generic parameter from suppressLoggingForError would be of help? I noticed that in the generic parameter of useErrorBoundary, <TError = any>, so I assume TError can be very flexible yet maintaining it can assure some basic type check. Please shed more light on this.


type UseErrorBoundaryState<TError> =
| { error: TError; hasError: true }
| { error: StateError<TError>; hasError: true }
| { error: null; hasError: false };

export type UseErrorBoundaryApi<TError> = {
resetBoundary: () => void;
showBoundary: (error: TError) => void;
};

function suppressLoggingForError<TError>(error: TError): StateError<TError> {
if (error != null) {
switch (typeof error) {
case "object": {
if (Object.isExtensible(error)) {
(error as StateError<TError>)._suppressLogging = true;
return error as StateError<TError>;
} else {
return Object.assign(Object.create(error as object), {
_suppressLogging: true as true,
});
}
}
default: {
return Object.assign(Error(error as string), {
_suppressLogging: true as true,
});
}
}
} else {
return Object.assign(Error(undefined), {
_suppressLogging: true as true,
});
}
}

export function useErrorBoundary<TError = any>(): UseErrorBoundaryApi<TError> {
const context = useContext(ErrorBoundaryContext);

Expand All @@ -27,11 +57,12 @@ export function useErrorBoundary<TError = any>(): UseErrorBoundaryApi<TError> {
context.resetErrorBoundary();
setState({ error: null, hasError: false });
},
showBoundary: (error: TError) =>
showBoundary: (error: TError) => {
setState({
error,
error: suppressLoggingForError(error),
hasError: true,
}),
});
},
}),
[context.resetErrorBoundary]
);
Expand Down