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,