Skip to content
28 changes: 27 additions & 1 deletion packages/react-debug-tools/src/ReactDebugHooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,7 @@ const DispatcherProxyHandler = {
const error = new Error('Missing method in Dispatcher: ' + prop);
// Note: This error name needs to stay in sync with react-devtools-shared
// TODO: refactor this if we ever combine the devtools and debug tools packages
error.name = 'UnsupportedFeatureError';
error.name = 'ReactDebugToolsUnsupportedHookError';
throw error;
},
};
Expand Down Expand Up @@ -667,6 +667,28 @@ function processDebugValues(
}
}

function handleRenderFunctionError(error: any): void {
// original error might be any type.
if (
error instanceof Error &&
error.name === 'ReactDebugToolsUnsupportedHookError'
) {
throw error;
}
// If the error is not caused by an unsupported feature, it means
// that the error is caused by user's code in renderFunction.
// In this case, we should wrap the original error inside a custom error
// so that devtools can give a clear message about it.
// $FlowFixMe: Flow doesn't know about 2nd argument of Error constructor
const wrapperError = new Error('Error rendering inspected component', {
cause: error,
});
// Note: This error name needs to stay in sync with react-devtools-shared
// TODO: refactor this if we ever combine the devtools and debug tools packages
wrapperError.name = 'ReactDebugToolsRenderFunctionError';
throw wrapperError;
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Stylistic choice but this feels a bit cleaner to me?

let newError = error;
if (!(error instanceof Error) || error.name !== 'ReactDebugToolsUnsupportedHookError') {
  // If the error is not caused by an unsupported feature, it means
  // that the error is caused by user's code in renderFunction.
  // In this case, we should wrap the original error inside a custom error
  // so that devtools can give a clear message about it.
  // $FlowFixMe: Flow doesn't know about 2nd argument of Error constructor
  let newError = new Error('Error rendering inspected component', {
    cause: error,
  });
  // Note: This error name needs to stay in sync with react-devtools-shared
  // TODO: refactor this if we ever combine the devtools and debug tools packages
  newError.name = 'ReactDebugToolsRenderFunctionError';
}

throw newError;

}

export function inspectHooks<Props>(
renderFunction: Props => React$Node,
props: Props,
Expand All @@ -686,6 +708,8 @@ export function inspectHooks<Props>(
try {
ancestorStackError = new Error();
renderFunction(props);
} catch (error) {
handleRenderFunctionError(error);
} finally {
readHookLog = hookLog;
hookLog = [];
Expand Down Expand Up @@ -730,6 +754,8 @@ function inspectHooksOfForwardRef<Props, Ref>(
try {
ancestorStackError = new Error();
renderFunction(props, ref);
} catch (error) {
handleRenderFunctionError(error);
} finally {
readHookLog = hookLog;
hookLog = [];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,10 @@ describe('ReactHooksInspection', () => {
return <div>{state}</div>;
}

const OriginalError = global.Error;
const MockError = jest.fn();
global.Error = MockError;

const initial = {};
let current = initial;
let getterCalls = 0;
Expand All @@ -279,7 +283,17 @@ describe('ReactHooksInspection', () => {
expect(() => {
expect(() => {
ReactDebugTools.inspectHooks(Foo, {}, FakeDispatcherRef);
}).toThrow("Cannot read property 'useState' of null");
}).toThrow(MockError);
expect(MockError.mock.calls.length).toBe(1);
// first argument is the error message
expect(MockError.mock.calls[0][0]).toBe(
'Error rendering inspected component',
);
// The second arg of the first call to the function was 'second arg'
expect(MockError.mock.calls[0][1]).toBeInstanceOf(OriginalError);
expect(MockError.mock.calls[0][1].message).toBe(
"Cannot read property 'useState' of null",
);
}).toErrorDev(
'Invalid hook call. Hooks can only be called inside of the body of a function component. This could happen for' +
' one of the following reasons:\n' +
Expand All @@ -294,6 +308,8 @@ describe('ReactHooksInspection', () => {
expect(setterCalls).toHaveLength(2);
expect(setterCalls[0]).not.toBe(initial);
expect(setterCalls[1]).toBe(initial);

global.Error = OriginalError;
});

describe('useDebugValue', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2145,7 +2145,7 @@ describe('InspectedElement', () => {
expect(value).toBe(null);

const error = errorBoundaryInstance.state.error;
expect(error.message).toBe('Expected');
expect(error.message).toBe('Error rendering inspected component');
expect(error.stack).toContain('inspectHooksOfFiber');
});

Expand Down