From 67645265896e9f76af0c64cfbf9518b2f88d88c9 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Tue, 19 Oct 2021 14:47:16 -0400 Subject: [PATCH 1/3] Scheduling Profiler: De-emphasize React internal frames This commit adds code to all React bundles to explicitly register the beginning and ending of their own code. This is done by creating Error objects (which capture a file name, line number, and column number) and registering them to a DevTools hook if present. Next, as the Scheduling Profiler logs metadata to the User Timing API, it prints these module ranges along with other metadata (like Lane values and profiler version number). Lastly, the Scheduling Profiler UI compares stack frames to these ranges when drawing the flame graph and dims or de-emphasizes frames that fall within an internal module. The net effect of this is that user code (and 3rd party code) stands out clearly in the flame graph while React internal modules are dimmed. Internal module ranges are completely optional. Older profiling samples, or ones recorded without the React DevTools extension installed, will simply not dim the internal frames. --- .../src/checkForDuplicateInstallations.js | 8 +- .../src/constants.js | 10 +- .../src/CanvasPage.js | 1 + .../src/content-views/FlamechartView.js | 94 +++++++++++++++++-- .../src/content-views/constants.js | 12 +++ .../__tests__/preprocessData-test.internal.js | 4 + .../src/import-worker/preprocessData.js | 59 ++++++++++++ .../src/types.js | 12 +++ .../src/backend/types.js | 5 + .../react-devtools-shared/src/constants.js | 10 ++ packages/react-devtools-shared/src/hook.js | 25 +++++ .../src/SchedulingProfiler.js | 24 +++++ .../shared/registerInternalModuleStart.js | 20 ++++ packages/shared/registerInternalModuleStop.js | 20 ++++ scripts/rollup/wrappers.js | 41 ++++++++ 15 files changed, 332 insertions(+), 13 deletions(-) create mode 100644 packages/shared/registerInternalModuleStart.js create mode 100644 packages/shared/registerInternalModuleStop.js diff --git a/packages/react-devtools-extensions/src/checkForDuplicateInstallations.js b/packages/react-devtools-extensions/src/checkForDuplicateInstallations.js index d86cac7709295..01db8edb34f74 100644 --- a/packages/react-devtools-extensions/src/checkForDuplicateInstallations.js +++ b/packages/react-devtools-extensions/src/checkForDuplicateInstallations.js @@ -9,13 +9,15 @@ declare var chrome: any; -import {__DEBUG__} from 'react-devtools-shared/src/constants'; +import { + INTERNAL_EXTENSION_ID, + LOCAL_EXTENSION_ID, + __DEBUG__, +} from 'react-devtools-shared/src/constants'; import {getBrowserName} from './utils'; import { EXTENSION_INSTALL_CHECK, EXTENSION_INSTALLATION_TYPE, - INTERNAL_EXTENSION_ID, - LOCAL_EXTENSION_ID, } from './constants'; const IS_CHROME = getBrowserName() === 'Chrome'; diff --git a/packages/react-devtools-extensions/src/constants.js b/packages/react-devtools-extensions/src/constants.js index 310bbaca11563..c17ad4d64acd8 100644 --- a/packages/react-devtools-extensions/src/constants.js +++ b/packages/react-devtools-extensions/src/constants.js @@ -7,6 +7,12 @@ * @flow strict-local */ +import { + CHROME_WEBSTORE_EXTENSION_ID, + INTERNAL_EXTENSION_ID, + LOCAL_EXTENSION_ID, +} from 'react-devtools-shared/src/constants'; + declare var chrome: any; export const CURRENT_EXTENSION_ID = chrome.runtime.id; @@ -15,10 +21,6 @@ export const EXTENSION_INSTALL_CHECK = 'extension-install-check'; export const SHOW_DUPLICATE_EXTENSION_WARNING = 'show-duplicate-extension-warning'; -export const CHROME_WEBSTORE_EXTENSION_ID = 'fmkadmapgofadopljbjfkapdkoienihi'; -export const INTERNAL_EXTENSION_ID = 'dnjnjgbfilfphmojnmhliehogmojhclc'; -export const LOCAL_EXTENSION_ID = 'ikiahnapldjmdmpkmfhjdjilojjhgcbf'; - export const EXTENSION_INSTALLATION_TYPE: | 'public' | 'internal' diff --git a/packages/react-devtools-scheduling-profiler/src/CanvasPage.js b/packages/react-devtools-scheduling-profiler/src/CanvasPage.js index 48efcf8105e7c..ab5c828280c0a 100644 --- a/packages/react-devtools-scheduling-profiler/src/CanvasPage.js +++ b/packages/react-devtools-scheduling-profiler/src/CanvasPage.js @@ -374,6 +374,7 @@ function AutoSizedCanvas({ surface, defaultFrame, data.flamechart, + data.internalModuleSourceToRanges, data.duration, ); flamechartViewRef.current = flamechartView; diff --git a/packages/react-devtools-scheduling-profiler/src/content-views/FlamechartView.js b/packages/react-devtools-scheduling-profiler/src/content-views/FlamechartView.js index 86f411a5b6e24..e1fdf8f4327f4 100644 --- a/packages/react-devtools-scheduling-profiler/src/content-views/FlamechartView.js +++ b/packages/react-devtools-scheduling-profiler/src/content-views/FlamechartView.js @@ -11,6 +11,7 @@ import type { Flamechart, FlamechartStackFrame, FlamechartStackLayer, + InternalModuleSourceToRanges, } from '../types'; import type { Interaction, @@ -20,6 +21,11 @@ import type { ViewRefs, } from '../view-base'; +import { + CHROME_WEBSTORE_EXTENSION_ID, + INTERNAL_EXTENSION_ID, + LOCAL_EXTENSION_ID, +} from 'react-devtools-shared/src/constants'; import { BackgroundColorView, Surface, @@ -69,6 +75,56 @@ function hoverColorForStackFrame(stackFrame: FlamechartStackFrame): string { return hslaColorToString(color); } +function isInternalModule( + internalModuleSourceToRanges: InternalModuleSourceToRanges, + flamechartStackFrame: FlamechartStackFrame, +): boolean { + const {locationColumn, locationLine, scriptUrl} = flamechartStackFrame; + + if (scriptUrl == null || locationColumn == null || locationLine == null) { + return true; + } + + // Internal modules are only registered if DevTools was running when the profile was captured, + // but DevTools should also hide its own frames to avoid over-emphasizing them. + if ( + // Handle webpack-internal:// sources + scriptUrl.includes('/react-devtools') || + scriptUrl.includes('/react_devtools') || + // Filter out known extension IDs + scriptUrl.includes(CHROME_WEBSTORE_EXTENSION_ID) || + scriptUrl.includes(INTERNAL_EXTENSION_ID) || + scriptUrl.includes(LOCAL_EXTENSION_ID) + + // Unfortunately this won't get everything, like relatively loaded chunks or Web Worker files. + ) { + return true; + } + + // Filter out React internal packages. + const ranges = internalModuleSourceToRanges.get(scriptUrl); + if (ranges != null) { + for (let i = 0; i < ranges.length; i++) { + const [startStackFrame, stopStackFrame] = ranges[i]; + + const isAfterStart = + locationLine > startStackFrame.lineNumber || + (locationLine === startStackFrame.lineNumber && + locationColumn >= startStackFrame.columnNumber); + const isBeforeStop = + locationLine < stopStackFrame.lineNumber || + (locationLine === stopStackFrame.lineNumber && + locationColumn <= stopStackFrame.columnNumber); + + if (isAfterStart && isBeforeStop) { + return true; + } + } + } + + return false; +} + class FlamechartStackLayerView extends View { /** Layer to display */ _stackLayer: FlamechartStackLayer; @@ -76,6 +132,8 @@ class FlamechartStackLayerView extends View { /** A set of `stackLayer`'s frames, for efficient lookup. */ _stackFrameSet: Set; + _internalModuleSourceToRanges: InternalModuleSourceToRanges; + _intrinsicSize: Size; _hoveredStackFrame: FlamechartStackFrame | null = null; @@ -85,11 +143,13 @@ class FlamechartStackLayerView extends View { surface: Surface, frame: Rect, stackLayer: FlamechartStackLayer, + internalModuleSourceToRanges: InternalModuleSourceToRanges, duration: number, ) { super(surface, frame); this._stackLayer = stackLayer; this._stackFrameSet = new Set(stackLayer); + this._internalModuleSourceToRanges = internalModuleSourceToRanges; this._intrinsicSize = { width: duration, height: FLAMECHART_FRAME_HEIGHT, @@ -160,9 +220,19 @@ class FlamechartStackLayerView extends View { } const showHoverHighlight = _hoveredStackFrame === _stackLayer[i]; - context.fillStyle = showHoverHighlight - ? hoverColorForStackFrame(stackFrame) - : defaultColorForStackFrame(stackFrame); + + let textFillStyle; + if (isInternalModule(this._internalModuleSourceToRanges, stackFrame)) { + context.fillStyle = showHoverHighlight + ? COLORS.INTERNAL_MODULE_FRAME_HOVER + : COLORS.INTERNAL_MODULE_FRAME; + textFillStyle = COLORS.INTERNAL_MODULE_FRAME_TEXT; + } else { + context.fillStyle = showHoverHighlight + ? hoverColorForStackFrame(stackFrame) + : defaultColorForStackFrame(stackFrame); + textFillStyle = COLORS.TEXT_COLOR; + } const drawableRect = intersectionOfRects(nodeRect, visibleArea); context.fillRect( @@ -172,7 +242,9 @@ class FlamechartStackLayerView extends View { drawableRect.size.height, ); - drawText(name, context, nodeRect, drawableRect); + drawText(name, context, nodeRect, drawableRect, { + fillStyle: textFillStyle, + }); } // Render bottom border. @@ -264,13 +336,22 @@ export class FlamechartView extends View { surface: Surface, frame: Rect, flamechart: Flamechart, + internalModuleSourceToRanges: InternalModuleSourceToRanges, duration: number, ) { super(surface, frame, layeredLayout); - this.setDataAndUpdateSubviews(flamechart, duration); + this.setDataAndUpdateSubviews( + flamechart, + internalModuleSourceToRanges, + duration, + ); } - setDataAndUpdateSubviews(flamechart: Flamechart, duration: number) { + setDataAndUpdateSubviews( + flamechart: Flamechart, + internalModuleSourceToRanges: InternalModuleSourceToRanges, + duration: number, + ) { const {surface, frame, _onHover, _hoveredStackFrame} = this; // Clear existing rows on data update @@ -285,6 +366,7 @@ export class FlamechartView extends View { surface, frame, stackLayer, + internalModuleSourceToRanges, duration, ); this._verticalStackView.addSubview(rowView); diff --git a/packages/react-devtools-scheduling-profiler/src/content-views/constants.js b/packages/react-devtools-scheduling-profiler/src/content-views/constants.js index a715945f77e92..4d896d1967e96 100644 --- a/packages/react-devtools-scheduling-profiler/src/content-views/constants.js +++ b/packages/react-devtools-scheduling-profiler/src/content-views/constants.js @@ -45,6 +45,9 @@ export const MIN_INTERVAL_SIZE_PX = 70; // TODO Replace this with "export let" vars export let COLORS = { BACKGROUND: '', + INTERNAL_MODULE_FRAME: '', + INTERNAL_MODULE_FRAME_HOVER: '', + INTERNAL_MODULE_FRAME_TEXT: '', NATIVE_EVENT: '', NATIVE_EVENT_HOVER: '', NETWORK_PRIMARY: '', @@ -107,6 +110,15 @@ export function updateColorsToMatchTheme(element: Element): boolean { COLORS = { BACKGROUND: computedStyle.getPropertyValue('--color-background'), + INTERNAL_MODULE_FRAME: computedStyle.getPropertyValue( + '--color-scheduling-profiler-internal-module', + ), + INTERNAL_MODULE_FRAME_HOVER: computedStyle.getPropertyValue( + '--color-scheduling-profiler-internal-module-hover', + ), + INTERNAL_MODULE_FRAME_TEXT: computedStyle.getPropertyValue( + '--color-scheduling-profiler-internal-module-text', + ), NATIVE_EVENT: computedStyle.getPropertyValue( '--color-scheduling-profiler-native-event', ), diff --git a/packages/react-devtools-scheduling-profiler/src/import-worker/__tests__/preprocessData-test.internal.js b/packages/react-devtools-scheduling-profiler/src/import-worker/__tests__/preprocessData-test.internal.js index e458f8060f84b..f8690dc57447c 100644 --- a/packages/react-devtools-scheduling-profiler/src/import-worker/__tests__/preprocessData-test.internal.js +++ b/packages/react-devtools-scheduling-profiler/src/import-worker/__tests__/preprocessData-test.internal.js @@ -282,6 +282,7 @@ describe('preprocessData', () => { "componentMeasures": Array [], "duration": 0.005, "flamechart": Array [], + "internalModuleSourceToRanges": Map {}, "laneToLabelMap": Map { 0 => "Sync", 1 => "InputContinuousHydration", @@ -449,6 +450,7 @@ describe('preprocessData', () => { "componentMeasures": Array [], "duration": 0.011, "flamechart": Array [], + "internalModuleSourceToRanges": Map {}, "laneToLabelMap": Map { 0 => "Sync", 1 => "InputContinuousHydration", @@ -636,6 +638,7 @@ describe('preprocessData', () => { "componentMeasures": Array [], "duration": 0.013, "flamechart": Array [], + "internalModuleSourceToRanges": Map {}, "laneToLabelMap": Map { 0 => "Sync", 1 => "InputContinuousHydration", @@ -914,6 +917,7 @@ describe('preprocessData', () => { ], "duration": 0.031, "flamechart": Array [], + "internalModuleSourceToRanges": Map {}, "laneToLabelMap": Map { 0 => "Sync", 1 => "InputContinuousHydration", diff --git a/packages/react-devtools-scheduling-profiler/src/import-worker/preprocessData.js b/packages/react-devtools-scheduling-profiler/src/import-worker/preprocessData.js index aff78142b81a6..6c41de1e3e279 100644 --- a/packages/react-devtools-scheduling-profiler/src/import-worker/preprocessData.js +++ b/packages/react-devtools-scheduling-profiler/src/import-worker/preprocessData.js @@ -13,6 +13,7 @@ import { } from '@elg/speedscope'; import type {TimelineEvent} from '@elg/speedscope'; import type { + ErrorStackFrame, BatchUID, Flamechart, Milliseconds, @@ -30,6 +31,7 @@ import type { import {REACT_TOTAL_NUM_LANES, SCHEDULING_PROFILER_VERSION} from '../constants'; import InvalidProfileError from './InvalidProfileError'; import {getBatchRange} from '../utils/getBatchRange'; +import ErrorStackParser from 'error-stack-parser'; type MeasureStackElement = {| type: ReactMeasureType, @@ -43,6 +45,8 @@ type ProcessorState = {| asyncProcessingPromises: Promise[], batchUID: BatchUID, currentReactComponentMeasure: ReactComponentMeasure | null, + internalModuleCurrentStackFrame: ErrorStackFrame | null, + internalModuleStackStringSet: Set, measureStack: MeasureStackElement[], nativeEventStack: NativeEvent[], nextRenderShouldGenerateNewBatchID: boolean, @@ -793,6 +797,49 @@ function processTimelineEvent( ); } // eslint-disable-line brace-style + // Internal module ranges + else if (name.startsWith('--react-internal-module-start-')) { + const stackFrameStart = name.substr(30); + + if (!state.internalModuleStackStringSet.has(stackFrameStart)) { + state.internalModuleStackStringSet.add(stackFrameStart); + + const parsedStackFrameStart = parseStackFrame(stackFrameStart); + + state.internalModuleCurrentStackFrame = parsedStackFrameStart; + } + } else if (name.startsWith('--react-internal-module-stop-')) { + const stackFrameStop = name.substr(19); + + if (!state.internalModuleStackStringSet.has(stackFrameStop)) { + state.internalModuleStackStringSet.add(stackFrameStop); + + const parsedStackFrameStop = parseStackFrame(stackFrameStop); + + if ( + parsedStackFrameStop !== null && + state.internalModuleCurrentStackFrame !== null + ) { + const parsedStackFrameStart = state.internalModuleCurrentStackFrame; + + state.internalModuleCurrentStackFrame = null; + + const range = [parsedStackFrameStart, parsedStackFrameStop]; + const ranges = currentProfilerData.internalModuleSourceToRanges.get( + parsedStackFrameStart.fileName, + ); + if (ranges == null) { + currentProfilerData.internalModuleSourceToRanges.set( + parsedStackFrameStart.fileName, + [range], + ); + } else { + ranges.push(range); + } + } + } + } // eslint-disable-line brace-style + // Other user timing marks/measures else if (ph === 'R' || ph === 'n') { // User Timing mark @@ -855,6 +902,15 @@ function preprocessFlamechart(rawData: TimelineEvent[]): Flamechart { return flamechart; } +function parseStackFrame(stackFrame: string): ErrorStackFrame | null { + const error = new Error(); + error.stack = stackFrame; + + const frames = ErrorStackParser.parse(error); + + return frames.length === 1 ? frames[0] : null; +} + export default async function preprocessData( timeline: TimelineEvent[], ): Promise { @@ -870,6 +926,7 @@ export default async function preprocessData( componentMeasures: [], duration: 0, flamechart, + internalModuleSourceToRanges: new Map(), laneToLabelMap: new Map(), laneToReactMeasureMap, nativeEvents: [], @@ -913,6 +970,8 @@ export default async function preprocessData( asyncProcessingPromises: [], batchUID: 0, currentReactComponentMeasure: null, + internalModuleCurrentStackFrame: null, + internalModuleStackStringSet: new Set(), measureStack: [], nativeEventStack: [], nextRenderShouldGenerateNewBatchID: true, diff --git a/packages/react-devtools-scheduling-profiler/src/types.js b/packages/react-devtools-scheduling-profiler/src/types.js index 4bfafe9a2eccc..e5b14e7897ace 100644 --- a/packages/react-devtools-scheduling-profiler/src/types.js +++ b/packages/react-devtools-scheduling-profiler/src/types.js @@ -17,6 +17,12 @@ export type Return = Return_<*, T>; // Project types +export type ErrorStackFrame = { + fileName: string, + lineNumber: number, + columnNumber: number, +}; + export type Milliseconds = number; export type ReactLane = number; @@ -169,11 +175,17 @@ export type ViewState = {| viewToMutableViewStateMap: Map, |}; +export type InternalModuleSourceToRanges = Map< + string, + Array<[ErrorStackFrame, ErrorStackFrame]>, +>; + export type ReactProfilerData = {| batchUIDToMeasuresMap: Map, componentMeasures: ReactComponentMeasure[], duration: number, flamechart: Flamechart, + internalModuleSourceToRanges: InternalModuleSourceToRanges, laneToLabelMap: Map, laneToReactMeasureMap: Map, nativeEvents: NativeEvent[], diff --git a/packages/react-devtools-shared/src/backend/types.js b/packages/react-devtools-shared/src/backend/types.js index 3f623ea656e48..4f92f6b5b012e 100644 --- a/packages/react-devtools-shared/src/backend/types.js +++ b/packages/react-devtools-shared/src/backend/types.js @@ -420,6 +420,11 @@ export type DevToolsHook = { didError?: boolean, ) => void, + // Scheduling Profiler internal module filtering + getInternalModuleRanges: () => Array<[Error, Error]>, + registerInternalModuleStart: (moduleStartError: Error) => void, + registerInternalModuleStop: (moduleStopError: Error) => void, + // Testing dangerous_setTargetConsoleForTesting?: (fakeConsole: Object) => void, ... diff --git a/packages/react-devtools-shared/src/constants.js b/packages/react-devtools-shared/src/constants.js index 40c28a0f6abdf..3c3aaae2fc461 100644 --- a/packages/react-devtools-shared/src/constants.js +++ b/packages/react-devtools-shared/src/constants.js @@ -7,6 +7,10 @@ * @flow */ +export const CHROME_WEBSTORE_EXTENSION_ID = 'fmkadmapgofadopljbjfkapdkoienihi'; +export const INTERNAL_EXTENSION_ID = 'dnjnjgbfilfphmojnmhliehogmojhclc'; +export const LOCAL_EXTENSION_ID = 'ikiahnapldjmdmpkmfhjdjilojjhgcbf'; + // Flip this flag to true to enable verbose console debug logging. export const __DEBUG__ = false; @@ -147,6 +151,9 @@ export const THEME_STYLES: {[style: Theme | DisplayDensity]: any} = { '--color-resize-bar-active': '#dcdcdc', '--color-resize-bar-border': '#d1d1d1', '--color-resize-bar-dot': '#333333', + '--color-scheduling-profiler-internal-module': '#d1d1d1', + '--color-scheduling-profiler-internal-module-hover': '#c9c9c9', + '--color-scheduling-profiler-internal-module-text': '#444', '--color-scheduling-profiler-native-event': '#ccc', '--color-scheduling-profiler-native-event-hover': '#aaa', '--color-scheduling-profiler-network-primary': '#fcf3dc', @@ -288,6 +295,9 @@ export const THEME_STYLES: {[style: Theme | DisplayDensity]: any} = { '--color-resize-bar-active': '#31363f', '--color-resize-bar-border': '#3d424a', '--color-resize-bar-dot': '#cfd1d5', + '--color-scheduling-profiler-internal-module': '#303542', + '--color-scheduling-profiler-internal-module-hover': '#363b4a', + '--color-scheduling-profiler-internal-module-text': '#7f8899', '--color-scheduling-profiler-native-event': '#b2b2b2', '--color-scheduling-profiler-native-event-hover': '#949494', '--color-scheduling-profiler-network-primary': '#fcf3dc', diff --git a/packages/react-devtools-shared/src/hook.js b/packages/react-devtools-shared/src/hook.js index 6f34f86132ab6..94ebe42a06046 100644 --- a/packages/react-devtools-shared/src/hook.js +++ b/packages/react-devtools-shared/src/hook.js @@ -490,6 +490,24 @@ export function installHook(target: any): DevToolsHook | null { } } + const openModuleRangesStack: Array = []; + const moduleRanges: Array<[Error, Error]> = []; + + function getInternalModuleRanges(): Array<[Error, Error]> { + return moduleRanges; + } + + function registerInternalModuleStart(moduleStartError: Error) { + openModuleRangesStack.push(moduleStartError); + } + + function registerInternalModuleStop(moduleStopError: Error) { + const moduleStartError = openModuleRangesStack.pop(); + if (moduleStartError) { + moduleRanges.push([moduleStartError, moduleStopError]); + } + } + // TODO: More meaningful names for "rendererInterfaces" and "renderers". const fiberRoots = {}; const rendererInterfaces = new Map(); @@ -520,6 +538,13 @@ export function installHook(target: any): DevToolsHook | null { onCommitFiberRoot, onPostCommitFiberRoot, setStrictMode, + + // Schedule Profiler runtime helpers. + // These internal React modules to report their own boundaries + // which in turn enables the profiler to dim or filter internal frames. + getInternalModuleRanges, + registerInternalModuleStart, + registerInternalModuleStop, }; if (__TEST__) { diff --git a/packages/react-reconciler/src/SchedulingProfiler.js b/packages/react-reconciler/src/SchedulingProfiler.js index 03148d499282d..6afaa8bab8ef6 100644 --- a/packages/react-reconciler/src/SchedulingProfiler.js +++ b/packages/react-reconciler/src/SchedulingProfiler.js @@ -98,6 +98,29 @@ function markVersionMetadata() { markAndClear(`--profiler-version-${SCHEDULING_PROFILER_VERSION}`); } +function markInternalModuleRanges() { + /* global __REACT_DEVTOOLS_GLOBAL_HOOK__ */ + if ( + typeof __REACT_DEVTOOLS_GLOBAL_HOOK__ !== 'undefined' && + typeof __REACT_DEVTOOLS_GLOBAL_HOOK__.getInternalModuleRanges === 'function' + ) { + const ranges = __REACT_DEVTOOLS_GLOBAL_HOOK__.getInternalModuleRanges(); + for (let i = 0; i < ranges.length; i++) { + const [startError, stopError] = ranges[i]; + + // Don't embed Error stack parsing logic into the reconciler. + // Just serialize the top stack frame and let the profiler parse it. + const startFrames = startError.stack.split('\n'); + const startFrame = startFrames.length > 1 ? startFrames[1] : ''; + const stopFrames = stopError.stack.split('\n'); + const stopFrame = stopFrames.length > 1 ? stopFrames[1] : ''; + + markAndClear(`--react-internal-module-start-${startFrame}`); + markAndClear(`--react-internal-module-stop-${stopFrame}`); + } + } +} + export function markCommitStarted(lanes: Lanes): void { if (enableSchedulingProfiler) { if (supportsUserTimingV3) { @@ -114,6 +137,7 @@ export function markCommitStarted(lanes: Lanes): void { // we can log this data only once (when started) and remove the per-commit logging. markVersionMetadata(); markLaneToLabelMetadata(); + markInternalModuleRanges(); } } } diff --git a/packages/shared/registerInternalModuleStart.js b/packages/shared/registerInternalModuleStart.js new file mode 100644 index 0000000000000..aa1154fe9f1bf --- /dev/null +++ b/packages/shared/registerInternalModuleStart.js @@ -0,0 +1,20 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + */ + +/* global __REACT_DEVTOOLS_GLOBAL_HOOK__ */ + +// Don't require this file directly; it's embedded by Rollup during build. + +if ( + typeof __REACT_DEVTOOLS_GLOBAL_HOOK__ !== 'undefined' && + typeof __REACT_DEVTOOLS_GLOBAL_HOOK__.registerInternalModuleStart === + 'function' +) { + __REACT_DEVTOOLS_GLOBAL_HOOK__.registerInternalModuleStart(new Error()); +} diff --git a/packages/shared/registerInternalModuleStop.js b/packages/shared/registerInternalModuleStop.js new file mode 100644 index 0000000000000..dab139f2557f9 --- /dev/null +++ b/packages/shared/registerInternalModuleStop.js @@ -0,0 +1,20 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + */ + +/* global __REACT_DEVTOOLS_GLOBAL_HOOK__ */ + +// Don't require this file directly; it's embedded by Rollup during build. + +if ( + typeof __REACT_DEVTOOLS_GLOBAL_HOOK__ !== 'undefined' && + typeof __REACT_DEVTOOLS_GLOBAL_HOOK__.registerInternalModuleStop === + 'function' +) { + __REACT_DEVTOOLS_GLOBAL_HOOK__.registerInternalModuleStop(new Error()); +} diff --git a/scripts/rollup/wrappers.js b/scripts/rollup/wrappers.js index 836929dbec87f..dac287bab6dd9 100644 --- a/scripts/rollup/wrappers.js +++ b/scripts/rollup/wrappers.js @@ -1,5 +1,7 @@ 'use strict'; +const {resolve} = require('path'); +const {readFileSync} = require('fs'); const {bundleTypes, moduleTypes} = require('./bundles'); const reactVersion = require('../../package.json').version; @@ -25,6 +27,26 @@ const { const {RECONCILER} = moduleTypes; +function registerInternalModuleStart(globalName) { + const path = resolve( + __dirname, + '..', + '..', + 'packages/shared/registerInternalModuleStart.js' + ); + return String(readFileSync(path)).trim(); +} + +function registerInternalModuleStop(globalName) { + const path = resolve( + __dirname, + '..', + '..', + 'packages/shared/registerInternalModuleStop.js' + ); + return String(readFileSync(path)).trim(); +} + const license = ` * Copyright (c) Facebook, Inc. and its affiliates. * * This source code is licensed under the MIT license found in the @@ -320,6 +342,25 @@ ${source} }; function wrapBundle(source, bundleType, globalName, filename, moduleType) { + switch (bundleType) { + case NODE_DEV: + case NODE_PROFILING: + case FB_WWW_DEV: + case FB_WWW_PROFILING: + case RN_OSS_DEV: + case RN_OSS_PROFILING: + case RN_FB_DEV: + case RN_FB_PROFILING: + // Profiling builds should self-register internal module boundaries with DevTools. + // This allows the Scheduling Profiler to de-emphasize (dim) internal stack frames. + source = ` + ${registerInternalModuleStart(globalName)} + ${source} + ${registerInternalModuleStop(globalName)} + `; + break; + } + if (moduleType === RECONCILER) { // Standalone reconciler is only used by third-party renderers. // It is handled separately. From a6f6461b874b28da4e3a5b1b4a033f9537b1f5e2 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Tue, 19 Oct 2021 23:30:18 -0400 Subject: [PATCH 2/3] Added new Rollup bundle config option: wrapWithModuleBoundaries This flag identifies certain modules (e.g. react, react-dom, scheduler) that should self-report their module boundaries to React DevTools for Scheduling Profiler purposes. Note that this flag only applies to DEV and PROFILE bundle types (not PRODUCTION). Also note that this change does not affect modules with wrapWithModuleBoundaries:false (e.g. server renderering, test renderer, jsx transform). --- scripts/rollup/build.js | 3 +- scripts/rollup/bundles.js | 62 ++++++++++++++++++++++++++++++++++++++ scripts/rollup/wrappers.js | 45 ++++++++++++++++----------- 3 files changed, 91 insertions(+), 19 deletions(-) diff --git a/scripts/rollup/build.js b/scripts/rollup/build.js index 4f7d26cddbec9..7e29c0e8c54d3 100644 --- a/scripts/rollup/build.js +++ b/scripts/rollup/build.js @@ -440,7 +440,8 @@ function getPlugins( bundleType, globalName, filename, - moduleType + moduleType, + bundle.wrapWithModuleBoundaries ); }, }, diff --git a/scripts/rollup/bundles.js b/scripts/rollup/bundles.js index c24281d43ef08..af7b30b7feb99 100644 --- a/scripts/rollup/bundles.js +++ b/scripts/rollup/bundles.js @@ -80,6 +80,7 @@ const bundles = [ entry: 'react', global: 'React', minifyWithProdErrorCodes: false, + wrapWithModuleBoundaries: true, externals: ['ReactNativeInternalFeatureFlags'], }, @@ -90,6 +91,7 @@ const bundles = [ entry: 'react/unstable-shared-subset', global: 'React', minifyWithProdErrorCodes: true, + wrapWithModuleBoundaries: false, externals: [], }, @@ -108,6 +110,7 @@ const bundles = [ entry: 'react/jsx-runtime', global: 'JSXRuntime', minifyWithProdErrorCodes: true, + wrapWithModuleBoundaries: false, externals: ['react', 'ReactNativeInternalFeatureFlags'], }, @@ -128,6 +131,7 @@ const bundles = [ entry: 'react/jsx-dev-runtime', global: 'JSXDEVRuntime', minifyWithProdErrorCodes: false, + wrapWithModuleBoundaries: false, externals: ['react', 'ReactNativeInternalFeatureFlags'], }, @@ -138,6 +142,7 @@ const bundles = [ entry: 'react-fetch/index.browser', global: 'ReactFetch', minifyWithProdErrorCodes: true, + wrapWithModuleBoundaries: false, externals: ['react'], }, @@ -148,6 +153,7 @@ const bundles = [ entry: 'react-fetch/index.node', global: 'ReactFetch', minifyWithProdErrorCodes: false, + wrapWithModuleBoundaries: false, externals: ['react', 'http', 'https'], }, @@ -158,6 +164,7 @@ const bundles = [ entry: 'react-fs/index.browser.server', global: 'ReactFilesystem', minifyWithProdErrorCodes: true, + wrapWithModuleBoundaries: false, externals: [], }, @@ -168,6 +175,7 @@ const bundles = [ entry: 'react-fs/index.node.server', global: 'ReactFilesystem', minifyWithProdErrorCodes: false, + wrapWithModuleBoundaries: false, externals: ['react', 'fs/promises', 'path'], }, @@ -178,6 +186,7 @@ const bundles = [ entry: 'react-pg/index.browser.server', global: 'ReactPostgres', minifyWithProdErrorCodes: true, + wrapWithModuleBoundaries: false, externals: [], }, @@ -188,6 +197,7 @@ const bundles = [ entry: 'react-pg/index.node.server', global: 'ReactPostgres', minifyWithProdErrorCodes: false, + wrapWithModuleBoundaries: false, externals: ['react', 'pg'], }, @@ -208,6 +218,7 @@ const bundles = [ entry: 'react-dom', global: 'ReactDOM', minifyWithProdErrorCodes: true, + wrapWithModuleBoundaries: true, externals: ['react'], }, @@ -219,6 +230,7 @@ const bundles = [ global: 'ReactDOMForked', enableNewReconciler: true, minifyWithProdErrorCodes: true, + wrapWithModuleBoundaries: true, externals: ['react'], }, @@ -229,6 +241,7 @@ const bundles = [ entry: 'react-dom/test-utils', global: 'ReactTestUtils', minifyWithProdErrorCodes: false, + wrapWithModuleBoundaries: false, externals: ['react', 'react-dom'], }, @@ -240,6 +253,7 @@ const bundles = [ entry: 'react-dom/testing', global: 'ReactDOMTesting', minifyWithProdErrorCodes: true, + wrapWithModuleBoundaries: false, externals: ['react'], }, @@ -253,6 +267,7 @@ const bundles = [ name: 'react-dom-server-legacy.browser', global: 'ReactDOMServer', minifyWithProdErrorCodes: true, + wrapWithModuleBoundaries: false, externals: ['react'], babel: opts => Object.assign({}, opts, { @@ -268,6 +283,7 @@ const bundles = [ name: 'react-dom-server-legacy.node', externals: ['react', 'stream'], minifyWithProdErrorCodes: false, + wrapWithModuleBoundaries: false, babel: opts => Object.assign({}, opts, { plugins: opts.plugins.concat([ @@ -284,6 +300,7 @@ const bundles = [ name: 'react-dom-server.browser', global: 'ReactDOMServer', minifyWithProdErrorCodes: true, + wrapWithModuleBoundaries: false, externals: ['react'], }, { @@ -293,6 +310,7 @@ const bundles = [ name: 'react-dom-server.node', global: 'ReactDOMServer', minifyWithProdErrorCodes: false, + wrapWithModuleBoundaries: false, externals: ['react'], }, { @@ -301,6 +319,7 @@ const bundles = [ entry: 'react-server-dom-relay/src/ReactDOMServerFB', global: 'ReactDOMServer', minifyWithProdErrorCodes: false, + wrapWithModuleBoundaries: false, externals: ['react'], }, @@ -311,6 +330,7 @@ const bundles = [ entry: 'react-server-dom-webpack/writer.browser.server', global: 'ReactServerDOMWriter', minifyWithProdErrorCodes: false, + wrapWithModuleBoundaries: false, externals: ['react'], }, { @@ -319,6 +339,7 @@ const bundles = [ entry: 'react-server-dom-webpack/writer.node.server', global: 'ReactServerDOMWriter', minifyWithProdErrorCodes: false, + wrapWithModuleBoundaries: false, externals: ['react'], }, @@ -329,6 +350,7 @@ const bundles = [ entry: 'react-server-dom-webpack', global: 'ReactServerDOMReader', minifyWithProdErrorCodes: true, + wrapWithModuleBoundaries: false, externals: ['react'], }, @@ -339,6 +361,7 @@ const bundles = [ entry: 'react-server-dom-webpack/plugin', global: 'ReactServerWebpackPlugin', minifyWithProdErrorCodes: false, + wrapWithModuleBoundaries: false, externals: ['fs', 'path', 'url', 'neo-async'], }, @@ -349,6 +372,7 @@ const bundles = [ entry: 'react-server-dom-webpack/node-loader', global: 'ReactServerWebpackNodeLoader', minifyWithProdErrorCodes: false, + wrapWithModuleBoundaries: false, externals: ['acorn'], }, @@ -359,6 +383,7 @@ const bundles = [ entry: 'react-server-dom-webpack/node-register', global: 'ReactFlightWebpackNodeRegister', minifyWithProdErrorCodes: false, + wrapWithModuleBoundaries: false, externals: ['url', 'module'], }, @@ -369,6 +394,7 @@ const bundles = [ entry: 'react-server-dom-relay/server', global: 'ReactFlightDOMRelayServer', // TODO: Rename to Writer minifyWithProdErrorCodes: false, + wrapWithModuleBoundaries: false, externals: [ 'react', 'ReactFlightDOMRelayServerIntegration', @@ -383,6 +409,7 @@ const bundles = [ entry: 'react-server-dom-relay', global: 'ReactFlightDOMRelayClient', // TODO: Rename to Reader minifyWithProdErrorCodes: true, + wrapWithModuleBoundaries: false, externals: [ 'react', 'ReactFlightDOMRelayClientIntegration', @@ -397,6 +424,7 @@ const bundles = [ entry: 'react-server-native-relay/server', global: 'ReactFlightNativeRelayServer', // TODO: Rename to Writer minifyWithProdErrorCodes: false, + wrapWithModuleBoundaries: false, externals: [ 'react', 'ReactFlightNativeRelayServerIntegration', @@ -412,6 +440,7 @@ const bundles = [ entry: 'react-server-native-relay', global: 'ReactFlightNativeRelayClient', // TODO: Rename to Reader minifyWithProdErrorCodes: true, + wrapWithModuleBoundaries: false, externals: [ 'react', 'ReactFlightNativeRelayClientIntegration', @@ -427,6 +456,7 @@ const bundles = [ entry: 'react-suspense-test-utils', global: 'ReactSuspenseTestUtils', minifyWithProdErrorCodes: false, + wrapWithModuleBoundaries: false, externals: ['react'], }, @@ -445,6 +475,7 @@ const bundles = [ global: 'ReactART', externals: ['react'], minifyWithProdErrorCodes: true, + wrapWithModuleBoundaries: true, babel: opts => Object.assign({}, opts, { // Include JSX @@ -468,6 +499,7 @@ const bundles = [ global: 'ReactNativeRenderer', externals: ['react-native', 'ReactNativeInternalFeatureFlags'], minifyWithProdErrorCodes: false, + wrapWithModuleBoundaries: true, babel: opts => Object.assign({}, opts, { plugins: opts.plugins.concat([ @@ -482,6 +514,7 @@ const bundles = [ global: 'ReactNativeRenderer', externals: ['react-native'], minifyWithProdErrorCodes: false, + wrapWithModuleBoundaries: true, babel: opts => Object.assign({}, opts, { plugins: opts.plugins.concat([ @@ -500,6 +533,7 @@ const bundles = [ global: 'ReactFabric', externals: ['react-native', 'ReactNativeInternalFeatureFlags'], minifyWithProdErrorCodes: false, + wrapWithModuleBoundaries: true, babel: opts => Object.assign({}, opts, { plugins: opts.plugins.concat([ @@ -514,6 +548,7 @@ const bundles = [ global: 'ReactFabric', externals: ['react-native'], minifyWithProdErrorCodes: false, + wrapWithModuleBoundaries: true, babel: opts => Object.assign({}, opts, { plugins: opts.plugins.concat([ @@ -544,6 +579,7 @@ const bundles = [ 'ReactNativeInternalFeatureFlags', ], minifyWithProdErrorCodes: false, + wrapWithModuleBoundaries: false, babel: opts => Object.assign({}, opts, { plugins: opts.plugins.concat([ @@ -559,6 +595,7 @@ const bundles = [ entry: 'react-noop-renderer', global: 'ReactNoopRenderer', minifyWithProdErrorCodes: true, + wrapWithModuleBoundaries: false, externals: ['react', 'scheduler', 'scheduler/unstable_mock', 'expect'], }, @@ -569,6 +606,7 @@ const bundles = [ entry: 'react-noop-renderer/persistent', global: 'ReactNoopRendererPersistent', minifyWithProdErrorCodes: true, + wrapWithModuleBoundaries: false, externals: ['react', 'scheduler', 'expect'], }, @@ -579,6 +617,7 @@ const bundles = [ entry: 'react-noop-renderer/server', global: 'ReactNoopRendererServer', minifyWithProdErrorCodes: true, + wrapWithModuleBoundaries: false, externals: ['react', 'scheduler', 'expect'], }, @@ -589,6 +628,7 @@ const bundles = [ entry: 'react-noop-renderer/flight-server', global: 'ReactNoopFlightServer', minifyWithProdErrorCodes: false, + wrapWithModuleBoundaries: false, externals: [ 'react', 'scheduler', @@ -604,6 +644,7 @@ const bundles = [ entry: 'react-noop-renderer/flight-client', global: 'ReactNoopFlightClient', minifyWithProdErrorCodes: false, + wrapWithModuleBoundaries: false, externals: [ 'react', 'scheduler', @@ -619,6 +660,7 @@ const bundles = [ entry: 'react-reconciler', global: 'ReactReconciler', minifyWithProdErrorCodes: true, + wrapWithModuleBoundaries: false, externals: ['react'], }, @@ -629,6 +671,7 @@ const bundles = [ entry: 'react-server', global: 'ReactServer', minifyWithProdErrorCodes: false, + wrapWithModuleBoundaries: false, externals: ['react'], }, @@ -639,6 +682,7 @@ const bundles = [ entry: 'react-server/flight', global: 'ReactFlightServer', minifyWithProdErrorCodes: false, + wrapWithModuleBoundaries: false, externals: ['react'], }, @@ -649,6 +693,7 @@ const bundles = [ entry: 'react-client/flight', global: 'ReactFlightClient', minifyWithProdErrorCodes: true, + wrapWithModuleBoundaries: false, externals: ['react'], }, @@ -659,6 +704,7 @@ const bundles = [ entry: 'react-reconciler/reflection', global: 'ReactFiberTreeReflection', minifyWithProdErrorCodes: true, + wrapWithModuleBoundaries: false, externals: [], }, @@ -669,6 +715,7 @@ const bundles = [ entry: 'react-reconciler/constants', global: 'ReactReconcilerConstants', minifyWithProdErrorCodes: true, + wrapWithModuleBoundaries: false, externals: [], }, @@ -686,6 +733,7 @@ const bundles = [ entry: 'react-is', global: 'ReactIs', minifyWithProdErrorCodes: true, + wrapWithModuleBoundaries: false, externals: [], }, @@ -696,6 +744,7 @@ const bundles = [ entry: 'react-debug-tools', global: 'ReactDebugTools', minifyWithProdErrorCodes: false, + wrapWithModuleBoundaries: false, externals: [], }, @@ -708,6 +757,7 @@ const bundles = [ entry: 'react-cache', global: 'ReactCacheOld', minifyWithProdErrorCodes: false, + wrapWithModuleBoundaries: false, externals: ['react', 'scheduler'], }, @@ -719,6 +769,7 @@ const bundles = [ global: 'createSubscription', externals: ['react'], minifyWithProdErrorCodes: true, + wrapWithModuleBoundaries: true, babel: opts => Object.assign({}, opts, { plugins: opts.plugins.concat([ @@ -734,6 +785,7 @@ const bundles = [ entry: 'use-subscription', global: 'useSubscription', minifyWithProdErrorCodes: true, + wrapWithModuleBoundaries: true, externals: ['react'], }, @@ -744,6 +796,7 @@ const bundles = [ entry: 'use-sync-external-store', global: 'useSyncExternalStore', minifyWithProdErrorCodes: true, + wrapWithModuleBoundaries: true, externals: ['react'], }, @@ -754,6 +807,7 @@ const bundles = [ entry: 'use-sync-external-store/extra', global: 'useSyncExternalStoreExtra', minifyWithProdErrorCodes: true, + wrapWithModuleBoundaries: true, externals: ['react', 'use-sync-external-store'], }, @@ -764,6 +818,7 @@ const bundles = [ entry: 'use-sync-external-store/index.native', global: 'useSyncExternalStoreNative', minifyWithProdErrorCodes: true, + wrapWithModuleBoundaries: true, externals: ['react', 'ReactNativeInternalFeatureFlags'], }, @@ -783,6 +838,7 @@ const bundles = [ entry: 'scheduler', global: 'Scheduler', minifyWithProdErrorCodes: true, + wrapWithModuleBoundaries: true, externals: ['ReactNativeInternalFeatureFlags'], }, @@ -802,6 +858,7 @@ const bundles = [ entry: 'scheduler/unstable_mock', global: 'SchedulerMock', minifyWithProdErrorCodes: false, + wrapWithModuleBoundaries: false, externals: ['ReactNativeInternalFeatureFlags'], }, @@ -818,6 +875,7 @@ const bundles = [ entry: 'scheduler/unstable_post_task', global: 'SchedulerPostTask', minifyWithProdErrorCodes: true, + wrapWithModuleBoundaries: false, externals: [], }, @@ -828,6 +886,7 @@ const bundles = [ entry: 'jest-react', global: 'JestReact', minifyWithProdErrorCodes: false, + wrapWithModuleBoundaries: false, externals: ['react', 'scheduler', 'scheduler/unstable_mock'], }, @@ -842,6 +901,7 @@ const bundles = [ entry: 'eslint-plugin-react-hooks', global: 'ESLintPluginReactHooks', minifyWithProdErrorCodes: false, + wrapWithModuleBoundaries: false, externals: [], }, @@ -852,6 +912,7 @@ const bundles = [ entry: 'react-refresh/babel', global: 'ReactFreshBabelPlugin', minifyWithProdErrorCodes: false, + wrapWithModuleBoundaries: false, externals: [], }, { @@ -860,6 +921,7 @@ const bundles = [ entry: 'react-refresh/runtime', global: 'ReactFreshRuntime', minifyWithProdErrorCodes: false, + wrapWithModuleBoundaries: false, externals: [], }, ]; diff --git a/scripts/rollup/wrappers.js b/scripts/rollup/wrappers.js index dac287bab6dd9..c83ecc0fbdb96 100644 --- a/scripts/rollup/wrappers.js +++ b/scripts/rollup/wrappers.js @@ -341,24 +341,33 @@ ${source} }, }; -function wrapBundle(source, bundleType, globalName, filename, moduleType) { - switch (bundleType) { - case NODE_DEV: - case NODE_PROFILING: - case FB_WWW_DEV: - case FB_WWW_PROFILING: - case RN_OSS_DEV: - case RN_OSS_PROFILING: - case RN_FB_DEV: - case RN_FB_PROFILING: - // Profiling builds should self-register internal module boundaries with DevTools. - // This allows the Scheduling Profiler to de-emphasize (dim) internal stack frames. - source = ` - ${registerInternalModuleStart(globalName)} - ${source} - ${registerInternalModuleStop(globalName)} - `; - break; +function wrapBundle( + source, + bundleType, + globalName, + filename, + moduleType, + wrapWithModuleBoundaries +) { + if (wrapWithModuleBoundaries) { + switch (bundleType) { + case NODE_DEV: + case NODE_PROFILING: + case FB_WWW_DEV: + case FB_WWW_PROFILING: + case RN_OSS_DEV: + case RN_OSS_PROFILING: + case RN_FB_DEV: + case RN_FB_PROFILING: + // Certain DEV and Profiling bundles should self-register their own module boundaries with DevTools. + // This allows the Scheduling Profiler to de-emphasize (dim) internal stack frames. + source = ` + ${registerInternalModuleStart(globalName)} + ${source} + ${registerInternalModuleStop(globalName)} + `; + break; + } } if (moduleType === RECONCILER) { From c3439e9bc673ba09a0ca2610e3f0484997cadb51 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Wed, 20 Oct 2021 10:29:37 -0400 Subject: [PATCH 3/3] Refactored code so that isInternalModule function could be unit tested. --- .../src/content-views/FlamechartView.js | 56 +------------ .../utils/__tests__/__modules__/module-one.js | 16 ++++ .../utils/__tests__/__modules__/module-two.js | 18 +++++ .../utils/__tests__/moduleFilters-test.js | 79 +++++++++++++++++++ .../src/content-views/utils/moduleFilters.js | 69 ++++++++++++++++ .../src/backend/types.js | 2 +- packages/react-devtools-shared/src/hook.js | 34 +++++--- .../src/SchedulingProfiler.js | 13 +-- 8 files changed, 212 insertions(+), 75 deletions(-) create mode 100644 packages/react-devtools-scheduling-profiler/src/content-views/utils/__tests__/__modules__/module-one.js create mode 100644 packages/react-devtools-scheduling-profiler/src/content-views/utils/__tests__/__modules__/module-two.js create mode 100644 packages/react-devtools-scheduling-profiler/src/content-views/utils/__tests__/moduleFilters-test.js create mode 100644 packages/react-devtools-scheduling-profiler/src/content-views/utils/moduleFilters.js diff --git a/packages/react-devtools-scheduling-profiler/src/content-views/FlamechartView.js b/packages/react-devtools-scheduling-profiler/src/content-views/FlamechartView.js index e1fdf8f4327f4..bbc8dcf13936b 100644 --- a/packages/react-devtools-scheduling-profiler/src/content-views/FlamechartView.js +++ b/packages/react-devtools-scheduling-profiler/src/content-views/FlamechartView.js @@ -21,11 +21,6 @@ import type { ViewRefs, } from '../view-base'; -import { - CHROME_WEBSTORE_EXTENSION_ID, - INTERNAL_EXTENSION_ID, - LOCAL_EXTENSION_ID, -} from 'react-devtools-shared/src/constants'; import { BackgroundColorView, Surface, @@ -36,6 +31,7 @@ import { rectIntersectsRect, verticallyStackedLayout, } from '../view-base'; +import {isInternalModule} from './utils/moduleFilters'; import { durationToWidth, positioningScaleFactor, @@ -75,56 +71,6 @@ function hoverColorForStackFrame(stackFrame: FlamechartStackFrame): string { return hslaColorToString(color); } -function isInternalModule( - internalModuleSourceToRanges: InternalModuleSourceToRanges, - flamechartStackFrame: FlamechartStackFrame, -): boolean { - const {locationColumn, locationLine, scriptUrl} = flamechartStackFrame; - - if (scriptUrl == null || locationColumn == null || locationLine == null) { - return true; - } - - // Internal modules are only registered if DevTools was running when the profile was captured, - // but DevTools should also hide its own frames to avoid over-emphasizing them. - if ( - // Handle webpack-internal:// sources - scriptUrl.includes('/react-devtools') || - scriptUrl.includes('/react_devtools') || - // Filter out known extension IDs - scriptUrl.includes(CHROME_WEBSTORE_EXTENSION_ID) || - scriptUrl.includes(INTERNAL_EXTENSION_ID) || - scriptUrl.includes(LOCAL_EXTENSION_ID) - - // Unfortunately this won't get everything, like relatively loaded chunks or Web Worker files. - ) { - return true; - } - - // Filter out React internal packages. - const ranges = internalModuleSourceToRanges.get(scriptUrl); - if (ranges != null) { - for (let i = 0; i < ranges.length; i++) { - const [startStackFrame, stopStackFrame] = ranges[i]; - - const isAfterStart = - locationLine > startStackFrame.lineNumber || - (locationLine === startStackFrame.lineNumber && - locationColumn >= startStackFrame.columnNumber); - const isBeforeStop = - locationLine < stopStackFrame.lineNumber || - (locationLine === stopStackFrame.lineNumber && - locationColumn <= stopStackFrame.columnNumber); - - if (isAfterStart && isBeforeStop) { - return true; - } - } - } - - return false; -} - class FlamechartStackLayerView extends View { /** Layer to display */ _stackLayer: FlamechartStackLayer; diff --git a/packages/react-devtools-scheduling-profiler/src/content-views/utils/__tests__/__modules__/module-one.js b/packages/react-devtools-scheduling-profiler/src/content-views/utils/__tests__/__modules__/module-one.js new file mode 100644 index 0000000000000..6c3ace6bfbc65 --- /dev/null +++ b/packages/react-devtools-scheduling-profiler/src/content-views/utils/__tests__/__modules__/module-one.js @@ -0,0 +1,16 @@ +/** + * Copyright (c) Facebook, Inc. and its 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 const outerErrorA = new Error(); + +export const moduleStartError = new Error(); +export const innerError = new Error(); +export const moduleStopError = new Error(); + +export const outerErrorB = new Error(); diff --git a/packages/react-devtools-scheduling-profiler/src/content-views/utils/__tests__/__modules__/module-two.js b/packages/react-devtools-scheduling-profiler/src/content-views/utils/__tests__/__modules__/module-two.js new file mode 100644 index 0000000000000..994f317900555 --- /dev/null +++ b/packages/react-devtools-scheduling-profiler/src/content-views/utils/__tests__/__modules__/module-two.js @@ -0,0 +1,18 @@ +/** + * Copyright (c) Facebook, Inc. and its 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 const moduleAStartError = new Error(); +export const innerErrorA = new Error(); +export const moduleAStopError = new Error(); + +export const outerError = new Error(); + +export const moduleBStartError = new Error(); +export const innerErrorB = new Error(); +export const moduleBStopError = new Error(); diff --git a/packages/react-devtools-scheduling-profiler/src/content-views/utils/__tests__/moduleFilters-test.js b/packages/react-devtools-scheduling-profiler/src/content-views/utils/__tests__/moduleFilters-test.js new file mode 100644 index 0000000000000..d84840371daa1 --- /dev/null +++ b/packages/react-devtools-scheduling-profiler/src/content-views/utils/__tests__/moduleFilters-test.js @@ -0,0 +1,79 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + */ + +import {isInternalModule} from '../moduleFilters'; + +describe('isInternalModule', () => { + let map; + + function createFlamechartStackFrame(scriptUrl, locationLine, locationColumn) { + return { + name: 'test', + timestamp: 0, + duration: 1, + scriptUrl, + locationLine, + locationColumn, + }; + } + + function createStackFrame(fileName, lineNumber, columnNumber) { + return { + columnNumber: columnNumber, + lineNumber: lineNumber, + fileName: fileName, + functionName: 'test', + source: ` at test (${fileName}:${lineNumber}:${columnNumber})`, + }; + } + + beforeEach(() => { + map = new Map(); + map.set('foo', [ + [createStackFrame('foo', 10, 0), createStackFrame('foo', 15, 100)], + ]); + map.set('bar', [ + [createStackFrame('bar', 10, 0), createStackFrame('bar', 15, 100)], + [createStackFrame('bar', 20, 0), createStackFrame('bar', 25, 100)], + ]); + }); + + it('should properly identify stack frames within the provided module ranges', () => { + expect( + isInternalModule(map, createFlamechartStackFrame('foo', 10, 0)), + ).toBe(true); + expect( + isInternalModule(map, createFlamechartStackFrame('foo', 12, 35)), + ).toBe(true); + expect( + isInternalModule(map, createFlamechartStackFrame('foo', 15, 100)), + ).toBe(true); + expect( + isInternalModule(map, createFlamechartStackFrame('bar', 12, 0)), + ).toBe(true); + expect( + isInternalModule(map, createFlamechartStackFrame('bar', 22, 125)), + ).toBe(true); + }); + + it('should properly identify stack frames outside of the provided module ranges', () => { + expect(isInternalModule(map, createFlamechartStackFrame('foo', 9, 0))).toBe( + false, + ); + expect( + isInternalModule(map, createFlamechartStackFrame('foo', 15, 101)), + ).toBe(false); + expect( + isInternalModule(map, createFlamechartStackFrame('bar', 17, 0)), + ).toBe(false); + expect( + isInternalModule(map, createFlamechartStackFrame('baz', 12, 0)), + ).toBe(false); + }); +}); diff --git a/packages/react-devtools-scheduling-profiler/src/content-views/utils/moduleFilters.js b/packages/react-devtools-scheduling-profiler/src/content-views/utils/moduleFilters.js new file mode 100644 index 0000000000000..7030e6124b603 --- /dev/null +++ b/packages/react-devtools-scheduling-profiler/src/content-views/utils/moduleFilters.js @@ -0,0 +1,69 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + */ + +import type { + FlamechartStackFrame, + InternalModuleSourceToRanges, +} from '../../types'; + +import { + CHROME_WEBSTORE_EXTENSION_ID, + INTERNAL_EXTENSION_ID, + LOCAL_EXTENSION_ID, +} from 'react-devtools-shared/src/constants'; + +export function isInternalModule( + internalModuleSourceToRanges: InternalModuleSourceToRanges, + flamechartStackFrame: FlamechartStackFrame, +): boolean { + const {locationColumn, locationLine, scriptUrl} = flamechartStackFrame; + + if (scriptUrl == null || locationColumn == null || locationLine == null) { + // This could indicate a browser-internal API like performance.mark(). + return false; + } + + // Internal modules are only registered if DevTools was running when the profile was captured, + // but DevTools should also hide its own frames to avoid over-emphasizing them. + if ( + // Handle webpack-internal:// sources + scriptUrl.includes('/react-devtools') || + scriptUrl.includes('/react_devtools') || + // Filter out known extension IDs + scriptUrl.includes(CHROME_WEBSTORE_EXTENSION_ID) || + scriptUrl.includes(INTERNAL_EXTENSION_ID) || + scriptUrl.includes(LOCAL_EXTENSION_ID) + // Unfortunately this won't get everything, like relatively loaded chunks or Web Worker files. + ) { + return true; + } + + // Filter out React internal packages. + const ranges = internalModuleSourceToRanges.get(scriptUrl); + if (ranges != null) { + for (let i = 0; i < ranges.length; i++) { + const [startStackFrame, stopStackFrame] = ranges[i]; + + const isAfterStart = + locationLine > startStackFrame.lineNumber || + (locationLine === startStackFrame.lineNumber && + locationColumn >= startStackFrame.columnNumber); + const isBeforeStop = + locationLine < stopStackFrame.lineNumber || + (locationLine === stopStackFrame.lineNumber && + locationColumn <= stopStackFrame.columnNumber); + + if (isAfterStart && isBeforeStop) { + return true; + } + } + } + + return false; +} diff --git a/packages/react-devtools-shared/src/backend/types.js b/packages/react-devtools-shared/src/backend/types.js index 4f92f6b5b012e..46e1287797981 100644 --- a/packages/react-devtools-shared/src/backend/types.js +++ b/packages/react-devtools-shared/src/backend/types.js @@ -421,7 +421,7 @@ export type DevToolsHook = { ) => void, // Scheduling Profiler internal module filtering - getInternalModuleRanges: () => Array<[Error, Error]>, + getInternalModuleRanges: () => Array<[string, string]>, registerInternalModuleStart: (moduleStartError: Error) => void, registerInternalModuleStop: (moduleStopError: Error) => void, diff --git a/packages/react-devtools-shared/src/hook.js b/packages/react-devtools-shared/src/hook.js index 94ebe42a06046..e265212e31c42 100644 --- a/packages/react-devtools-shared/src/hook.js +++ b/packages/react-devtools-shared/src/hook.js @@ -490,21 +490,37 @@ export function installHook(target: any): DevToolsHook | null { } } - const openModuleRangesStack: Array = []; - const moduleRanges: Array<[Error, Error]> = []; + type StackFrameString = string; - function getInternalModuleRanges(): Array<[Error, Error]> { + const openModuleRangesStack: Array = []; + const moduleRanges: Array<[StackFrameString, StackFrameString]> = []; + + function getTopStackFrameString(error: Error): StackFrameString | null { + const frames = error.stack.split('\n'); + const frame = frames.length > 1 ? frames[1] : null; + return frame; + } + + function getInternalModuleRanges(): Array< + [StackFrameString, StackFrameString], + > { return moduleRanges; } - function registerInternalModuleStart(moduleStartError: Error) { - openModuleRangesStack.push(moduleStartError); + function registerInternalModuleStart(error: Error) { + const startStackFrame = getTopStackFrameString(error); + if (startStackFrame !== null) { + openModuleRangesStack.push(startStackFrame); + } } - function registerInternalModuleStop(moduleStopError: Error) { - const moduleStartError = openModuleRangesStack.pop(); - if (moduleStartError) { - moduleRanges.push([moduleStartError, moduleStopError]); + function registerInternalModuleStop(error: Error) { + if (openModuleRangesStack.length > 0) { + const startStackFrame = openModuleRangesStack.pop(); + const stopStackFrame = getTopStackFrameString(error); + if (stopStackFrame !== null) { + moduleRanges.push([startStackFrame, stopStackFrame]); + } } } diff --git a/packages/react-reconciler/src/SchedulingProfiler.js b/packages/react-reconciler/src/SchedulingProfiler.js index 6afaa8bab8ef6..4df55137fda97 100644 --- a/packages/react-reconciler/src/SchedulingProfiler.js +++ b/packages/react-reconciler/src/SchedulingProfiler.js @@ -106,17 +106,10 @@ function markInternalModuleRanges() { ) { const ranges = __REACT_DEVTOOLS_GLOBAL_HOOK__.getInternalModuleRanges(); for (let i = 0; i < ranges.length; i++) { - const [startError, stopError] = ranges[i]; + const [startStackFrame, stopStackFrame] = ranges[i]; - // Don't embed Error stack parsing logic into the reconciler. - // Just serialize the top stack frame and let the profiler parse it. - const startFrames = startError.stack.split('\n'); - const startFrame = startFrames.length > 1 ? startFrames[1] : ''; - const stopFrames = stopError.stack.split('\n'); - const stopFrame = stopFrames.length > 1 ? stopFrames[1] : ''; - - markAndClear(`--react-internal-module-start-${startFrame}`); - markAndClear(`--react-internal-module-stop-${stopFrame}`); + markAndClear(`--react-internal-module-start-${startStackFrame}`); + markAndClear(`--react-internal-module-stop-${stopStackFrame}`); } } }