From 8fc5763b8acb3fe8ca8c350cff7675a5bce2b332 Mon Sep 17 00:00:00 2001 From: Ruslan Lesiutin <28902667+hoxyq@users.noreply.github.com> Date: Wed, 13 May 2026 22:05:14 +0100 Subject: [PATCH] fix[describeClassComponentFrame]: invoke constructor with new keyword (#36455) For JavaScript runtimes that do not have [`Reflect`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Reflect) supported, we had a fall back that was calling the constructor with overridden `this` context via ``` fn.apply(Fake.prototype); ``` In ES6, it is required to call constructor only with the `new` keyword, otherwise the runtime is expected to throw a corresponding TypeError: ``` TypeError: Class constructor <> cannot be invoked without 'new' ``` We've observed this error in Hermes runtime, but this is applicable to V8 or any other runtime. The only reason why V8 wasn't affected is because it implemented Reflect APIs. Instead of the incorrect call, we will fall back to calling `new fn()`, but with a temporary patched prototype of the class, which would make a trap out of the setter for `props` object. We use the same approach when `Reflect` APIs are available, but instead of modifying the prototype, we pass the fake context: https://github.com/facebook/react/blob/d5736f098edee62c44f27b053e6e48f5fa443803/packages/shared/ReactComponentStackFrame.js#L129-L148 --- See tests implemented. Without the changes, the test would fail with the `TypeError` mentioned above. --- .../shared/DevToolsComponentStackFrame.js | 29 ++++++++++++++- .../src/__tests__/ReactErrorStacks-test.js | 37 +++++++++++++++++++ packages/shared/ReactComponentStackFrame.js | 29 ++++++++++++++- scripts/jest/setupTests.js | 26 +++++++------ 4 files changed, 105 insertions(+), 16 deletions(-) diff --git a/packages/react-devtools-shared/src/backend/shared/DevToolsComponentStackFrame.js b/packages/react-devtools-shared/src/backend/shared/DevToolsComponentStackFrame.js index b29ed9af7306..33df375a971e 100644 --- a/packages/react-devtools-shared/src/backend/shared/DevToolsComponentStackFrame.js +++ b/packages/react-devtools-shared/src/backend/shared/DevToolsComponentStackFrame.js @@ -129,8 +129,33 @@ export function describeNativeComponentFrame( } catch (x) { control = x; } - // $FlowFixMe[prop-missing] found when upgrading Flow - fn.call(Fake.prototype); + + let prototypeModified = false; + let prevProps; + try { + prevProps = Object.getOwnPropertyDescriptor( + fn.prototype, + 'props', + ); + Object.defineProperty(fn.prototype, 'props', { + configurable: true, + set() { + throw Error(); + }, + }); + prototypeModified = true; + + // eslint-disable-next-line no-new + new fn(); + } finally { + if (prototypeModified) { + if (prevProps !== undefined) { + Object.defineProperty(fn.prototype, 'props', prevProps); + } else { + delete fn.prototype.props; + } + } + } } } else { try { diff --git a/packages/react-reconciler/src/__tests__/ReactErrorStacks-test.js b/packages/react-reconciler/src/__tests__/ReactErrorStacks-test.js index c761501839a0..59a662beefd9 100644 --- a/packages/react-reconciler/src/__tests__/ReactErrorStacks-test.js +++ b/packages/react-reconciler/src/__tests__/ReactErrorStacks-test.js @@ -342,4 +342,41 @@ describe('ReactFragment', () => { __DEV__ ? componentStack(['SomethingThatErrors']) : null, ]); }); + + it('captures class component stacks via the no-Reflect.construct fallback', async () => { + class FailingClassComponent extends React.Component { + render() { + throw new Error('uh oh'); + } + } + + const root = ReactNoop.createRoot({onCaughtError}); + + const originalReflectConstruct = Reflect.construct; + delete Reflect.construct; + + try { + root.render( + + + , + ); + await waitForAll([]); + } finally { + Object.defineProperty(Reflect, 'construct', { + value: originalReflectConstruct, + }); + } + + expect(rootCaughtErrors).toEqual([ + 'uh oh', + componentStack(['FailingClassComponent', 'CatchingBoundary']), + __DEV__ ? componentStack(['FailingClassComponent']) : null, + ]); + + // The throwing trap should be reset, we use props object for it + expect( + Object.getOwnPropertyDescriptor(FailingClassComponent.prototype, 'props'), + ).toBeUndefined(); + }); }); diff --git a/packages/shared/ReactComponentStackFrame.js b/packages/shared/ReactComponentStackFrame.js index 1951ab07c7d1..969b65095935 100644 --- a/packages/shared/ReactComponentStackFrame.js +++ b/packages/shared/ReactComponentStackFrame.js @@ -152,8 +152,33 @@ export function describeNativeComponentFrame( } catch (x) { control = x; } - // $FlowFixMe[prop-missing] found when upgrading Flow - fn.call(Fake.prototype); + + let prototypeModified = false; + let prevProps; + try { + prevProps = Object.getOwnPropertyDescriptor( + fn.prototype, + 'props', + ); + Object.defineProperty(fn.prototype, 'props', { + configurable: true, + set() { + throw Error(); + }, + }); + prototypeModified = true; + + // eslint-disable-next-line no-new + new fn(); + } finally { + if (prototypeModified) { + if (prevProps !== undefined) { + Object.defineProperty(fn.prototype, 'props', prevProps); + } else { + delete fn.prototype.props; + } + } + } } } else { try { diff --git a/scripts/jest/setupTests.js b/scripts/jest/setupTests.js index aadb28017829..ea4f60746f4d 100644 --- a/scripts/jest/setupTests.js +++ b/scripts/jest/setupTests.js @@ -115,6 +115,13 @@ if (process.env.REACT_CLASS_EQUIVALENCE_TEST) { return format.replace(/%s/g, () => args[argIndex++]); }; const OriginalError = global.Error; + // Cache Reflect methods so the proxies keep working even if a test + // temporarily deletes or overrides them (e.g. to exercise no-Reflect + // fallback paths in the React source). + const ReflectApply = Reflect.apply; + const ReflectConstruct = Reflect.construct; + const ReflectGet = Reflect.get; + const ReflectSet = Reflect.set; // V8's Error.captureStackTrace (used in Jest) fails if the error object is // a Proxy, so we need to pass it the unproxied instance. const originalErrorInstances = new WeakMap(); @@ -131,21 +138,16 @@ if (process.env.REACT_CLASS_EQUIVALENCE_TEST) { const proxy = new Proxy(error, { set(target, key, value, receiver) { if (key === 'message') { - return Reflect.set( - target, - key, - decodeErrorMessage(value), - receiver - ); + return ReflectSet(target, key, decodeErrorMessage(value), receiver); } - return Reflect.set(target, key, value, receiver); + return ReflectSet(target, key, value, receiver); }, get(target, key, receiver) { if (key === 'stack') { // https://github.com/nodejs/node/issues/60862 - return Reflect.get(target, key); + return ReflectGet(target, key); } - return Reflect.get(target, key, receiver); + return ReflectGet(target, key, receiver); }, }); originalErrorInstances.set(proxy, error); @@ -153,12 +155,12 @@ if (process.env.REACT_CLASS_EQUIVALENCE_TEST) { }; const ErrorProxy = new Proxy(OriginalError, { apply(target, thisArg, argumentsList) { - const error = Reflect.apply(target, thisArg, argumentsList); + const error = ReflectApply(target, thisArg, argumentsList); error.message = decodeErrorMessage(error.message); return proxyErrorInstance(error); }, construct(target, argumentsList, newTarget) { - const error = Reflect.construct(target, argumentsList, newTarget); + const error = ReflectConstruct(target, argumentsList, newTarget); error.message = decodeErrorMessage(error.message); return proxyErrorInstance(error); }, @@ -166,7 +168,7 @@ if (process.env.REACT_CLASS_EQUIVALENCE_TEST) { if (key === 'captureStackTrace') { return captureStackTrace; } - return Reflect.get(target, key, receiver); + return ReflectGet(target, key, receiver); }, }); ErrorProxy.OriginalError = OriginalError;