Skip to content

Commit e528728

Browse files
authored
feat[devtools]: symbolicate source for inspected element (#28471)
Stacked on #28351, please review only the last commit. Top-level description of the approach: 1. Once user selects an element from the tree, frontend asks backend to return the inspected element, this is where we simulate an error happening in `render` function of the component and then we parse the error stack. As an improvement, we should probably migrate from custom implementation of error stack parser to `error-stack-parser` from npm. 2. When frontend receives the inspected element and this object is being propagated, we create a Promise for symbolicated source, which is then passed down to all components, which are using `source`. 3. These components use `use` hook for this promise and are wrapped in Suspense. Caching: 1. For browser extension, we cache Promises based on requested resource + key + column, also added use of `chrome.devtools.inspectedWindow.getResource` API. 2. For standalone case (RN), we cache based on requested resource url, we cache the content of it.
1 parent 61bd004 commit e528728

24 files changed

+748
-305
lines changed

.eslintrc.js

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -455,6 +455,13 @@ module.exports = {
455455
__IS_CHROME__: 'readonly',
456456
__IS_FIREFOX__: 'readonly',
457457
__IS_EDGE__: 'readonly',
458+
__IS_INTERNAL_VERSION__: 'readonly',
459+
},
460+
},
461+
{
462+
files: ['packages/react-devtools-shared/**/*.js'],
463+
globals: {
464+
__IS_INTERNAL_VERSION__: 'readonly',
458465
},
459466
},
460467
],

packages/react-debug-tools/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,6 @@
2828
"react": "^17.0.0"
2929
},
3030
"dependencies": {
31-
"error-stack-parser": "^2.0.2"
31+
"error-stack-parser": "^2.1.4"
3232
}
3333
}

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

Lines changed: 35 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ import {
3333
import {localStorageSetItem} from 'react-devtools-shared/src/storage';
3434

3535
import type {FrontendBridge} from 'react-devtools-shared/src/bridge';
36-
import type {InspectedElement} from 'react-devtools-shared/src/frontend/types';
36+
import type {Source} from 'react-devtools-shared/src/shared/types';
3737

3838
installHook(window);
3939

@@ -127,36 +127,55 @@ function reload() {
127127
store: ((store: any): Store),
128128
warnIfLegacyBackendDetected: true,
129129
viewElementSourceFunction,
130+
fetchFileWithCaching,
130131
}),
131132
);
132133
}, 100);
133134
}
134135

136+
const resourceCache: Map<string, string> = new Map();
137+
138+
// As a potential improvement, this should be done from the backend of RDT.
139+
// Browser extension is doing this via exchanging messages
140+
// between devtools_page and dedicated content script for it, see `fetchFileWithCaching.js`.
141+
async function fetchFileWithCaching(url: string) {
142+
if (resourceCache.has(url)) {
143+
return Promise.resolve(resourceCache.get(url));
144+
}
145+
146+
return fetch(url)
147+
.then(data => data.text())
148+
.then(content => {
149+
resourceCache.set(url, content);
150+
151+
return content;
152+
});
153+
}
154+
135155
function canViewElementSourceFunction(
136-
inspectedElement: InspectedElement,
156+
_source: Source,
157+
symbolicatedSource: Source | null,
137158
): boolean {
138-
if (
139-
inspectedElement.canViewSource === false ||
140-
inspectedElement.source === null
141-
) {
159+
if (symbolicatedSource == null) {
142160
return false;
143161
}
144162

145-
const {source} = inspectedElement;
146-
147-
return doesFilePathExist(source.sourceURL, projectRoots);
163+
return doesFilePathExist(symbolicatedSource.sourceURL, projectRoots);
148164
}
149165

150166
function viewElementSourceFunction(
151-
id: number,
152-
inspectedElement: InspectedElement,
167+
_source: Source,
168+
symbolicatedSource: Source | null,
153169
): void {
154-
const {source} = inspectedElement;
155-
if (source !== null) {
156-
launchEditor(source.sourceURL, source.line, projectRoots);
157-
} else {
158-
log.error('Cannot inspect element', id);
170+
if (symbolicatedSource == null) {
171+
return;
159172
}
173+
174+
launchEditor(
175+
symbolicatedSource.sourceURL,
176+
symbolicatedSource.line,
177+
projectRoots,
178+
);
160179
}
161180

162181
function onDisconnected() {

packages/react-devtools-extensions/src/main/fetchFileWithCaching.js

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
/* global chrome */
22

3+
import {normalizeUrl} from 'react-devtools-shared/src/utils';
34
import {__DEBUG__} from 'react-devtools-shared/src/constants';
45

56
let debugIDCounter = 0;
@@ -107,17 +108,35 @@ const fetchFromPage = async (url, resolve, reject) => {
107108
});
108109
};
109110

110-
// Fetching files from the extension won't make use of the network cache
111-
// for resources that have already been loaded by the page.
112-
// This helper function allows the extension to request files to be fetched
113-
// by the content script (running in the page) to increase the likelihood of a cache hit.
114-
const fetchFileWithCaching = url => {
111+
// 1. Check if resource is available via chrome.devtools.inspectedWindow.getResources
112+
// 2. Check if resource was loaded previously and available in network cache via chrome.devtools.network.getHAR
113+
// 3. Fallback to fetching directly from the page context (from backend)
114+
async function fetchFileWithCaching(url: string): Promise<string> {
115+
if (__IS_CHROME__ || __IS_EDGE__) {
116+
const resources = await new Promise(resolve =>
117+
chrome.devtools.inspectedWindow.getResources(r => resolve(r)),
118+
);
119+
120+
const normalizedReferenceURL = normalizeUrl(url);
121+
const resource = resources.find(r => r.url === normalizedReferenceURL);
122+
123+
if (resource != null) {
124+
const content = await new Promise(resolve =>
125+
resource.getContent(fetchedContent => resolve(fetchedContent)),
126+
);
127+
128+
if (content) {
129+
return content;
130+
}
131+
}
132+
}
133+
115134
return new Promise((resolve, reject) => {
116135
// Try fetching from the Network cache first.
117136
// If DevTools was opened after the page started loading, we may have missed some requests.
118137
// So fall back to a fetch() from the page and hope we get a cached response that way.
119138
fetchFromNetworkCache(url, resolve, reject);
120139
});
121-
};
140+
}
122141

123142
export default fetchFileWithCaching;

packages/react-devtools-extensions/src/main/index.js

Lines changed: 8 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -128,34 +128,13 @@ function createBridgeAndStore() {
128128
}
129129
};
130130

131-
const viewElementSourceFunction = id => {
132-
const rendererID = store.getRendererIDForElement(id);
133-
if (rendererID != null) {
134-
// Ask the renderer interface to determine the component function,
135-
// and store it as a global variable on the window
136-
bridge.send('viewElementSource', {id, rendererID});
131+
const viewElementSourceFunction = (source, symbolicatedSource) => {
132+
const {sourceURL, line, column} = symbolicatedSource
133+
? symbolicatedSource
134+
: source;
137135

138-
setTimeout(() => {
139-
// Ask Chrome to display the location of the component function,
140-
// or a render method if it is a Class (ideally Class instance, not type)
141-
// assuming the renderer found one.
142-
chrome.devtools.inspectedWindow.eval(`
143-
if (window.$type != null) {
144-
if (
145-
window.$type &&
146-
window.$type.prototype &&
147-
window.$type.prototype.isReactComponent
148-
) {
149-
// inspect Component.render, not constructor
150-
inspect(window.$type.prototype.render);
151-
} else {
152-
// inspect Functional Component
153-
inspect(window.$type);
154-
}
155-
}
156-
`);
157-
}, 100);
158-
}
136+
// We use 1-based line and column, Chrome expects them 0-based.
137+
chrome.devtools.panels.openResource(sourceURL, line - 1, column - 1);
159138
};
160139

161140
// TODO (Webpack 5) Hopefully we can remove this prop after the Webpack 5 migration.
@@ -183,17 +162,14 @@ function createBridgeAndStore() {
183162
store,
184163
warnIfUnsupportedVersionDetected: true,
185164
viewAttributeSourceFunction,
165+
// Firefox doesn't support chrome.devtools.panels.openResource yet
166+
canViewElementSourceFunction: () => __IS_CHROME__ || __IS_EDGE__,
186167
viewElementSourceFunction,
187-
viewUrlSourceFunction,
188168
}),
189169
);
190170
};
191171
}
192172

193-
const viewUrlSourceFunction = (url, line, col) => {
194-
chrome.devtools.panels.openResource(url, line, col);
195-
};
196-
197173
function ensureInitialHTMLIsCleared(container) {
198174
if (container._hasInitialHTMLBeenCleared) {
199175
return;

packages/react-devtools-extensions/webpack.config.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ const LOGGING_URL = process.env.LOGGING_URL || null;
3939
const IS_CHROME = process.env.IS_CHROME === 'true';
4040
const IS_FIREFOX = process.env.IS_FIREFOX === 'true';
4141
const IS_EDGE = process.env.IS_EDGE === 'true';
42+
const IS_INTERNAL_VERSION = process.env.FEATURE_FLAG_TARGET === 'extension-fb';
4243

4344
const featureFlagTarget = process.env.FEATURE_FLAG_TARGET || 'extension-oss';
4445

@@ -119,6 +120,7 @@ module.exports = {
119120
__IS_CHROME__: IS_CHROME,
120121
__IS_FIREFOX__: IS_FIREFOX,
121122
__IS_EDGE__: IS_EDGE,
123+
__IS_INTERNAL_VERSION__: IS_INTERNAL_VERSION,
122124
'process.env.DEVTOOLS_PACKAGE': `"react-devtools-extensions"`,
123125
'process.env.DEVTOOLS_VERSION': `"${DEVTOOLS_VERSION}"`,
124126
'process.env.EDITOR_URL': EDITOR_URL != null ? `"${EDITOR_URL}"` : null,

packages/react-devtools-shared/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
"@reach/tooltip": "^0.16.0",
2020
"clipboard-js": "^0.3.6",
2121
"compare-versions": "^5.0.3",
22+
"jsc-safe-url": "^0.2.4",
2223
"json5": "^2.1.3",
2324
"local-storage-fallback": "^4.1.1",
2425
"lodash.throttle": "^4.1.1",

packages/react-devtools-shared/src/backendAPI.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -261,7 +261,9 @@ export function convertInspectedElementBackendToFrontend(
261261
rendererPackageName,
262262
rendererVersion,
263263
rootType,
264-
source,
264+
// Previous backend implementations (<= 5.0.1) have a different interface for Source, with fileName.
265+
// This gates the source features for only compatible backends: >= 5.0.2
266+
source: source && source.sourceURL ? source : null,
265267
type,
266268
owners:
267269
owners === null

0 commit comments

Comments
 (0)