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
facebook · Aug 9, 2018 · 54d86eb · 54d86eb
1 parent 067cc24
commit 54d86eb
Show file tree

Hide file tree

Showing 2 changed files with 133 additions and 13 deletions.
diff --git a/packages/shared/__tests__/describeComponentFrame-test.js b/packages/shared/__tests__/describeComponentFrame-test.js
@@ -0,0 +1,111 @@
+/**
+ * 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 ReactDOM;
+
+describe('Component stack trace displaying', () => {
+  beforeEach(() => {
+    React = require('react');
+    ReactDOM = require('react-dom');
+  });
+
+  it('should provide filenames in stack traces', () => {
+    class Component extends React.Component {
+      render() {
+        return [<span>a</span>, <span>b</span>];
+      }
+    }
+
+    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(
+        <Component __source={{fileName, lineNumber: i}} />,
+        container,
+      );
+    });
+    if (__DEV__) {
+      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++;
+      }
+    }
+  });
+});
diff --git a/packages/shared/describeComponentFrame.js b/packages/shared/describeComponentFrame.js
@@ -7,22 +7,31 @@
  * @flow
  */
 
+const BEFORE_SLASH_RE = /^(.*)[\\\/]/;
+
 export default function(
   name: null | string,
   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) {
+    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;
 }