diff --git a/packages/react-devtools-inline/__tests__/__e2e__/components.test.js b/packages/react-devtools-inline/__tests__/__e2e__/components.test.js index 39467d3a9c932..6d0d4d0fd4ce2 100644 --- a/packages/react-devtools-inline/__tests__/__e2e__/components.test.js +++ b/packages/react-devtools-inline/__tests__/__e2e__/components.test.js @@ -99,8 +99,7 @@ test.describe('Components', () => { expect(propName).toBe('label'); expect(propValue).toBe('"one"'); - expect(sourceText).toBe(null); - // TODO: expect(sourceText).toMatch(/ListApp[a-zA-Z]*\.js/); + expect(sourceText).toMatch(/ListApp[a-zA-Z]*\.js/); }); test('should allow props to be edited', async () => { diff --git a/packages/react-devtools-shared/src/__tests__/inspectedElement-test.js b/packages/react-devtools-shared/src/__tests__/inspectedElement-test.js index a863c6144abcc..030731a46533f 100644 --- a/packages/react-devtools-shared/src/__tests__/inspectedElement-test.js +++ b/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); diff --git a/packages/react-devtools-shared/src/__tests__/storeComponentFilters-test.js b/packages/react-devtools-shared/src/__tests__/storeComponentFilters-test.js index 571302961dc51..c83a797b42e0c 100644 --- a/packages/react-devtools-shared/src/__tests__/storeComponentFilters-test.js +++ b/packages/react-devtools-shared/src/__tests__/storeComponentFilters-test.js @@ -226,9 +226,13 @@ describe('Store component filters', () => { // @reactVersion >= 16.0 it('should filter by path', async () => { - const Component = () =>
Hi
; + // This component should use props object in order to throw for component stack generation + // See ReactComponentStackFrame:155 or DevToolsComponentStackFrame:147 + const Component = props => { + return
{props.message}
; + }; - await actAsync(async () => render()); + await actAsync(async () => render()); expect(store).toMatchInlineSnapshot(` [root] ▾ @@ -242,13 +246,7 @@ describe('Store component filters', () => { ]), ); - // TODO: Filtering should work on component location. - // expect(store).toMatchInlineSnapshot(`[root]`); - expect(store).toMatchInlineSnapshot(` - [root] - ▾ -
- `); + expect(store).toMatchInlineSnapshot(`[root]`); await actAsync( async () => @@ -497,19 +495,17 @@ describe('Store component filters', () => { ]), ); - utils.act( - () => - utils.withErrorsOrWarningsIgnored(['test-only:'], () => { - render( - - - - - , - ); - }), - false, - ); + utils.withErrorsOrWarningsIgnored(['test-only:'], () => { + utils.act(() => { + render( + + + + + , + ); + }, false); + }); expect(store).toMatchInlineSnapshot(``); expect(store.errorCount).toBe(0); diff --git a/packages/react-devtools-shared/src/backend/DevToolsComponentStackFrame.js b/packages/react-devtools-shared/src/backend/DevToolsComponentStackFrame.js index 98d944477d860..90e1316caa9f1 100644 --- a/packages/react-devtools-shared/src/backend/DevToolsComponentStackFrame.js +++ b/packages/react-devtools-shared/src/backend/DevToolsComponentStackFrame.js @@ -74,8 +74,6 @@ export function describeNativeComponentFrame( } } - let control; - const previousPrepareStackTrace = Error.prepareStackTrace; // $FlowFixMe[incompatible-type] It does accept undefined. Error.prepareStackTrace = undefined; @@ -91,64 +89,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 @@ -167,7 +241,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 "" + // but we have a user-provided "displayName" + // splice it in to make the stack more readable. + if (fn.displayName && frame.includes('')) { + frame = frame.replace('', fn.displayName); + } + if (__DEV__) { if (typeof fn === 'function') { componentFrameCache.set(fn, frame); diff --git a/packages/react-devtools-shared/src/backend/legacy/renderer.js b/packages/react-devtools-shared/src/backend/legacy/renderer.js index 8f8af5594f8d4..e037d45075a80 100644 --- a/packages/react-devtools-shared/src/backend/legacy/renderer.js +++ b/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, diff --git a/packages/react-devtools-shared/src/backend/renderer.js b/packages/react-devtools-shared/src/backend/renderer.js index 0327ec6576d8e..1fa36dc268cf6 100644 --- a/packages/react-devtools-shared/src/backend/renderer.js +++ b/packages/react-devtools-shared/src/backend/renderer.js @@ -41,6 +41,7 @@ import {sessionStorageGetItem} from 'react-devtools-shared/src/storage'; import { gt, gte, + parseSourceFromComponentStack, serializeToString, } from 'react-devtools-shared/src/backend/utils'; import { @@ -123,6 +124,8 @@ import type { ElementType, Plugins, } from 'react-devtools-shared/src/frontend/types'; +import type {Source} from 'react-devtools-shared/src/shared/types'; +import {getStackByFiberInDevAndProd} from './DevToolsFiberComponentStack'; type getDisplayNameForFiberType = (fiber: Fiber) => string | null; type getTypeSymbolType = (type: any) => symbol | number; @@ -584,6 +587,8 @@ const fiberToIDMap: Map = new Map(); // operations that should be the same whether the current and work-in-progress Fiber is used. const idToArbitraryFiberMap: Map = new Map(); +const fiberToComponentStackMap: WeakMap = new WeakMap(); + export function attach( hook: DevToolsHook, rendererID: number, @@ -1024,15 +1029,19 @@ export function attach( } } - // TODO: Figure out a way to filter by path in the new model which has no debug info. - // if (hideElementsWithPaths.size > 0) { - // const {fileName} = ...; - // for (const pathRegExp of hideElementsWithPaths) { - // if (pathRegExp.test(fileName)) { - // return true; - // } - // } - // } + if (hideElementsWithPaths.size > 0) { + const source = getSourceForFiber(fiber); + + if (source != null) { + const {fileName} = source; + // eslint-disable-next-line no-for-of-loops/no-for-of-loops + for (const pathRegExp of hideElementsWithPaths) { + if (pathRegExp.test(fileName)) { + return true; + } + } + } + } return false; } @@ -1241,10 +1250,12 @@ export function attach( } fiberToIDMap.delete(fiber); + fiberToComponentStackMap.delete(fiber); const {alternate} = fiber; if (alternate !== null) { fiberToIDMap.delete(alternate); + fiberToComponentStackMap.delete(alternate); } if (forceErrorForFiberIDs.has(fiberID)) { @@ -3356,6 +3367,11 @@ export function attach( } } + let source = null; + if (canViewSource) { + source = getSourceForFiber(fiber); + } + return { id, @@ -3388,6 +3404,7 @@ export function attach( // Can view component source location. canViewSource, + source, // Does the component have legacy context attached to it. hasLegacyContext, @@ -4515,6 +4532,34 @@ export function attach( return idToArbitraryFiberMap.has(id); } + function getComponentStackForFiber(fiber: Fiber): string | null { + let componentStack = fiberToComponentStackMap.get(fiber); + if (componentStack == null) { + const dispatcherRef = renderer.currentDispatcherRef; + if (dispatcherRef == null) { + return null; + } + + componentStack = getStackByFiberInDevAndProd( + ReactTypeOfWork, + fiber, + dispatcherRef, + ); + fiberToComponentStackMap.set(fiber, componentStack); + } + + return componentStack; + } + + function getSourceForFiber(fiber: Fiber): Source | null { + const componentStack = getComponentStackForFiber(fiber); + if (componentStack == null) { + return null; + } + + return parseSourceFromComponentStack(componentStack); + } + return { cleanup, clearErrorsAndWarnings, @@ -4525,6 +4570,8 @@ export function attach( findNativeNodesForFiberID, flushInitialOperations, getBestMatchForTrackedPath, + getComponentStackForFiber, + getSourceForFiber, getDisplayNameForFiberID, getFiberForNative, getFiberIDForNative, diff --git a/packages/react-devtools-shared/src/backend/types.js b/packages/react-devtools-shared/src/backend/types.js index 6b0cd5ee6c2b0..df45122f6314f 100644 --- a/packages/react-devtools-shared/src/backend/types.js +++ b/packages/react-devtools-shared/src/backend/types.js @@ -29,6 +29,7 @@ import type {InitBackend} from 'react-devtools-shared/src/backend'; import type {TimelineDataExport} from 'react-devtools-timeline/src/types'; import type {BrowserTheme} from 'react-devtools-shared/src/frontend/types'; import type {BackendBridge} from 'react-devtools-shared/src/bridge'; +import type {Source} from 'react-devtools-shared/src/shared/types'; import type Agent from './agent'; type BundleType = @@ -278,6 +279,7 @@ export type InspectedElement = { // List of owners owners: Array | null, + source: Source | null, type: ElementType, diff --git a/packages/react-devtools-shared/src/backend/utils.js b/packages/react-devtools-shared/src/backend/utils.js index a1975e43c6323..ddc1151a76bc2 100644 --- a/packages/react-devtools-shared/src/backend/utils.js +++ b/packages/react-devtools-shared/src/backend/utils.js @@ -12,6 +12,7 @@ import {compareVersions} from 'compare-versions'; import {dehydrate} from '../hydration'; import isArray from 'shared/isArray'; +import type {Source} from 'react-devtools-shared/src/shared/types'; import type {DehydratedData} from 'react-devtools-shared/src/frontend/types'; // TODO: update this to the first React version that has a corresponding DevTools backend @@ -289,3 +290,35 @@ export const isReactNativeEnvironment = (): boolean => { // We should probably define the client for DevTools on the backend side and share it with the frontend return window.document == null; }; + +export function parseSourceFromComponentStack( + componentStack: string, +): Source | null { + const frames = componentStack.split('\n'); + // eslint-disable-next-line no-for-of-loops/no-for-of-loops + for (const frame of frames) { + const openingBracketIndex = frame.lastIndexOf('('); + if (openingBracketIndex === -1) continue; + const closingBracketIndex = frame.lastIndexOf(')'); + if ( + closingBracketIndex === -1 || + openingBracketIndex >= closingBracketIndex + ) + continue; + + const url = frame.slice(openingBracketIndex + 1, closingBracketIndex); + const match = url.match(/^(.+):(\d+):(\d+)$/); + if (match == null) continue; + + const [, fileURL, lineNumber, columnNumber] = match; + const [, , ...fileNameTokens] = fileURL.split('/').filter(Boolean); + + return { + fileName: fileNameTokens.join('/'), + lineNumber: parseInt(lineNumber, 10), + columnNumber: parseInt(columnNumber, 10), + }; + } + + return null; +} diff --git a/packages/react-devtools-shared/src/backendAPI.js b/packages/react-devtools-shared/src/backendAPI.js index 21ae444a1ef8c..6fcc35b574277 100644 --- a/packages/react-devtools-shared/src/backendAPI.js +++ b/packages/react-devtools-shared/src/backendAPI.js @@ -228,6 +228,7 @@ export function convertInspectedElementBackendToFrontend( id, type, owners, + source, context, hooks, plugins, @@ -260,7 +261,7 @@ export function convertInspectedElementBackendToFrontend( rendererPackageName, rendererVersion, rootType, - source: null, // TODO: Load source location lazily. + source, type, owners: owners === null diff --git a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementView.js b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementView.js index 2f1503c65e487..4ccaea7958894 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementView.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementView.js @@ -240,6 +240,8 @@ export default function InspectedElementView({ // This function is based on describeComponentFrame() in packages/shared/ReactComponentStackFrame function formatSourceForDisplay(fileName: string, lineNumber: string) { + // Note: this RegExp doesn't work well with URLs from Metro, + // which provides bundle URL with query parameters prefixed with /& const BEFORE_SLASH_RE = /^(.*)[\\\/]/; let nameOnly = fileName.replace(BEFORE_SLASH_RE, ''); diff --git a/packages/react-devtools-shared/src/frontend/types.js b/packages/react-devtools-shared/src/frontend/types.js index 9623efd3dd230..10c2e62c20cbc 100644 --- a/packages/react-devtools-shared/src/frontend/types.js +++ b/packages/react-devtools-shared/src/frontend/types.js @@ -18,6 +18,7 @@ import type { Dehydrated, Unserializable, } from 'react-devtools-shared/src/hydration'; +import type {Source} from 'react-devtools-shared/src/shared/types'; export type BrowserTheme = 'dark' | 'light'; @@ -219,7 +220,7 @@ export type InspectedElement = { owners: Array | null, // Location of component in source code. - source: null, // TODO: Reinstate a way to load this lazily. + source: Source | null, type: ElementType, diff --git a/packages/react-devtools-shared/src/shared/types.js b/packages/react-devtools-shared/src/shared/types.js new file mode 100644 index 0000000000000..b7158d0873f51 --- /dev/null +++ b/packages/react-devtools-shared/src/shared/types.js @@ -0,0 +1,14 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + */ + +export type Source = { + fileName: string, + lineNumber: number, + columnNumber: number, +}; diff --git a/packages/shared/ReactVersion.js b/packages/shared/ReactVersion.js index d0a14cfff7806..9055167e29cc5 100644 --- a/packages/shared/ReactVersion.js +++ b/packages/shared/ReactVersion.js @@ -1,16 +1 @@ -/** - * Copyright (c) Meta Platforms, Inc. and affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -// TODO: this is special because it gets imported during build. -// -// TODO: 18.0.0 has not been released to NPM; -// It exists as a placeholder so that DevTools can support work tag changes between releases. -// When we next publish a release, update the matching TODO in backend/renderer.js -// TODO: This module is used both by the release scripts and to expose a version -// at runtime. We should instead inject the version number as part of the build -// process, and use the ReactVersions.js module as the single source of truth. -export default '18.2.0'; +export default '18.3.0-PLACEHOLDER';