Skip to content

Commit 9fc04ea

Browse files
author
Brian Vaughn
authored
DevTools: Improve named hooks network caching (#22198)
While testing the recently-launched named hooks feature, I noticed that one of the two big performance bottlenecks is fetching the source file. This was unexpected since the source file has already been loaded by the page. (After all, DevTools is inspecting a component defined in that same file.) To address this, I made the following changes: - [x] Keep CPU bound work (parsing source map and AST) in a worker so it doesn't block the main thread but move I/O bound code (fetching files) to the main thread. - [x] Inject a function into the page (as part of the content script) to fetch cached files for the extension. Communicate with this function using `eval()` (to send it messages) and `chrome.runtime.sendMessage()` to return its responses to the extension). With the above changes in place, the extension gets cached responses from a lot of sites- but not Facebook. This seems to be due to the following: * Facebook's response headers include [`vary: 'Origin'`](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Vary). * The `fetch` made from the content script does not include an `Origin` request header. To reduce the impact of cases where we can't re-use the Network cache, this PR also makes additional changes: - [x] Use `devtools.network.onRequestFinished` to (pre)cache resources as the page loads them. This allows us to avoid requesting a resource that's already been loaded in most cases. - [x] In case DevTools was opened _after_ some requests were made, we also now pre-fetch (and cache in memory) source files when a component is selected (if it has hooks). If the component's hooks are later evaluated, the source map will be faster to access. (Note that in many cases, this prefetch is very fast since it is returned from the disk cache.) With the above changes, we've reduced the time spent in `loadSourceFiles` to nearly nothing.
1 parent 67f3836 commit 9fc04ea

15 files changed

+1364
-816
lines changed

packages/react-devtools-extensions/src/__tests__/parseHookNames-test.js

+49-21
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,23 @@ function requireText(path, encoding) {
2525
}
2626
}
2727

28+
function initFetchMock() {
29+
const fetchMock = require('jest-fetch-mock');
30+
fetchMock.enableMocks();
31+
fetchMock.mockIf(/.+$/, request => {
32+
const url = request.url;
33+
const isLoadingExternalSourceMap = /external\/.*\.map/.test(url);
34+
if (isLoadingExternalSourceMap) {
35+
// Assert that url contains correct query params
36+
expect(url.includes('?foo=bar&param=some_value')).toBe(true);
37+
const fileSystemPath = url.split('?')[0];
38+
return requireText(fileSystemPath, 'utf8');
39+
}
40+
return requireText(url, 'utf8');
41+
});
42+
return fetchMock;
43+
}
44+
2845
describe('parseHookNames', () => {
2946
let fetchMock;
3047
let inspectHooks;
@@ -37,12 +54,32 @@ describe('parseHookNames', () => {
3754
console.trace('source-map-support');
3855
});
3956

40-
fetchMock = require('jest-fetch-mock');
41-
fetchMock.enableMocks();
57+
fetchMock = initFetchMock();
4258

4359
inspectHooks = require('react-debug-tools/src/ReactDebugHooks')
4460
.inspectHooks;
45-
parseHookNames = require('../parseHookNames/parseHookNames').parseHookNames;
61+
62+
// Jest can't run the workerized version of this module.
63+
const {
64+
flattenHooksList,
65+
loadSourceAndMetadata,
66+
} = require('../parseHookNames/loadSourceAndMetadata');
67+
const parseSourceAndMetadata = require('../parseHookNames/parseSourceAndMetadata')
68+
.parseSourceAndMetadata;
69+
parseHookNames = async hooksTree => {
70+
const hooksList = flattenHooksList(hooksTree);
71+
72+
// Runs in the UI thread so it can share Network cache:
73+
const locationKeyToHookSourceAndMetadata = await loadSourceAndMetadata(
74+
hooksList,
75+
);
76+
77+
// Runs in a Worker because it's CPU intensive:
78+
return parseSourceAndMetadata(
79+
hooksList,
80+
locationKeyToHookSourceAndMetadata,
81+
);
82+
};
4683

4784
// Jest (jest-runner?) configures Errors to automatically account for source maps.
4885
// This changes behavior between our tests and the browser.
@@ -55,18 +92,6 @@ describe('parseHookNames', () => {
5592
Error.prepareStackTrace = (error, trace) => {
5693
return error.stack;
5794
};
58-
59-
fetchMock.mockIf(/.+$/, request => {
60-
const url = request.url;
61-
const isLoadingExternalSourceMap = /external\/.*\.map/.test(url);
62-
if (isLoadingExternalSourceMap) {
63-
// Assert that url contains correct query params
64-
expect(url.includes('?foo=bar&param=some_value')).toBe(true);
65-
const fileSystemPath = url.split('?')[0];
66-
return requireText(fileSystemPath, 'utf8');
67-
}
68-
return requireText(url, 'utf8');
69-
});
7095
});
7196

7297
afterEach(() => {
@@ -880,18 +905,20 @@ describe('parseHookNames', () => {
880905
describe('parseHookNames worker', () => {
881906
let inspectHooks;
882907
let parseHookNames;
883-
let workerizedParseHookNamesMock;
908+
let workerizedParseSourceAndMetadataMock;
884909

885910
beforeEach(() => {
886911
window.Worker = undefined;
887912

888-
workerizedParseHookNamesMock = jest.fn();
913+
workerizedParseSourceAndMetadataMock = jest.fn();
889914

890-
jest.mock('../parseHookNames/parseHookNames.worker.js', () => {
915+
initFetchMock();
916+
917+
jest.mock('../parseHookNames/parseSourceAndMetadata.worker.js', () => {
891918
return {
892919
__esModule: true,
893920
default: () => ({
894-
parseHookNames: workerizedParseHookNamesMock,
921+
parseSourceAndMetadata: workerizedParseSourceAndMetadataMock,
895922
}),
896923
};
897924
});
@@ -912,11 +939,12 @@ describe('parseHookNames worker', () => {
912939
.Component;
913940

914941
window.Worker = true;
915-
// resets module so mocked worker instance can be updated
942+
943+
// Reset module so mocked worker instance can be updated.
916944
jest.resetModules();
917945
parseHookNames = require('../parseHookNames').parseHookNames;
918946

919947
await getHookNamesForComponent(Component);
920-
expect(workerizedParseHookNamesMock).toHaveBeenCalledTimes(1);
948+
expect(workerizedParseSourceAndMetadataMock).toHaveBeenCalledTimes(1);
921949
});
922950
});

packages/react-devtools-extensions/src/background.js

+15-2
Original file line numberDiff line numberDiff line change
@@ -117,14 +117,27 @@ chrome.tabs.onUpdated.addListener((tabId, changeInfo, tab) => {
117117
});
118118

119119
chrome.runtime.onMessage.addListener((request, sender) => {
120-
if (sender.tab) {
120+
const tab = sender.tab;
121+
if (tab) {
122+
const id = tab.id;
121123
// This is sent from the hook content script.
122124
// It tells us a renderer has attached.
123125
if (request.hasDetectedReact) {
124126
// We use browserAction instead of pageAction because this lets us
125127
// display a custom default popup when React is *not* detected.
126128
// It is specified in the manifest.
127-
setIconAndPopup(request.reactBuildType, sender.tab.id);
129+
setIconAndPopup(request.reactBuildType, id);
130+
} else {
131+
switch (request.payload?.type) {
132+
case 'fetch-file-with-cache-complete':
133+
case 'fetch-file-with-cache-error':
134+
// Forward the result of fetch-in-page requests back to the extension.
135+
const devtools = ports[id]?.devtools;
136+
if (devtools) {
137+
devtools.postMessage(request);
138+
}
139+
break;
140+
}
128141
}
129142
}
130143
});

packages/react-devtools-extensions/src/injectGlobalHook.js

+62-17
Original file line numberDiff line numberDiff line change
@@ -23,40 +23,85 @@ let lastDetectionResult;
2323
// (it will be injected directly into the page).
2424
// So instead, the hook will use postMessage() to pass message to us here.
2525
// And when this happens, we'll send a message to the "background page".
26-
window.addEventListener('message', function(evt) {
27-
if (evt.source !== window || !evt.data) {
26+
window.addEventListener('message', function onMessage({data, source}) {
27+
if (source !== window || !data) {
2828
return;
2929
}
30-
if (evt.data.source === 'react-devtools-detector') {
31-
lastDetectionResult = {
32-
hasDetectedReact: true,
33-
reactBuildType: evt.data.reactBuildType,
34-
};
35-
chrome.runtime.sendMessage(lastDetectionResult);
36-
} else if (evt.data.source === 'react-devtools-inject-backend') {
37-
const script = document.createElement('script');
38-
script.src = chrome.runtime.getURL('build/react_devtools_backend.js');
39-
document.documentElement.appendChild(script);
40-
script.parentNode.removeChild(script);
30+
31+
switch (data.source) {
32+
case 'react-devtools-detector':
33+
lastDetectionResult = {
34+
hasDetectedReact: true,
35+
reactBuildType: data.reactBuildType,
36+
};
37+
chrome.runtime.sendMessage(lastDetectionResult);
38+
break;
39+
case 'react-devtools-extension':
40+
if (data.payload?.type === 'fetch-file-with-cache') {
41+
const url = data.payload.url;
42+
43+
const reject = value => {
44+
chrome.runtime.sendMessage({
45+
source: 'react-devtools-content-script',
46+
payload: {
47+
type: 'fetch-file-with-cache-error',
48+
url,
49+
value,
50+
},
51+
});
52+
};
53+
54+
const resolve = value => {
55+
chrome.runtime.sendMessage({
56+
source: 'react-devtools-content-script',
57+
payload: {
58+
type: 'fetch-file-with-cache-complete',
59+
url,
60+
value,
61+
},
62+
});
63+
};
64+
65+
fetch(url, {cache: 'force-cache'}).then(
66+
response => {
67+
if (response.ok) {
68+
response
69+
.text()
70+
.then(text => resolve(text))
71+
.catch(error => reject(null));
72+
} else {
73+
reject(null);
74+
}
75+
},
76+
error => reject(null),
77+
);
78+
}
79+
break;
80+
case 'react-devtools-inject-backend':
81+
const script = document.createElement('script');
82+
script.src = chrome.runtime.getURL('build/react_devtools_backend.js');
83+
document.documentElement.appendChild(script);
84+
script.parentNode.removeChild(script);
85+
break;
4186
}
4287
});
4388

4489
// NOTE: Firefox WebExtensions content scripts are still alive and not re-injected
4590
// while navigating the history to a document that has not been destroyed yet,
4691
// replay the last detection result if the content script is active and the
4792
// document has been hidden and shown again.
48-
window.addEventListener('pageshow', function(evt) {
49-
if (!lastDetectionResult || evt.target !== window.document) {
93+
window.addEventListener('pageshow', function({target}) {
94+
if (!lastDetectionResult || target !== window.document) {
5095
return;
5196
}
5297
chrome.runtime.sendMessage(lastDetectionResult);
5398
});
5499

55100
const detectReact = `
56-
window.__REACT_DEVTOOLS_GLOBAL_HOOK__.on('renderer', function(evt) {
101+
window.__REACT_DEVTOOLS_GLOBAL_HOOK__.on('renderer', function({reactBuildType}) {
57102
window.postMessage({
58103
source: 'react-devtools-detector',
59-
reactBuildType: evt.reactBuildType,
104+
reactBuildType,
60105
}, '*');
61106
});
62107
`;

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

+84-1
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,27 @@ const LOCAL_STORAGE_SUPPORTS_PROFILING_KEY =
2525

2626
const isChrome = getBrowserName() === 'Chrome';
2727

28+
const cachedNetworkEvents = new Map();
29+
30+
// Cache JavaScript resources as the page loads them.
31+
// This helps avoid unnecessary duplicate requests when hook names are parsed.
32+
// Responses with a Vary: 'Origin' might not match future requests.
33+
// This lets us avoid a possible (expensive) cache miss.
34+
// For more info see: github.com/facebook/react/pull/22198
35+
chrome.devtools.network.onRequestFinished.addListener(
36+
function onRequestFinished(event) {
37+
if (event.request.method === 'GET') {
38+
switch (event.response.content.mimeType) {
39+
case 'application/javascript':
40+
case 'application/x-javascript':
41+
case 'text/javascript':
42+
cachedNetworkEvents.set(event.request.url, event);
43+
break;
44+
}
45+
}
46+
},
47+
);
48+
2849
let panelCreated = false;
2950

3051
// The renderer interface can't read saved component filters directly,
@@ -212,20 +233,76 @@ function createPanelIfReactLoaded() {
212233
}
213234
};
214235

236+
// For some reason in Firefox, chrome.runtime.sendMessage() from a content script
237+
// never reaches the chrome.runtime.onMessage event listener.
238+
let fetchFileWithCaching = null;
239+
if (isChrome) {
240+
// Fetching files from the extension won't make use of the network cache
241+
// for resources that have already been loaded by the page.
242+
// This helper function allows the extension to request files to be fetched
243+
// by the content script (running in the page) to increase the likelihood of a cache hit.
244+
fetchFileWithCaching = url => {
245+
const event = cachedNetworkEvents.get(url);
246+
if (event != null) {
247+
// If this resource has already been cached locally,
248+
// skip the network queue (which might not be a cache hit anyway)
249+
// and just use the cached response.
250+
return new Promise(resolve => {
251+
event.getContent(content => resolve(content));
252+
});
253+
}
254+
255+
// If DevTools was opened after the page started loading,
256+
// we may have missed some requests.
257+
// So fall back to a fetch() and hope we get a cached response.
258+
259+
return new Promise((resolve, reject) => {
260+
function onPortMessage({payload, source}) {
261+
if (source === 'react-devtools-content-script') {
262+
switch (payload?.type) {
263+
case 'fetch-file-with-cache-complete':
264+
chrome.runtime.onMessage.removeListener(onPortMessage);
265+
resolve(payload.value);
266+
break;
267+
case 'fetch-file-with-cache-error':
268+
chrome.runtime.onMessage.removeListener(onPortMessage);
269+
reject(payload.value);
270+
break;
271+
}
272+
}
273+
}
274+
275+
chrome.runtime.onMessage.addListener(onPortMessage);
276+
277+
chrome.devtools.inspectedWindow.eval(`
278+
window.postMessage({
279+
source: 'react-devtools-extension',
280+
payload: {
281+
type: 'fetch-file-with-cache',
282+
url: "${url}",
283+
},
284+
});
285+
`);
286+
});
287+
};
288+
}
289+
215290
root = createRoot(document.createElement('div'));
216291

217292
render = (overrideTab = mostRecentOverrideTab) => {
218293
mostRecentOverrideTab = overrideTab;
219294
import('./parseHookNames').then(
220-
({parseHookNames, purgeCachedMetadata}) => {
295+
({parseHookNames, prefetchSourceFiles, purgeCachedMetadata}) => {
221296
root.render(
222297
createElement(DevTools, {
223298
bridge,
224299
browserTheme: getBrowserTheme(),
225300
componentsPortalContainer,
226301
enabledInspectedElementContextMenu: true,
302+
fetchFileWithCaching,
227303
loadHookNames: parseHookNames,
228304
overrideTab,
305+
prefetchSourceFiles,
229306
profilerPortalContainer,
230307
purgeCachedHookNamesMetadata: purgeCachedMetadata,
231308
showTabBar: false,
@@ -366,6 +443,9 @@ function createPanelIfReactLoaded() {
366443

367444
// Re-initialize DevTools panel when a new page is loaded.
368445
chrome.devtools.network.onNavigated.addListener(function onNavigated() {
446+
// Clear cached requests when a new page is opened.
447+
cachedNetworkEvents.clear();
448+
369449
// Re-initialize saved filters on navigation,
370450
// since global values stored on window get reset in this case.
371451
syncSavedPreferences();
@@ -382,6 +462,9 @@ function createPanelIfReactLoaded() {
382462

383463
// Load (or reload) the DevTools extension when the user navigates to a new page.
384464
function checkPageForReact() {
465+
// Clear cached requests when a new page is opened.
466+
cachedNetworkEvents.clear();
467+
385468
syncSavedPreferences();
386469
createPanelIfReactLoaded();
387470
}

0 commit comments

Comments
 (0)