Facebook/react-native
1
0
Fork 0
mirror of https://github.com/facebook/react-native.git synced 2025-11-01 09:14:26 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix parseLogBoxLog test that test hermes component stacks (#43281)

Browse Source 
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/43281

## Overview
This diff fixes a bug in the hermes component stack location parser, and fixes the hermes component stack tests which were not using hermes stack parsing, which is why the bug wasn't caught.

The bug fix is that React component stacks may not all have stack frame locations. For example, this stack:

```
at MyComponent (/path/to/filename.js:1:2)
at MyOtherComponent                         <-- no location
at MyAppComponent (/path/to/app.js:100:20)
```

This can happen when we're unable to make a component throw (e.g. it doesn't use a hook or access props). We have plans to fix these frames, but currently they can exist.

The bug was when `parseHermesStack` finds a frame without an `entry`, it would reset the `entries`. But if entries is already non-null, or if the current frame is a frame without a source, we should continue.

### Caveats

The handling here fixes the behavior to go back to skipping these frames. I'm not sure what the best way to handle these cases are, since these frames do not have source location and should skip symbolication. We should follow up with handling for these frames.

## Why it wasn't caught

In D18627930 we changed the hermes component stack parsing to check `global.HermesInternal`, but the tests for the hermes component stacks were still using the `stacktrace-parser`. I updated the tests to set/reset the global, which caught the bug.

Changelog:
[General][Fixed] - Support hermes component stacks with missing source info.

Reviewed By: yungsters

Differential Revision: D54423252

fbshipit-source-id: 80ded8b99eab919e60f847369dcb1f3afa72b6be
This commit is contained in:
Rick Hanlon
2024-03-06 07:44:01 -08:00
committed by Facebook GitHub Bot
parent b2fba371df
commit 82db330360
2 changed files with 23 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/react-native/Libraries/Core/Devtools/parseHermesStack.js
Vendored
+6
View File
@@ -72,6 +72,7 @@ const RE_FRAME =
// Capturing groups:
// 1. count of skipped frames
const RE_SKIPPED = /^ {4}... skipping (\d+) frames$/;
const RE_COMPONENT_NO_STACK = /^ {4}at .*$/;
function isInternalBytecodeSourceUrl(sourceUrl: string): boolean {
// See https://github.com/facebook/hermes/blob/3332fa020cae0bab751f648db7c94e1d687eeec7/lib/VM/Runtime.cpp#L1100
@@ -132,6 +133,11 @@ module.exports = function parseHermesStack(stack: string): HermesParsedStack {
entries.push(entry);
continue;
}
if (RE_COMPONENT_NO_STACK.test(line)) {
// Skip component stacks without source location.
// TODO: This will not be displayed, not sure how to handle it.
continue;
}
// No match - we're still in the message
lastMessageLine = i;
entries = [];
packages/react-native/Libraries/LogBox/Data/__tests__/parseLogBoxLog-test.js
Vendored
+17 -6
View File
@@ -993,14 +993,25 @@ Please follow the instructions at: fburl.com/rn-remote-assets`,
});
});
describe('Handles component stack frames formatted as call stacks', () => {
describe('Handles component stack frames formatted as call stacks in Hermes', () => {
let originalHermesInternal;
beforeEach(() => {
originalHermesInternal = global.HermesInternal;
// $FlowFixMe[cannot-write] - Jest
global.HermesInternal = true;
});
afterEach(() => {
// $FlowFixMe[cannot-write] - Jest
global.HermesInternal = originalHermesInternal;
});
// In new versions of React, the component stack frame format changed to match call stacks.
it('detects a component stack in an interpolated warning', () => {
expect(
parseLogBoxLog([
'Warning: Function components cannot be given refs. Attempts to access this ref will fail. Did you mean to use React.forwardRef()?%s%s',
'\n\nCheck the render method of `MyComponent`.',
'\n at MyComponent (/path/to/filename.js:1:2)\n at MyOtherComponent\n at MyAppComponent (/path/to/app.js:100:20)',
'\n at MyComponent (/path/to/filename.js:1:2)\n at MyOtherComponent\n at MyAppComponent (/path/to/app.js:100:20)',
]),
).toEqual({
componentStack: [
@@ -1037,7 +1048,7 @@ Please follow the instructions at: fburl.com/rn-remote-assets`,
it('detects a component stack in the first argument', () => {
expect(
parseLogBoxLog([
'Some kind of message\n at MyComponent (/path/to/filename.js:1:2)\n at MyOtherComponent\n at MyAppComponent (/path/to/app.js:100:20)',
'Some kind of message\n at MyComponent (/path/to/filename.js:1:2)\n at MyOtherComponent\n at MyAppComponent (/path/to/app.js:100:20)',
]),
).toEqual({
componentStack: [
@@ -1068,7 +1079,7 @@ Please follow the instructions at: fburl.com/rn-remote-assets`,
expect(
parseLogBoxLog([
'Some kind of message',
'\n at MyComponent (/path/to/filename.js:1:2)\n at MyOtherComponent\n at MyAppComponent (/path/to/app.js:100:20)',
'\n at MyComponent (/path/to/filename.js:1:2)\n at MyOtherComponent\n at MyAppComponent (/path/to/app.js:100:20)',
]),
).toEqual({
componentStack: [
@@ -1101,7 +1112,7 @@ Please follow the instructions at: fburl.com/rn-remote-assets`,
'Warning: Each child in a list should have a unique "key" prop.%s%s See https://fb.me/react-warning-keys for more information.%s',
'\n\nCheck the render method of `MyOtherComponent`.',
'',
'\n at MyComponent (/path/to/filename.js:1:2)\n at MyOtherComponent\n at MyAppComponent (/path/to/app.js:100:20)',
'\n at MyComponent (/path/to/filename.js:1:2)\n at MyOtherComponent\n at MyAppComponent (/path/to/app.js:100:20)',
]),
).toEqual({
componentStack: [
@@ -1148,7 +1159,7 @@ Please follow the instructions at: fburl.com/rn-remote-assets`,
originalMessage: '### Error',
name: '',
componentStack:
'\n at MyComponent (/path/to/filename.js:1:2)\n at MyOtherComponent\n at MyAppComponent (/path/to/app.js:100:20)',
'\n at MyComponent (/path/to/filename.js:1:2)\n at MyOtherComponent\n at MyAppComponent (/path/to/app.js:100:20)',
stack: [
{
column: 1,