Skip to content

Commit 54d86eb

Browse files
billyjanitschgaearon
authored andcommitted
Improve display of filenames in component stack (#12059)
* Improve display of filenames in component stack * Add explanatory comment * tests: add tests for component stack trace displaying * Tweak test * Rewrite test and revert implementation * Extract a variable * Rewrite implementation
1 parent 067cc24 commit 54d86eb

File tree

2 files changed

+133
-13
lines changed

2 files changed

+133
-13
lines changed
Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
/**
2+
* Copyright (c) 2016-present, Facebook, Inc.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*
7+
* @emails react-core
8+
*/
9+
10+
'use strict';
11+
12+
let React;
13+
let ReactDOM;
14+
15+
describe('Component stack trace displaying', () => {
16+
beforeEach(() => {
17+
React = require('react');
18+
ReactDOM = require('react-dom');
19+
});
20+
21+
it('should provide filenames in stack traces', () => {
22+
class Component extends React.Component {
23+
render() {
24+
return [<span>a</span>, <span>b</span>];
25+
}
26+
}
27+
28+
spyOnDev(console, 'error');
29+
const container = document.createElement('div');
30+
const fileNames = {
31+
'': '',
32+
'/': '',
33+
'\\': '',
34+
Foo: 'Foo',
35+
'Bar/Foo': 'Foo',
36+
'Bar\\Foo': 'Foo',
37+
'Baz/Bar/Foo': 'Foo',
38+
'Baz\\Bar\\Foo': 'Foo',
39+
40+
'Foo.js': 'Foo.js',
41+
'Foo.jsx': 'Foo.jsx',
42+
'/Foo.js': 'Foo.js',
43+
'/Foo.jsx': 'Foo.jsx',
44+
'\\Foo.js': 'Foo.js',
45+
'\\Foo.jsx': 'Foo.jsx',
46+
'Bar/Foo.js': 'Foo.js',
47+
'Bar/Foo.jsx': 'Foo.jsx',
48+
'Bar\\Foo.js': 'Foo.js',
49+
'Bar\\Foo.jsx': 'Foo.jsx',
50+
'/Bar/Foo.js': 'Foo.js',
51+
'/Bar/Foo.jsx': 'Foo.jsx',
52+
'\\Bar\\Foo.js': 'Foo.js',
53+
'\\Bar\\Foo.jsx': 'Foo.jsx',
54+
'Bar/Baz/Foo.js': 'Foo.js',
55+
'Bar/Baz/Foo.jsx': 'Foo.jsx',
56+
'Bar\\Baz\\Foo.js': 'Foo.js',
57+
'Bar\\Baz\\Foo.jsx': 'Foo.jsx',
58+
'/Bar/Baz/Foo.js': 'Foo.js',
59+
'/Bar/Baz/Foo.jsx': 'Foo.jsx',
60+
'\\Bar\\Baz\\Foo.js': 'Foo.js',
61+
'\\Bar\\Baz\\Foo.jsx': 'Foo.jsx',
62+
'C:\\funny long (path)/Foo.js': 'Foo.js',
63+
'C:\\funny long (path)/Foo.jsx': 'Foo.jsx',
64+
65+
'index.js': 'index.js',
66+
'index.jsx': 'index.jsx',
67+
'/index.js': 'index.js',
68+
'/index.jsx': 'index.jsx',
69+
'\\index.js': 'index.js',
70+
'\\index.jsx': 'index.jsx',
71+
'Bar/index.js': 'Bar/index.js',
72+
'Bar/index.jsx': 'Bar/index.jsx',
73+
'Bar\\index.js': 'Bar/index.js',
74+
'Bar\\index.jsx': 'Bar/index.jsx',
75+
'/Bar/index.js': 'Bar/index.js',
76+
'/Bar/index.jsx': 'Bar/index.jsx',
77+
'\\Bar\\index.js': 'Bar/index.js',
78+
'\\Bar\\index.jsx': 'Bar/index.jsx',
79+
'Bar/Baz/index.js': 'Baz/index.js',
80+
'Bar/Baz/index.jsx': 'Baz/index.jsx',
81+
'Bar\\Baz\\index.js': 'Baz/index.js',
82+
'Bar\\Baz\\index.jsx': 'Baz/index.jsx',
83+
'/Bar/Baz/index.js': 'Baz/index.js',
84+
'/Bar/Baz/index.jsx': 'Baz/index.jsx',
85+
'\\Bar\\Baz\\index.js': 'Baz/index.js',
86+
'\\Bar\\Baz\\index.jsx': 'Baz/index.jsx',
87+
'C:\\funny long (path)/index.js': 'funny long (path)/index.js',
88+
'C:\\funny long (path)/index.jsx': 'funny long (path)/index.jsx',
89+
};
90+
Object.keys(fileNames).forEach((fileName, i) => {
91+
ReactDOM.render(
92+
<Component __source={{fileName, lineNumber: i}} />,
93+
container,
94+
);
95+
});
96+
if (__DEV__) {
97+
let i = 0;
98+
expect(console.error.calls.count()).toBe(Object.keys(fileNames).length);
99+
for (let fileName in fileNames) {
100+
if (!fileNames.hasOwnProperty(fileName)) {
101+
continue;
102+
}
103+
const args = console.error.calls.argsFor(i);
104+
const stack = args[args.length - 1];
105+
const expected = fileNames[fileName];
106+
expect(stack).toContain(`at ${expected}:`);
107+
i++;
108+
}
109+
}
110+
});
111+
});

packages/shared/describeComponentFrame.js

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -7,22 +7,31 @@
77
* @flow
88
*/
99

10+
const BEFORE_SLASH_RE = /^(.*)[\\\/]/;
11+
1012
export default function(
1113
name: null | string,
1214
source: any,
1315
ownerName: null | string,
1416
) {
15-
return (
16-
'\n in ' +
17-
(name || 'Unknown') +
18-
(source
19-
? ' (at ' +
20-
source.fileName.replace(/^.*[\\\/]/, '') +
21-
':' +
22-
source.lineNumber +
23-
')'
24-
: ownerName
25-
? ' (created by ' + ownerName + ')'
26-
: '')
27-
);
17+
let sourceInfo = '';
18+
if (source) {
19+
let path = source.fileName;
20+
let fileName = path.replace(BEFORE_SLASH_RE, '');
21+
if (/^index\./.test(fileName)) {
22+
// Special case: include closest folder name for `index.*` filenames.
23+
const match = path.match(BEFORE_SLASH_RE);
24+
if (match) {
25+
const pathBeforeSlash = match[1];
26+
if (pathBeforeSlash) {
27+
const folderName = pathBeforeSlash.replace(BEFORE_SLASH_RE, '');
28+
fileName = folderName + '/' + fileName;
29+
}
30+
}
31+
}
32+
sourceInfo = ' (at ' + fileName + ':' + source.lineNumber + ')';
33+
} else if (ownerName) {
34+
sourceInfo = ' (created by ' + ownerName + ')';
35+
}
36+
return '\n in ' + (name || 'Unknown') + sourceInfo;
2837
}

0 commit comments

Comments
 (0)