Skip to content

Commit

Permalink
Update stack diffing algorithm in describeNativeComponentFrame
Browse files Browse the repository at this point in the history 
- Attempt have a predefined "root" or common frame between sample and
  control frames
  • Loading branch information
@KarimP
KarimP committed Jul 19, 2023
1 parent d445cee commit 9cc71f5
Showing 1 changed file with 134 additions and 64 deletions.
198 changes: 134 additions & 64 deletions packages/shared/ReactComponentStackFrame.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,17 @@ if (__DEV__) {
componentFrameCache = new PossiblyWeakMap<Function, string>();
}

/**
* Leverages native browser/VM stack frames to get proper details (e.g.
* filename, line + col number) for a single component in a component stack. We
* do this by:
* (1) throwing and catching an error in the function - this will be our
* control error.
* (2) calling the component which will eventually throw an error that we'll
* catch - this will be our sample error.
* (3) diffing the control and sample error stacks to find the stack frame
* which represents our component.
*/
export function describeNativeComponentFrame(
fn: Function,
construct: boolean,
Expand All @@ -76,89 +87,148 @@ export function describeNativeComponentFrame(
}
}

let control;

reentry = true;
const previousPrepareStackTrace = Error.prepareStackTrace;
// $FlowFixMe[incompatible-type] It does accept undefined.
Error.prepareStackTrace = undefined;
let previousDispatcher;

if (__DEV__) {
previousDispatcher = ReactCurrentDispatcher.current;
// Set the dispatcher in DEV because this might be call in the render function
// for warnings.
ReactCurrentDispatcher.current = null;
disableLogs();
}
try {
// This should throw.
if (construct) {
// Something should be setting the props in the constructor.
const Fake = function () {
throw Error();
};
// $FlowFixMe[prop-missing]
Object.defineProperty(Fake.prototype, 'props', {
set: function () {
// We use a throwing setter instead of frozen or non-writable props
// because that won't throw in a non-strict mode function.
throw Error();
},
});
if (typeof Reflect === 'object' && Reflect.construct) {
// We construct a different control for this case to include any extra
// frames added by the construct call.
try {
Reflect.construct(Fake, []);
} catch (x) {
control = x;
}
Reflect.construct(fn, [], Fake);
} else {
try {
Fake.call();
} catch (x) {
control = x;
}
// $FlowFixMe[prop-missing] found when upgrading Flow
fn.call(Fake.prototype);

/**
* Finding a common stack frame between sample and control errors can be
* tricky given the different types and levels of stack trace truncation from
* different JS VMs. So instead we'll attempt to control what that common
* frame should be through this class:
* Having both the sample and control errors be under the
* `DescribeNativeComponentFrameRoot` method call will ensure that a stack
* frame exists that has the method name `DescribeNativeComponentFrameRoot` in
* it for both control and sample stacks.
*
* Note that we're using a class method here instead of a plain object
* property to prevent Closure compiler from eliding away the object and the
* extra method call.
*/
class RunInRootFrame {
constructor() {
// Bun and Safari will require setting these properties should the class
// ever get transpiled into an ES2015 constructor function.
// $FlowFixMe[method-unbinding]
this.DetermineComponentFrameRoot.displayName =
'DetermineComponentFrameRoot';

// Before ES6, the `name` property was not configurable.
if (
// $FlowFixMe[method-unbinding]
Object.getOwnPropertyDescriptor(this.DetermineComponentFrameRoot)
?.configurable
) {
// Configurable properties can be updated even if its writable
// descriptor is set to `false`. V8 utilizes a function's `name`
// property when generating a stack trace.
// $FlowFixMe[method-unbinding]
Object.defineProperty(this.DetermineComponentFrameRoot, 'name', {
value: 'DetermineComponentFrameRoot',
});
}
} else {
}

DetermineComponentFrameRoot(): [?string, ?string] {
let control;
try {
throw Error();
} catch (x) {
control = x;
}
// TODO(luna): This will currently only throw if the function component
// tries to access React/ReactDOM/props. We should probably make this throw
// in simple components too
const maybePromise = fn();
// This should throw.
if (construct) {
// Something should be setting the props in the constructor.
const Fake = function () {
throw Error();
};
// $FlowFixMe[prop-missing]
Object.defineProperty(Fake.prototype, 'props', {
set: function () {
// We use a throwing setter instead of frozen or non-writable props
// because that won't throw in a non-strict mode function.
throw Error();
},
});
if (typeof Reflect === 'object' && Reflect.construct) {
// We construct a different control for this case to include any extra
// frames added by the construct call.
try {
Reflect.construct(Fake, []);
} catch (x) {
control = x;
}
Reflect.construct(fn, [], Fake);
} else {
try {
Fake.call();
} catch (x) {
control = x;
}
// $FlowFixMe[prop-missing] found when upgrading Flow
fn.call(Fake.prototype);
}
} else {
try {
throw Error();
} catch (x) {
control = x;
}
// TODO(luna): This will currently only throw if the function component
// tries to access React/ReactDOM/props. We should probably make this throw
// in simple components too
const maybePromise = fn();

// If the function component returns a promise, it's likely an async
// component, which we don't yet support. Attach a noop catch handler to
// silence the error.
// TODO: Implement component stacks for async client components?
if (maybePromise && typeof maybePromise.catch === 'function') {
maybePromise.catch(() => {});
// If the function component returns a promise, it's likely an async
// component, which we don't yet support. Attach a noop catch handler to
// silence the error.
// TODO: Implement component stacks for async client components?
if (maybePromise && typeof maybePromise.catch === 'function') {
maybePromise.catch(() => {});
}
}
} catch (sample) {
// This is inlined manually because closure doesn't do it for us.
if (sample && control && typeof sample.stack === 'string') {
return [sample.stack, control.stack];
}
}
return [null, null];
}
} catch (sample) {
// This is inlined manually because closure doesn't do it for us.
if (sample && control && typeof sample.stack === 'string') {
}

try {
const [sampleStack, controlStack] =
new RunInRootFrame().DetermineComponentFrameRoot();
if (sampleStack && controlStack) {
// This extracts the first frame from the sample that isn't also in the control.
// Skipping one frame that we assume is the frame that calls the two.
const sampleLines = sample.stack.split('\n');
const controlLines = control.stack.split('\n');
let s = sampleLines.length - 1;
let c = controlLines.length - 1;
while (s >= 1 && c >= 0 && sampleLines[s] !== controlLines[c]) {
// We expect at least one stack frame to be shared.
// Typically this will be the root most one. However, stack frames may be
// cut off due to maximum stack limits. In this case, one maybe cut off
// earlier than the other. We assume that the sample is longer or the same
// and there for cut off earlier. So we should find the root most frame in
// the sample somewhere in the control.
c--;
const sampleLines = sampleStack.split('\n');
const controlLines = controlStack.split('\n');
let s = sampleLines.findIndex(line =>
line.includes('DetermineComponentFrameRoot'),
);
let c = controlLines.findIndex(line =>
line.includes('DetermineComponentFrameRoot'),
);
if (s === -1 || c === -1) {
s = sampleLines.length - 1;
c = controlLines.length - 1;
while (s >= 1 && c >= 0 && sampleLines[s] !== controlLines[c]) {
// We expect at least one stack frame to be shared.
// Typically this will be the root most one. However, stack frames may be
// cut off due to maximum stack limits. In this case, one maybe cut off
// earlier than the other. We assume that the sample is longer or the same
// and there for cut off earlier. So we should find the root most frame in
// the sample somewhere in the control.
c--;
}
}
for (; s >= 1 && c >= 0; s--, c--) {
// Next we find the first one that isn't the same which should be the
Expand Down

0 comments on commit 9cc71f5

Please sign in to comment.