From ba60f9ed9501daf0428a888cc0ae4d6ede7d2fdf Mon Sep 17 00:00:00 2001 From: Billy Janitsch Date: Fri, 19 Jan 2018 21:16:32 -0500 Subject: [PATCH 1/7] Improve display of filenames in component stack --- packages/shared/describeComponentFrame.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/shared/describeComponentFrame.js b/packages/shared/describeComponentFrame.js index 5c619e6164e..70e16aedeec 100644 --- a/packages/shared/describeComponentFrame.js +++ b/packages/shared/describeComponentFrame.js @@ -17,7 +17,7 @@ export default function( (name || 'Unknown') + (source ? ' (at ' + - source.fileName.replace(/^.*[\\\/]/, '') + + source.fileName.match(/[^\\\/]*([\\\/]index\.[^\\\/]+)?$/)[0] + ':' + source.lineNumber + ')' From 1add3631bd2acdc13203b44eff8ad17cf0718b35 Mon Sep 17 00:00:00 2001 From: Billy Janitsch Date: Sat, 20 Jan 2018 14:47:37 -0500 Subject: [PATCH 2/7] Add explanatory comment --- packages/shared/describeComponentFrame.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/shared/describeComponentFrame.js b/packages/shared/describeComponentFrame.js index 70e16aedeec..1bc2e8f545a 100644 --- a/packages/shared/describeComponentFrame.js +++ b/packages/shared/describeComponentFrame.js @@ -17,6 +17,8 @@ export default function( (name || 'Unknown') + (source ? ' (at ' + + // Extract filename, or folder/filename in the case of an + // index file, e.g. "Foo.js", or "Bar/index.js" source.fileName.match(/[^\\\/]*([\\\/]index\.[^\\\/]+)?$/)[0] + ':' + source.lineNumber + From d4be46af30894563c73ee7e8cbf2457b15bc988f Mon Sep 17 00:00:00 2001 From: raunofreiberg Date: Tue, 27 Feb 2018 23:50:14 +0200 Subject: [PATCH 3/7] tests: add tests for component stack trace displaying --- .../__tests__/describeComponentFrame-test.js | 65 +++++++++++++++++++ 1 file changed, 65 insertions(+) create mode 100644 packages/shared/__tests__/describeComponentFrame-test.js diff --git a/packages/shared/__tests__/describeComponentFrame-test.js b/packages/shared/__tests__/describeComponentFrame-test.js new file mode 100644 index 00000000000..890a2be92ab --- /dev/null +++ b/packages/shared/__tests__/describeComponentFrame-test.js @@ -0,0 +1,65 @@ +/** + * Copyright (c) 2016-present, Facebook, Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @emails react-core + */ + +'use strict'; + +let React; +let ReactTestUtils; + +describe('Component stack trace displaying', () => { + beforeEach(() => { + React = require('react'); + ReactTestUtils = require('react-dom/test-utils'); + spyOnDev(console, 'error'); + }); + + it('should provide stack trace to closest directory and index.js', () => { + class Component extends React.Component { + render() { + return [a, b]; + } + } + + ReactTestUtils.renderIntoDocument( + , + ); + + if (__DEV__) { + expect(console.error.calls.count()).toBe(1); + expect(console.error.calls.argsFor(0)[0]).toBe( + 'Warning: Each child in an array or iterator should have a unique "key" prop. ' + + 'See https://fb.me/react-warning-keys for more information.\n' + + ' in Component (at Baz/index.js:25)', + ); + } + }); + + it('should provide stack trace to solely a file when not named index.js', () => { + class Component extends React.Component { + render() { + return [a, b]; + } + } + + ReactTestUtils.renderIntoDocument( + , + ); + + if (__DEV__) { + expect(console.error.calls.count()).toBe(1); + expect(console.error.calls.argsFor(0)[0]).toBe( + 'Warning: Each child in an array or iterator should have a unique "key" prop. ' + + 'See https://fb.me/react-warning-keys for more information.\n' + + ' in Component (at Foo.js:25)', + ); + } + }); +}); From a60975b5afd0e62f077c3ac5f1479ee7025d464c Mon Sep 17 00:00:00 2001 From: Dan Date: Thu, 9 Aug 2018 02:32:45 +0100 Subject: [PATCH 4/7] Tweak test --- .../__tests__/describeComponentFrame-test.js | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/packages/shared/__tests__/describeComponentFrame-test.js b/packages/shared/__tests__/describeComponentFrame-test.js index 890a2be92ab..6560e5d63c1 100644 --- a/packages/shared/__tests__/describeComponentFrame-test.js +++ b/packages/shared/__tests__/describeComponentFrame-test.js @@ -10,13 +10,12 @@ 'use strict'; let React; -let ReactTestUtils; +let ReactDOM; describe('Component stack trace displaying', () => { beforeEach(() => { React = require('react'); - ReactTestUtils = require('react-dom/test-utils'); - spyOnDev(console, 'error'); + ReactDOM = require('react-dom'); }); it('should provide stack trace to closest directory and index.js', () => { @@ -26,19 +25,20 @@ describe('Component stack trace displaying', () => { } } - ReactTestUtils.renderIntoDocument( + spyOnDev(console, 'error'); + const container = document.createElement('div'); + ReactDOM.render( , + container, ); if (__DEV__) { expect(console.error.calls.count()).toBe(1); - expect(console.error.calls.argsFor(0)[0]).toBe( - 'Warning: Each child in an array or iterator should have a unique "key" prop. ' + - 'See https://fb.me/react-warning-keys for more information.\n' + - ' in Component (at Baz/index.js:25)', - ); + const args = console.error.calls.argsFor(0); + const stack = args[args.length - 1]; + expect(stack).toBe('\n in Component (at Baz/index.js:25)'); } }); @@ -49,17 +49,17 @@ describe('Component stack trace displaying', () => { } } - ReactTestUtils.renderIntoDocument( + spyOnDev(console, 'error'); + const container = document.createElement('div'); + ReactDOM.render( , + container, ); if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - expect(console.error.calls.argsFor(0)[0]).toBe( - 'Warning: Each child in an array or iterator should have a unique "key" prop. ' + - 'See https://fb.me/react-warning-keys for more information.\n' + - ' in Component (at Foo.js:25)', - ); + const args = console.error.calls.argsFor(0); + const stack = args[args.length - 1]; + expect(stack).toBe('\n in Component (at Foo.js:25)'); } }); }); From 86d2afe6ce4faf5506c39b7a6dba34a0a833bc56 Mon Sep 17 00:00:00 2001 From: Dan Date: Thu, 9 Aug 2018 02:46:58 +0100 Subject: [PATCH 5/7] Rewrite test and revert implementation --- .../__tests__/describeComponentFrame-test.js | 64 +++++++++---------- packages/shared/describeComponentFrame.js | 4 +- 2 files changed, 33 insertions(+), 35 deletions(-) diff --git a/packages/shared/__tests__/describeComponentFrame-test.js b/packages/shared/__tests__/describeComponentFrame-test.js index 6560e5d63c1..c6bda8cf76e 100644 --- a/packages/shared/__tests__/describeComponentFrame-test.js +++ b/packages/shared/__tests__/describeComponentFrame-test.js @@ -18,7 +18,7 @@ describe('Component stack trace displaying', () => { ReactDOM = require('react-dom'); }); - it('should provide stack trace to closest directory and index.js', () => { + it('should provide filenames in stack traces', () => { class Component extends React.Component { render() { return [a, b]; @@ -27,39 +27,39 @@ describe('Component stack trace displaying', () => { spyOnDev(console, 'error'); const container = document.createElement('div'); - ReactDOM.render( - , - container, - ); - + const fileNames = { + 'Foo.js': 'Foo.js', + 'Foo.jsx': 'Foo.jsx', + 'Bar/Foo.js': 'Foo.js', + 'Bar/Foo.jsx': 'Foo.jsx', + 'Bar\\Foo.js': 'Foo.js', + 'Bar\\Foo.jsx': 'Foo.jsx', + 'Bar/Baz/Foo.js': 'Foo.js', + 'Bar/Baz/Foo.jsx': 'Foo.jsx', + 'Bar\\Baz\\Foo.js': 'Foo.js', + 'Bar\\Baz\\Foo.jsx': 'Foo.jsx', + 'C:\\funny long (path)/Foo.js': 'Foo.js', + 'C:\\funny long (path)/Foo.jsx': 'Foo.jsx', + }; + Object.keys(fileNames).forEach((fileName, i) => { + ReactDOM.render( + , + container, + ); + }); if (__DEV__) { - expect(console.error.calls.count()).toBe(1); - const args = console.error.calls.argsFor(0); - const stack = args[args.length - 1]; - expect(stack).toBe('\n in Component (at Baz/index.js:25)'); - } - }); - - it('should provide stack trace to solely a file when not named index.js', () => { - class Component extends React.Component { - render() { - return [a, b]; + let i = 0; + expect(console.error.calls.count()).toBe(Object.keys(fileNames).length); + for (let fileName in fileNames) { + if (!fileNames.hasOwnProperty(fileName)) { + continue; + } + const args = console.error.calls.argsFor(i); + const stack = args[args.length - 1]; + const expected = fileNames[fileName]; + expect(stack).toContain(`at ${expected}:`); + i++; } } - - spyOnDev(console, 'error'); - const container = document.createElement('div'); - ReactDOM.render( - , - container, - ); - - if (__DEV__) { - const args = console.error.calls.argsFor(0); - const stack = args[args.length - 1]; - expect(stack).toBe('\n in Component (at Foo.js:25)'); - } }); }); diff --git a/packages/shared/describeComponentFrame.js b/packages/shared/describeComponentFrame.js index 1bc2e8f545a..5c619e6164e 100644 --- a/packages/shared/describeComponentFrame.js +++ b/packages/shared/describeComponentFrame.js @@ -17,9 +17,7 @@ export default function( (name || 'Unknown') + (source ? ' (at ' + - // Extract filename, or folder/filename in the case of an - // index file, e.g. "Foo.js", or "Bar/index.js" - source.fileName.match(/[^\\\/]*([\\\/]index\.[^\\\/]+)?$/)[0] + + source.fileName.replace(/^.*[\\\/]/, '') + ':' + source.lineNumber + ')' From 287bad97b358ed7e35163c6900e17d74823a3e19 Mon Sep 17 00:00:00 2001 From: Dan Date: Thu, 9 Aug 2018 02:51:41 +0100 Subject: [PATCH 6/7] Extract a variable --- packages/shared/describeComponentFrame.js | 26 +++++++++++------------ 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/packages/shared/describeComponentFrame.js b/packages/shared/describeComponentFrame.js index 5c619e6164e..f0027e50b7e 100644 --- a/packages/shared/describeComponentFrame.js +++ b/packages/shared/describeComponentFrame.js @@ -12,17 +12,17 @@ export default function( source: any, ownerName: null | string, ) { - return ( - '\n in ' + - (name || 'Unknown') + - (source - ? ' (at ' + - source.fileName.replace(/^.*[\\\/]/, '') + - ':' + - source.lineNumber + - ')' - : ownerName - ? ' (created by ' + ownerName + ')' - : '') - ); + let sourceInfo = ''; + if (source) { + sourceInfo = + ' (at ' + + source.fileName.replace(/^.*[\\\/]/, '') + + ':' + + source.lineNumber + + ')'; + } else if (ownerName) { + sourceInfo = ' (created by ' + ownerName + ')'; + } + + return '\n in ' + (name || 'Unknown') + sourceInfo; } From 69f7c41b02143d4b116a5f223b25b436e77b9d67 Mon Sep 17 00:00:00 2001 From: Dan Date: Thu, 9 Aug 2018 03:29:16 +0100 Subject: [PATCH 7/7] Rewrite implementation --- .../__tests__/describeComponentFrame-test.js | 46 +++++++++++++++++++ packages/shared/describeComponentFrame.js | 23 +++++++--- 2 files changed, 62 insertions(+), 7 deletions(-) diff --git a/packages/shared/__tests__/describeComponentFrame-test.js b/packages/shared/__tests__/describeComponentFrame-test.js index c6bda8cf76e..5aabebaaddc 100644 --- a/packages/shared/__tests__/describeComponentFrame-test.js +++ b/packages/shared/__tests__/describeComponentFrame-test.js @@ -28,18 +28,64 @@ describe('Component stack trace displaying', () => { spyOnDev(console, 'error'); const container = document.createElement('div'); const fileNames = { + '': '', + '/': '', + '\\': '', + Foo: 'Foo', + 'Bar/Foo': 'Foo', + 'Bar\\Foo': 'Foo', + 'Baz/Bar/Foo': 'Foo', + 'Baz\\Bar\\Foo': 'Foo', + 'Foo.js': 'Foo.js', 'Foo.jsx': 'Foo.jsx', + '/Foo.js': 'Foo.js', + '/Foo.jsx': 'Foo.jsx', + '\\Foo.js': 'Foo.js', + '\\Foo.jsx': 'Foo.jsx', 'Bar/Foo.js': 'Foo.js', 'Bar/Foo.jsx': 'Foo.jsx', 'Bar\\Foo.js': 'Foo.js', 'Bar\\Foo.jsx': 'Foo.jsx', + '/Bar/Foo.js': 'Foo.js', + '/Bar/Foo.jsx': 'Foo.jsx', + '\\Bar\\Foo.js': 'Foo.js', + '\\Bar\\Foo.jsx': 'Foo.jsx', 'Bar/Baz/Foo.js': 'Foo.js', 'Bar/Baz/Foo.jsx': 'Foo.jsx', 'Bar\\Baz\\Foo.js': 'Foo.js', 'Bar\\Baz\\Foo.jsx': 'Foo.jsx', + '/Bar/Baz/Foo.js': 'Foo.js', + '/Bar/Baz/Foo.jsx': 'Foo.jsx', + '\\Bar\\Baz\\Foo.js': 'Foo.js', + '\\Bar\\Baz\\Foo.jsx': 'Foo.jsx', 'C:\\funny long (path)/Foo.js': 'Foo.js', 'C:\\funny long (path)/Foo.jsx': 'Foo.jsx', + + 'index.js': 'index.js', + 'index.jsx': 'index.jsx', + '/index.js': 'index.js', + '/index.jsx': 'index.jsx', + '\\index.js': 'index.js', + '\\index.jsx': 'index.jsx', + 'Bar/index.js': 'Bar/index.js', + 'Bar/index.jsx': 'Bar/index.jsx', + 'Bar\\index.js': 'Bar/index.js', + 'Bar\\index.jsx': 'Bar/index.jsx', + '/Bar/index.js': 'Bar/index.js', + '/Bar/index.jsx': 'Bar/index.jsx', + '\\Bar\\index.js': 'Bar/index.js', + '\\Bar\\index.jsx': 'Bar/index.jsx', + 'Bar/Baz/index.js': 'Baz/index.js', + 'Bar/Baz/index.jsx': 'Baz/index.jsx', + 'Bar\\Baz\\index.js': 'Baz/index.js', + 'Bar\\Baz\\index.jsx': 'Baz/index.jsx', + '/Bar/Baz/index.js': 'Baz/index.js', + '/Bar/Baz/index.jsx': 'Baz/index.jsx', + '\\Bar\\Baz\\index.js': 'Baz/index.js', + '\\Bar\\Baz\\index.jsx': 'Baz/index.jsx', + 'C:\\funny long (path)/index.js': 'funny long (path)/index.js', + 'C:\\funny long (path)/index.jsx': 'funny long (path)/index.jsx', }; Object.keys(fileNames).forEach((fileName, i) => { ReactDOM.render( diff --git a/packages/shared/describeComponentFrame.js b/packages/shared/describeComponentFrame.js index f0027e50b7e..23c5a9815f7 100644 --- a/packages/shared/describeComponentFrame.js +++ b/packages/shared/describeComponentFrame.js @@ -7,6 +7,8 @@ * @flow */ +const BEFORE_SLASH_RE = /^(.*)[\\\/]/; + export default function( name: null | string, source: any, @@ -14,15 +16,22 @@ export default function( ) { let sourceInfo = ''; if (source) { - sourceInfo = - ' (at ' + - source.fileName.replace(/^.*[\\\/]/, '') + - ':' + - source.lineNumber + - ')'; + let path = source.fileName; + let fileName = path.replace(BEFORE_SLASH_RE, ''); + if (/^index\./.test(fileName)) { + // Special case: include closest folder name for `index.*` filenames. + const match = path.match(BEFORE_SLASH_RE); + if (match) { + const pathBeforeSlash = match[1]; + if (pathBeforeSlash) { + const folderName = pathBeforeSlash.replace(BEFORE_SLASH_RE, ''); + fileName = folderName + '/' + fileName; + } + } + } + sourceInfo = ' (at ' + fileName + ':' + source.lineNumber + ')'; } else if (ownerName) { sourceInfo = ' (created by ' + ownerName + ')'; } - return '\n in ' + (name || 'Unknown') + sourceInfo; }