refactor[devtools]: lazily define source for fiber based on component…

… stack

packages/react-devtools-core/src/standalone.js

@@ -144,7 +144,7 @@ function canViewElementSourceFunction(
 
   const {source} = inspectedElement;
 
-  return doesFilePathExist(source.fileName, projectRoots);
+  return doesFilePathExist(source.sourceURL, projectRoots);
 }
 
 function viewElementSourceFunction(
@@ -153,7 +153,7 @@ function viewElementSourceFunction(
 ): void {
   const {source} = inspectedElement;
   if (source !== null) {
-    launchEditor(source.fileName, source.lineNumber, projectRoots);
+    launchEditor(source.sourceURL, source.line, projectRoots);
   } else {
     log.error('Cannot inspect element', id);
   }

packages/react-devtools-inline/__tests__/__e2e__/components.test.js

@@ -92,15 +92,14 @@ test.describe('Components', () => {
           ? valueElement.value
           : valueElement.innerText;
 
-        return [name, value, source ? source.innerText : null];
+        return [name, value, source.innerText];
       },
       {name: isEditableName, value: isEditableValue}
     );
 
     expect(propName).toBe('label');
     expect(propValue).toBe('"one"');
-    expect(sourceText).toBe(null);
-    // TODO: expect(sourceText).toMatch(/ListApp[a-zA-Z]*\.js/);
+    expect(sourceText).toMatch(/e2e-app[a-zA-Z]*\.js/);
   });
 
   test('should allow props to be edited', async () => {

packages/react-devtools-shared/src/__tests__/inspectedElement-test.js

@@ -424,7 +424,9 @@ describe('InspectedElement', () => {
     targetRenderCount = 0;
 
     let inspectedElement = await inspectElementAtIndex(1);
-    expect(targetRenderCount).toBe(1);
+    // One more because we call render function for generating component stack,
+    // which is required for defining source location
+    expect(targetRenderCount).toBe(2);
     expect(inspectedElement.props).toMatchInlineSnapshot(`
       {
         "a": 1,
@@ -485,7 +487,9 @@ describe('InspectedElement', () => {
     targetRenderCount = 0;
 
     let inspectedElement = await inspectElementAtIndex(1);
-    expect(targetRenderCount).toBe(1);
+    // One more because we call render function for generating component stack,
+    // which is required for defining source location
+    expect(targetRenderCount).toBe(2);
     expect(inspectedElement.props).toMatchInlineSnapshot(`
       {
         "a": 1,
@@ -555,7 +559,9 @@ describe('InspectedElement', () => {
     const inspectedElement = await inspectElementAtIndex(0);
 
     expect(inspectedElement).not.toBe(null);
-    expect(targetRenderCount).toBe(2);
+    // One more because we call render function for generating component stack,
+    // which is required for defining source location
+    expect(targetRenderCount).toBe(3);
     expect(console.error).toHaveBeenCalledTimes(1);
     expect(console.info).toHaveBeenCalledTimes(1);
     expect(console.log).toHaveBeenCalledTimes(1);

packages/react-devtools-shared/src/__tests__/storeComponentFilters-test.js

@@ -224,11 +224,16 @@ describe('Store component filters', () => {
     `);
   });
 
+  // Disabled: filtering by path was removed, source is now determined lazily, including symbolication if applicable
   // @reactVersion >= 16.0
-  it('should filter by path', async () => {
-    const Component = () => <div>Hi</div>;
+  xit('should filter by path', async () => {
+    // This component should use props object in order to throw for component stack generation
+    // See ReactComponentStackFrame:155 or DevToolsComponentStackFrame:147
+    const Component = props => {
+      return <div>{props.message}</div>;
+    };
 
-    await actAsync(async () => render(<Component />));
+    await actAsync(async () => render(<Component message="Hi" />));
     expect(store).toMatchInlineSnapshot(`
       [root]
         ▾ <Component>
@@ -242,13 +247,7 @@ describe('Store component filters', () => {
         ]),
     );
 
-    // TODO: Filtering should work on component location.
-    // expect(store).toMatchInlineSnapshot(`[root]`);
-    expect(store).toMatchInlineSnapshot(`
-      [root]
-        ▾ <Component>
-            <div>
-    `);
+    expect(store).toMatchInlineSnapshot(`[root]`);
 
     await actAsync(
       async () =>
@@ -497,19 +496,17 @@ describe('Store component filters', () => {
           ]),
       );
 
-      utils.act(
-        () =>
-          utils.withErrorsOrWarningsIgnored(['test-only:'], () => {
-            render(
-              <React.Fragment>
-                <ComponentWithError />
-                <ComponentWithWarning />
-                <ComponentWithWarningAndError />
-              </React.Fragment>,
-            );
-          }),
-        false,
-      );
+      utils.withErrorsOrWarningsIgnored(['test-only:'], () => {
+        utils.act(() => {
+          render(
+            <React.Fragment>
+              <ComponentWithError />
+              <ComponentWithWarning />
+              <ComponentWithWarningAndError />
+            </React.Fragment>,
+          );
+        }, false);
+      });
 
       expect(store).toMatchInlineSnapshot(``);
       expect(store.errorCount).toBe(0);

packages/react-devtools-shared/src/backend/DevToolsComponentStackFrame.js

@@ -81,8 +81,6 @@ export function describeNativeComponentFrame(
     }
   }
 
-  let control;
-
   const previousPrepareStackTrace = Error.prepareStackTrace;
   // $FlowFixMe[incompatible-type] It does accept undefined.
   Error.prepareStackTrace = undefined;
@@ -98,64 +96,140 @@ export function describeNativeComponentFrame(
   currentDispatcherRef.current = null;
   disableLogs();
 
-  try {
-    // This should throw.
-    if (construct) {
-      // Something should be setting the props in the constructor.
-      const Fake = function () {
-        throw Error();
-      };
-      // $FlowFixMe[prop-missing]
-      Object.defineProperty(Fake.prototype, 'props', {
-        set: function () {
-          // We use a throwing setter instead of frozen or non-writable props
-          // because that won't throw in a non-strict mode function.
-          throw Error();
-        },
-      });
-      if (typeof Reflect === 'object' && Reflect.construct) {
-        // We construct a different control for this case to include any extra
-        // frames added by the construct call.
-        try {
-          Reflect.construct(Fake, []);
-        } catch (x) {
-          control = x;
+  // NOTE: keep in sync with the implementation in ReactComponentStackFrame
+
+  /**
+   * Finding a common stack frame between sample and control errors can be
+   * tricky given the different types and levels of stack trace truncation from
+   * different JS VMs. So instead we'll attempt to control what that common
+   * frame should be through this object method:
+   * Having both the sample and control errors be in the function under the
+   * `DescribeNativeComponentFrameRoot` property, + setting the `name` and
+   * `displayName` properties of the function ensures that a stack
+   * frame exists that has the method name `DescribeNativeComponentFrameRoot` in
+   * it for both control and sample stacks.
+   */
+  const RunInRootFrame = {
+    DetermineComponentFrameRoot(): [?string, ?string] {
+      let control;
+      try {
+        // This should throw.
+        if (construct) {
+          // Something should be setting the props in the constructor.
+          const Fake = function () {
+            throw Error();
+          };
+          // $FlowFixMe[prop-missing]
+          Object.defineProperty(Fake.prototype, 'props', {
+            set: function () {
+              // We use a throwing setter instead of frozen or non-writable props
+              // because that won't throw in a non-strict mode function.
+              throw Error();
+            },
+          });
+          if (typeof Reflect === 'object' && Reflect.construct) {
+            // We construct a different control for this case to include any extra
+            // frames added by the construct call.
+            try {
+              Reflect.construct(Fake, []);
+            } catch (x) {
+              control = x;
+            }
+            Reflect.construct(fn, [], Fake);
+          } else {
+            try {
+              Fake.call();
+            } catch (x) {
+              control = x;
+            }
+            // $FlowFixMe[prop-missing] found when upgrading Flow
+            fn.call(Fake.prototype);
+          }
+        } else {
+          try {
+            throw Error();
+          } catch (x) {
+            control = x;
+          }
+          // TODO(luna): This will currently only throw if the function component
+          // tries to access React/ReactDOM/props. We should probably make this throw
+          // in simple components too
+          const maybePromise = fn();
+
+          // If the function component returns a promise, it's likely an async
+          // component, which we don't yet support. Attach a noop catch handler to
+          // silence the error.
+          // TODO: Implement component stacks for async client components?
+          if (maybePromise && typeof maybePromise.catch === 'function') {
+            maybePromise.catch(() => {});
+          }
         }
-        Reflect.construct(fn, [], Fake);
-      } else {
-        try {
-          Fake.call();
-        } catch (x) {
-          control = x;
+      } catch (sample) {
+        // This is inlined manually because closure doesn't do it for us.
+        if (sample && control && typeof sample.stack === 'string') {
+          return [sample.stack, control.stack];
         }
-        // $FlowFixMe[prop-missing] found when upgrading Flow
-        fn.call(Fake.prototype);
-      }
-    } else {
-      try {
-        throw Error();
-      } catch (x) {
-        control = x;
       }
-      fn();
-    }
-  } catch (sample) {
-    // This is inlined manually because closure doesn't do it for us.
-    if (sample && control && typeof sample.stack === 'string') {
+      return [null, null];
+    },
+  };
+  // $FlowFixMe[prop-missing]
+  RunInRootFrame.DetermineComponentFrameRoot.displayName =
+    'DetermineComponentFrameRoot';
+  const namePropDescriptor = Object.getOwnPropertyDescriptor(
+    RunInRootFrame.DetermineComponentFrameRoot,
+    'name',
+  );
+  // Before ES6, the `name` property was not configurable.
+  if (namePropDescriptor && namePropDescriptor.configurable) {
+    // V8 utilizes a function's `name` property when generating a stack trace.
+    Object.defineProperty(
+      RunInRootFrame.DetermineComponentFrameRoot,
+      // Configurable properties can be updated even if its writable descriptor
+      // is set to `false`.
+      // $FlowFixMe[cannot-write]
+      'name',
+      {value: 'DetermineComponentFrameRoot'},
+    );
+  }
+
+  try {
+    const [sampleStack, controlStack] =
+      RunInRootFrame.DetermineComponentFrameRoot();
+    if (sampleStack && controlStack) {
       // This extracts the first frame from the sample that isn't also in the control.
       // Skipping one frame that we assume is the frame that calls the two.
-      const sampleLines = sample.stack.split('\n');
-      const controlLines = control.stack.split('\n');
-      let s = sampleLines.length - 1;
-      let c = controlLines.length - 1;
-      while (s >= 1 && c >= 0 && sampleLines[s] !== controlLines[c]) {
-        // We expect at least one stack frame to be shared.
-        // Typically this will be the root most one. However, stack frames may be
-        // cut off due to maximum stack limits. In this case, one maybe cut off
-        // earlier than the other. We assume that the sample is longer or the same
-        // and there for cut off earlier. So we should find the root most frame in
-        // the sample somewhere in the control.
-        c--;
+      const sampleLines = sampleStack.split('\n');
+      const controlLines = controlStack.split('\n');
+      let s = 0;
+      let c = 0;
+      while (
+        s < sampleLines.length &&
+        !sampleLines[s].includes('DetermineComponentFrameRoot')
+      ) {
+        s++;
+      }
+      while (
+        c < controlLines.length &&
+        !controlLines[c].includes('DetermineComponentFrameRoot')
+      ) {
+        c++;
+      }
+      // We couldn't find our intentionally injected common root frame, attempt
+      // to find another common root frame by search from the bottom of the
+      // control stack...
+      if (s === sampleLines.length || c === controlLines.length) {
+        s = sampleLines.length - 1;
+        c = controlLines.length - 1;
+        while (s >= 1 && c >= 0 && sampleLines[s] !== controlLines[c]) {
+          // We expect at least one stack frame to be shared.
+          // Typically this will be the root most one. However, stack frames may be
+          // cut off due to maximum stack limits. In this case, one maybe cut off
+          // earlier than the other. We assume that the sample is longer or the same
+          // and there for cut off earlier. So we should find the root most frame in
+          // the sample somewhere in the control.
+          c--;
+        }
       }
       for (; s >= 1 && c >= 0; s--, c--) {
         // Next we find the first one that isn't the same which should be the
@@ -174,7 +248,15 @@ export function describeNativeComponentFrame(
               // The next one that isn't the same should be our match though.
               if (c < 0 || sampleLines[s] !== controlLines[c]) {
                 // V8 adds a "new" prefix for native classes. Let's remove it to make it prettier.
-                const frame = '\n' + sampleLines[s].replace(' at new ', ' at ');
+                let frame = '\n' + sampleLines[s].replace(' at new ', ' at ');
+
+                // If our component frame is labeled "<anonymous>"
+                // but we have a user-provided "displayName"
+                // splice it in to make the stack more readable.
+                if (fn.displayName && frame.includes('<anonymous>')) {
+                  frame = frame.replace('<anonymous>', fn.displayName);
+                }
+
                 if (__DEV__) {
                   if (typeof fn === 'function') {
                     componentFrameCache.set(fn, frame);

packages/react-devtools-shared/src/backend/legacy/renderer.js

@@ -828,6 +828,7 @@ export function attach(
 
       // Can view component source location.
       canViewSource: type === ElementTypeClass || type === ElementTypeFunction,
+      source: null,
 
       // Only legacy context exists in legacy versions.
       hasLegacyContext: true,