-
-
Notifications
You must be signed in to change notification settings - Fork 221
fix: fix #189 remove the redundant uncaught error and error stack trace from the console #191
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.
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
Changes from all commits
1cb275b
08bb7fc
bcc460f
195a724
a14b929
0e6bd3b
abca669
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,6 +18,10 @@ const initialState: ErrorBoundaryState = { | |
| error: null, | ||
| }; | ||
|
|
||
| function handleSuppressLogging(event: ErrorEvent) { | ||
| if (event.error._suppressLogging) event.preventDefault(); | ||
| } | ||
|
|
||
| export class ErrorBoundary extends Component< | ||
| ErrorBoundaryProps, | ||
| ErrorBoundaryState | ||
|
|
@@ -29,6 +33,10 @@ export class ErrorBoundary extends Component< | |
| this.state = initialState; | ||
| } | ||
|
|
||
| static defaultProps = { | ||
| suppressLogging: false, | ||
| }; | ||
|
|
||
| static getDerivedStateFromError(error: Error) { | ||
| return { didCatch: true, error }; | ||
| } | ||
|
|
@@ -46,8 +54,14 @@ export class ErrorBoundary extends Component< | |
| } | ||
| } | ||
|
|
||
| componentWillUnmount() { | ||
| window.removeEventListener("error", handleSuppressLogging); | ||
| } | ||
|
|
||
| componentDidCatch(error: Error, info: ErrorInfo) { | ||
| this.props.onError?.(error, info); | ||
| window.addEventListener("error", () => {}); // Some test cases will fail without this line, somehow. | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can't do this. It would break uncaught exception behavior (see facebook/react#28515) and cause a memory leak (since
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moving forward, I guess I would dive deeper into the React Internals and try to understand how the error handling works in there. Thanks. |
||
| window.removeEventListener("error", handleSuppressLogging); | ||
| } | ||
|
|
||
| componentDidUpdate( | ||
|
|
@@ -114,6 +128,8 @@ export class ErrorBoundary extends Component< | |
| didCatch, | ||
| error, | ||
| resetErrorBoundary: this.resetErrorBoundary, | ||
| suppressLogging: this.props.suppressLogging as boolean, | ||
| handleSuppressLogging, | ||
| }, | ||
| }, | ||
| childToRender | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,8 +2,12 @@ import { useContext, useMemo, useState } from "react"; | |
| import { assertErrorBoundaryContext } from "./assertErrorBoundaryContext"; | ||
| import { ErrorBoundaryContext } from "./ErrorBoundaryContext"; | ||
|
|
||
| type StateError<TError> = | ||
| | (TError & { _suppressLogging: boolean }) | ||
| | (Error & { _suppressLogging: boolean }); | ||
|
|
||
| type UseErrorBoundaryState<TError> = | ||
| | { error: TError; hasError: true } | ||
| | { error: StateError<TError>; hasError: true } | ||
| | { error: null; hasError: false }; | ||
|
|
||
| export type UseErrorBoundaryApi<TError> = { | ||
|
|
@@ -16,6 +20,35 @@ export function useErrorBoundary<TError = any>(): UseErrorBoundaryApi<TError> { | |
|
|
||
| assertErrorBoundaryContext(context); | ||
|
|
||
| const suppressLoggingForError = function <TError>( | ||
| error: TError | ||
| ): StateError<TError> { | ||
| const suppressLogging = { | ||
| _suppressLogging: context.suppressLogging as boolean, | ||
| }; | ||
| if (error != null) { | ||
| switch (typeof error) { | ||
| case "object": { | ||
| if (Object.isExtensible(error)) { | ||
| (error as StateError<TError>)._suppressLogging = | ||
| context.suppressLogging; | ||
| return error as StateError<TError>; | ||
| } else { | ||
| return Object.assign( | ||
| Object.create(error as object), | ||
| suppressLogging | ||
| ); | ||
| } | ||
| } | ||
| default: { | ||
| return Object.assign(Error(error as string), suppressLogging); | ||
| } | ||
| } | ||
| } else { | ||
| return Object.assign(Error(undefined), suppressLogging); | ||
| } | ||
| }; | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This got lost in a previous round of discussion, but the Let's say we had a custom error type (e.g. This might be an unlikely case but I think it's probably still not good for us to have a misleading generic type. That being said, I don't think this generic type is actually really doing anything that useful and we can just remove it entirely. As a small efficiency nit too, I think we could lift the So... diff --git a/src/useErrorBoundary.ts b/src/useErrorBoundary.ts
index d215e58..30bec77 100644
--- a/src/useErrorBoundary.ts
+++ b/src/useErrorBoundary.ts
@@ -1,55 +1,25 @@
import { useContext, useMemo, useState } from "react";
import { assertErrorBoundaryContext } from "./assertErrorBoundaryContext";
-import { ErrorBoundaryContext } from "./ErrorBoundaryContext";
+import {
+ ErrorBoundaryContext,
+ ErrorBoundaryContextType,
+} from "./ErrorBoundaryContext";
-type StateError<TError> =
- | (TError & { _suppressLogging: boolean })
- | (Error & { _suppressLogging: boolean });
-
-type UseErrorBoundaryState<TError> =
- | { error: StateError<TError>; hasError: true }
+type UseErrorBoundaryState =
+ | { error: unknown; hasError: true }
| { error: null; hasError: false };
-export type UseErrorBoundaryApi<TError> = {
+export type UseErrorBoundaryApi = {
resetBoundary: () => void;
- showBoundary: (error: TError) => void;
+ showBoundary: (error: unknown) => void;
};
-export function useErrorBoundary<TError = any>(): UseErrorBoundaryApi<TError> {
+export function useErrorBoundary(): UseErrorBoundaryApi {
const context = useContext(ErrorBoundaryContext);
assertErrorBoundaryContext(context);
- const suppressLoggingForError = function <TError>(
- error: TError
- ): StateError<TError> {
- const suppressLogging = {
- _suppressLogging: context.suppressLogging as boolean,
- };
- if (error != null) {
- switch (typeof error) {
- case "object": {
- if (Object.isExtensible(error)) {
- (error as StateError<TError>)._suppressLogging =
- context.suppressLogging;
- return error as StateError<TError>;
- } else {
- return Object.assign(
- Object.create(error as object),
- suppressLogging
- );
- }
- }
- default: {
- return Object.assign(Error(error as string), suppressLogging);
- }
- }
- } else {
- return Object.assign(Error(undefined), suppressLogging);
- }
- };
-
- const [state, setState] = useState<UseErrorBoundaryState<TError>>({
+ const [state, setState] = useState<UseErrorBoundaryState>({
error: null,
hasError: false,
});
@@ -60,12 +30,12 @@ export function useErrorBoundary<TError = any>(): UseErrorBoundaryApi<TError> {
context.resetErrorBoundary();
setState({ error: null, hasError: false });
},
- showBoundary: (error: TError) => {
+ showBoundary: (error: unknown) => {
if (context.suppressLogging) {
window.addEventListener("error", context.handleSuppressLogging);
}
setState({
- error: suppressLoggingForError(error),
+ error: suppressLoggingForError(error, context),
hasError: true,
});
},
@@ -79,3 +49,30 @@ export function useErrorBoundary<TError = any>(): UseErrorBoundaryApi<TError> {
return memoized;
}
+
+function suppressLoggingForError(
+ error: unknown,
+ context: ErrorBoundaryContextType
+): unknown {
+ const suppressLogging = {
+ _suppressLogging: context.suppressLogging as boolean,
+ };
+
+ if (error != null) {
+ switch (typeof error) {
+ case "object": {
+ if (Object.isExtensible(error)) {
+ (error as any)._suppressLogging = context.suppressLogging;
+ return error;
+ } else {
+ return Object.assign(Object.create(error as object), suppressLogging);
+ }
+ }
+ default: {
+ return Object.assign(Error(error as string), suppressLogging);
+ }
+ }
+ } else {
+ return Object.assign(Error(undefined), suppressLogging);
+ }
+} |
||
|
|
||
| const [state, setState] = useState<UseErrorBoundaryState<TError>>({ | ||
| error: null, | ||
| hasError: false, | ||
|
|
@@ -27,11 +60,15 @@ export function useErrorBoundary<TError = any>(): UseErrorBoundaryApi<TError> { | |
| context.resetErrorBoundary(); | ||
| setState({ error: null, hasError: false }); | ||
| }, | ||
| showBoundary: (error: TError) => | ||
| showBoundary: (error: TError) => { | ||
| if (context.suppressLogging) { | ||
| window.addEventListener("error", context.handleSuppressLogging); | ||
| } | ||
| setState({ | ||
| error, | ||
| error: suppressLoggingForError(error), | ||
| hasError: true, | ||
| }), | ||
| }); | ||
| }, | ||
| }), | ||
| [context.resetErrorBoundary] | ||
| ); | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.