Skip to content

Commit

Permalink
RN DevX] LogBox - Handle tapping component stacks if possible
Browse files Browse the repository at this point in the history 
Summary:
This diff adds handling for tapping to open components in component stacks, whenever they're available.

Changelog: [Internal]

Reviewed By: motiz88

Differential Revision: D18882338

fbshipit-source-id: 574bff1c41c952c33e647cb142d278ace60b4631
  • Loading branch information
@rickhanlonii @facebook-github-bot
rickhanlonii authored and facebook-github-bot committed Dec 10, 2019
1 parent 1051b59 commit bbfd64e
Show file tree
Hide file tree
Showing 6 changed files with 886 additions and 62 deletions.
14 changes: 12 additions & 2 deletions Libraries/LogBox/Data/__tests__/LogBoxLog-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,13 @@ function getLogBoxLog() {
message: {content: '...', substitutions: []},
stack: createStack(['A', 'B', 'C']),
category: 'Message category...',
componentStack: [{component: 'LogBoxLog', location: 'LogBoxLog.js:1'}],
componentStack: [
{
content: 'LogBoxLog',
fileName: 'LogBoxLog.js',
location: {column: -1, row: 1},
},
],
codeFrame: {
fileName: '/path/to/RKJSModules/Apps/CrashReact/CrashReactApp.js',
location: {row: 199, column: 0},
Expand Down Expand Up @@ -69,7 +75,11 @@ describe('LogBoxLog', () => {
expect(log.stack).toEqual(createStack(['A', 'B', 'C']));
expect(log.category).toEqual('Message category...');
expect(log.componentStack).toEqual([
{component: 'LogBoxLog', location: 'LogBoxLog.js:1'},
{
content: 'LogBoxLog',
fileName: 'LogBoxLog.js',
location: {column: -1, row: 1},
},
]);
expect(log.codeFrame).toEqual({
fileName: '/path/to/RKJSModules/Apps/CrashReact/CrashReactApp.js',
Expand Down
46 changes: 36 additions & 10 deletions Libraries/LogBox/Data/__tests__/parseLogBoxLog-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,16 @@ describe('parseLogBoxLog', () => {
]),
).toEqual({
componentStack: [
{component: 'MyComponent', location: 'filename.js:1'},
{component: 'MyOtherComponent', location: 'filename2.js:1'},
{
content: 'MyComponent',
fileName: 'filename.js',
location: {column: -1, row: 1},
},
{
content: 'MyOtherComponent',
fileName: 'filename2.js',
location: {column: -1, row: 1},
},
],
category:
'Warning: Function components cannot be given refs. Attempts to access this ref will fail. Did you mean to use React.forwardRef()?%s',
Expand All @@ -143,8 +151,16 @@ describe('parseLogBoxLog', () => {
]),
).toEqual({
componentStack: [
{component: 'MyComponent', location: 'filename.js:1'},
{component: 'MyOtherComponent', location: 'filename2.js:1'},
{
content: 'MyComponent',
fileName: 'filename.js',
location: {column: -1, row: 1},
},
{
content: 'MyOtherComponent',
fileName: 'filename2.js',
location: {column: -1, row: 1},
},
],
category: 'Some kind of message',
message: {
Expand All @@ -164,8 +180,16 @@ describe('parseLogBoxLog', () => {
]),
).toEqual({
componentStack: [
{component: 'MyComponent', location: 'filename.js:1'},
{component: 'MyOtherComponent', location: 'filename2.js:1'},
{
content: 'MyComponent',
fileName: 'filename.js',
location: {column: -1, row: 1},
},
{
content: 'MyOtherComponent',
fileName: 'filename2.js',
location: {column: -1, row: 1},
},
],
category:
'Some kind of message Some other kind of message Some third kind of message',
Expand Down Expand Up @@ -418,12 +442,14 @@ Please follow the instructions at: fburl.com/rn-remote-assets`,
},
componentStack: [
{
component: 'MyComponent',
location: 'filename.js:1',
content: 'MyComponent',
fileName: 'filename.js',
location: {column: -1, row: 1},
},
{
component: 'MyOtherComponent',
location: 'filename2.js:1',
content: 'MyOtherComponent',
fileName: 'filename2.js',
location: {column: -1, row: 1},
},
],
stack: [
Expand Down
22 changes: 12 additions & 10 deletions Libraries/LogBox/Data/parseLogBoxLog.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,7 @@ export type Message = $ReadOnly<{|
>,
|}>;

export type ComponentStack = $ReadOnlyArray<
$ReadOnly<{|
component: string,
location: string,
|}>,
>;
export type ComponentStack = $ReadOnlyArray<CodeFrame>;

const SUBSTITUTION = UTFSequence.BOM + '%s';

Expand Down Expand Up @@ -127,18 +122,25 @@ export function parseCategory(
},
};
}

export function parseComponentStack(message: string): ComponentStack {
return message
.split(/\n {4}in /g)
.map(s => {
if (!s) {
return null;
}
let [component, location] = s.split(/ \(at /);
if (!location) {
[component, location] = s.split(/ \(/);
const match = s.match(/(.*) \(at (.*\.js):([\d]+)\)/);
if (!match) {
return null;
}
return {component, location: location && location.replace(')', '')};

let [content, fileName, row] = match.slice(1);
return {
content,
fileName,
location: {column: -1, row: parseInt(row, 10)},
};
})
.filter(Boolean);
}
Expand Down
72 changes: 63 additions & 9 deletions Libraries/LogBox/UI/LogBoxInspectorReactFrames.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,37 @@ import View from '../../Components/View/View';
import LogBoxButton from './LogBoxButton';
import * as LogBoxStyle from './LogBoxStyle';
import LogBoxInspectorSection from './LogBoxInspectorSection';
import openFileInEditor from '../../Core/Devtools/openFileInEditor';
import type LogBoxLog from '../Data/LogBoxLog';

type Props = $ReadOnly<{|
log: LogBoxLog,
|}>;

const BEFORE_SLASH_RE = /^(.*)[\\/]/;

// Taken from React https://github.com/facebook/react/blob/206d61f72214e8ae5b935f0bf8628491cb7f0797/packages/react-devtools-shared/src/backend/describeComponentFrame.js#L27-L41
function getPrettyFileName(path) {
let fileName = path.replace(BEFORE_SLASH_RE, '');

// In DEV, include code for a common special case:
// prefer "folder/index.js" instead of just "index.js".
if (/^index\./.test(fileName)) {
const match = path.match(BEFORE_SLASH_RE);
if (match) {
const pathBeforeSlash = match[1];
if (pathBeforeSlash) {
const folderName = pathBeforeSlash.replace(BEFORE_SLASH_RE, '');
// Note the below string contains a zero width space after the "/" character.
// This is to prevent browsers like Chrome from formatting the file name as a link.
// (Since this is a source link, it would not work to open the source file anyway.)
fileName = folderName + '/​' + fileName;
}
}
}

return fileName;
}
function LogBoxInspectorReactFrames(props: Props): React.Node {
const [collapsed, setCollapsed] = React.useState(true);
if (props.log.componentStack == null || props.log.componentStack.length < 1) {
Expand All @@ -39,6 +64,10 @@ function LogBoxInspectorReactFrames(props: Props): React.Node {
}

function getCollapseMessage() {
if (props.log.componentStack.length <= 3) {
return;
}

const count = props.log.componentStack.length - 3;
if (collapsed) {
return `See ${count} more components`;
Expand All @@ -53,15 +82,34 @@ function LogBoxInspectorReactFrames(props: Props): React.Node {
<View
// Unfortunately we don't have a unique identifier for stack traces.
key={index}
style={componentStyles.frame}>
<View style={componentStyles.component}>
<Text style={componentStyles.frameName}>
<Text style={componentStyles.bracket}>{'<'}</Text>
{frame.component}
<Text style={componentStyles.bracket}>{' />'}</Text>
style={componentStyles.frameContainer}>
<LogBoxButton
backgroundColor={{
default: 'transparent',
pressed: LogBoxStyle.getBackgroundColor(1),
}}
onPress={
// Older versions of DevTools do not provide full path.
// This will not work on Windows, remove check once the
// DevTools return the full file path.
frame.fileName.startsWith('/')
? () =>
openFileInEditor(frame.fileName, frame.location?.row ?? 1)
: null
}
style={componentStyles.frame}>
<View style={componentStyles.component}>
<Text style={componentStyles.frameName}>
<Text style={componentStyles.bracket}>{'<'}</Text>
{frame.content}
<Text style={componentStyles.bracket}>{' />'}</Text>
</Text>
</View>
<Text style={componentStyles.frameLocation}>
{getPrettyFileName(frame.fileName)}
{frame.location ? `:${frame.location.row}` : ''}
</Text>
</View>
<Text style={componentStyles.frameLocation}>{frame.location}</Text>
</LogBoxButton>
</View>
))}
<View style={componentStyles.collapseContainer}>
Expand Down Expand Up @@ -96,9 +144,15 @@ const componentStyles = StyleSheet.create({
paddingVertical: 5,
paddingHorizontal: 10,
},
frameContainer: {
flexDirection: 'row',
paddingHorizontal: 15,
},
frame: {
paddingHorizontal: 25,
flex: 1,
paddingVertical: 4,
paddingHorizontal: 10,
borderRadius: 5,
},
component: {
flexDirection: 'row',
Expand Down
127 changes: 124 additions & 3 deletions Libraries/LogBox/UI/__tests__/LogBoxInspectorReactFrames-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ describe('LogBoxInspectorReactFrames', () => {
expect(output).toMatchSnapshot();
});

it('should render componentStack frames', () => {
it('should render componentStack frames without full path pressable', () => {
const output = render.shallowRender(
<LogBoxInspectorReactFrames
log={
Expand All @@ -55,8 +55,129 @@ describe('LogBoxInspectorReactFrames', () => {
category: 'Some kind of message',
componentStack: [
{
component: 'MyComponent',
location: 'MyComponentFile.js:1',
content: 'MyComponent',
fileName: 'MyComponentFile.js',
location: {
row: 1,
column: -1,
},
},
],
})
}
/>,
);

expect(output).toMatchSnapshot();
});

it('should render componentStack frames with full path pressable', () => {
const output = render.shallowRender(
<LogBoxInspectorReactFrames
log={
new LogBoxLog({
level: 'warn',
isComponentError: false,
message: {
content: 'Some kind of message',
substitutions: [],
},
stack: [],
category: 'Some kind of message',
componentStack: [
{
content: 'MyComponent',
fileName: '/path/to/MyComponentFile.js',
location: {
row: 1,
column: -1,
},
},
],
})
}
/>,
);

expect(output).toMatchSnapshot();
});

it('should render componentStack frames with parent folder of index.js', () => {
const output = render.shallowRender(
<LogBoxInspectorReactFrames
log={
new LogBoxLog({
level: 'warn',
isComponentError: false,
message: {
content: 'Some kind of message',
substitutions: [],
},
stack: [],
category: 'Some kind of message',
componentStack: [
{
content: 'MyComponent',
fileName: '/path/to/index.js',
location: {
row: 1,
column: -1,
},
},
],
})
}
/>,
);

expect(output).toMatchSnapshot();
});

it('should render componentStack frames with more than 3 stacks', () => {
const output = render.shallowRender(
<LogBoxInspectorReactFrames
log={
new LogBoxLog({
level: 'warn',
isComponentError: false,
message: {
content: 'Some kind of message',
substitutions: [],
},
stack: [],
category: 'Some kind of message',
componentStack: [
{
content: 'MyComponent',
fileName: '/path/to/index.js',
location: {
row: 1,
column: -1,
},
},
{
content: 'MyComponent2',
fileName: '/path/to/index2.js',
location: {
row: 1,
column: -1,
},
},
{
content: 'MyComponent3',
fileName: '/path/to/index3.js',
location: {
row: 1,
column: -1,
},
},
{
content: 'MyComponent4',
fileName: '/path/to/index4.js',
location: {
row: 1,
column: -1,
},
},
],
})
Expand Down
Loading

0 comments on commit bbfd64e

Please sign in to comment.