Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DevTools: Improve named hooks network caching #22198

Merged
merged 11 commits into from
Sep 1, 2021
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,23 @@ function requireText(path, encoding) {
}
}

function initFetchMock() {
const fetchMock = require('jest-fetch-mock');
fetchMock.enableMocks();
fetchMock.mockIf(/.+$/, request => {
const url = request.url;
const isLoadingExternalSourceMap = /external\/.*\.map/.test(url);
if (isLoadingExternalSourceMap) {
// Assert that url contains correct query params
expect(url.includes('?foo=bar&param=some_value')).toBe(true);
const fileSystemPath = url.split('?')[0];
return requireText(fileSystemPath, 'utf8');
}
return requireText(url, 'utf8');
});
return fetchMock;
}

describe('parseHookNames', () => {
let fetchMock;
let inspectHooks;
Expand All @@ -37,12 +54,32 @@ describe('parseHookNames', () => {
console.trace('source-map-support');
});

fetchMock = require('jest-fetch-mock');
fetchMock.enableMocks();
fetchMock = initFetchMock();

inspectHooks = require('react-debug-tools/src/ReactDebugHooks')
.inspectHooks;
parseHookNames = require('../parseHookNames/parseHookNames').parseHookNames;

// Jest can't run the workerized version of this module.
const {
flattenHooksList,
loadSourceAndMetadata,
} = require('../parseHookNames/loadSourceAndMetadata');
const parseSourceAndMetadata = require('../parseHookNames/parseSourceAndMetadata')
.parseSourceAndMetadata;
parseHookNames = async hooksTree => {
const hooksList = flattenHooksList(hooksTree);

// Runs in the UI thread so it can share Network cache:
const locationKeyToHookSourceAndMetadata = await loadSourceAndMetadata(
hooksList,
);

// Runs in a Worker because it's CPU intensive:
return parseSourceAndMetadata(
hooksList,
locationKeyToHookSourceAndMetadata,
);
};

// Jest (jest-runner?) configures Errors to automatically account for source maps.
// This changes behavior between our tests and the browser.
Expand All @@ -55,18 +92,6 @@ describe('parseHookNames', () => {
Error.prepareStackTrace = (error, trace) => {
return error.stack;
};

fetchMock.mockIf(/.+$/, request => {
const url = request.url;
const isLoadingExternalSourceMap = /external\/.*\.map/.test(url);
if (isLoadingExternalSourceMap) {
// Assert that url contains correct query params
expect(url.includes('?foo=bar&param=some_value')).toBe(true);
const fileSystemPath = url.split('?')[0];
return requireText(fileSystemPath, 'utf8');
}
return requireText(url, 'utf8');
});
});

afterEach(() => {
Expand Down Expand Up @@ -880,18 +905,20 @@ describe('parseHookNames', () => {
describe('parseHookNames worker', () => {
let inspectHooks;
let parseHookNames;
let workerizedParseHookNamesMock;
let workerizedParseSourceAndMetadataMock;

beforeEach(() => {
window.Worker = undefined;

workerizedParseHookNamesMock = jest.fn();
workerizedParseSourceAndMetadataMock = jest.fn();

jest.mock('../parseHookNames/parseHookNames.worker.js', () => {
initFetchMock();
bvaughn marked this conversation as resolved.
Show resolved Hide resolved

jest.mock('../parseHookNames/parseSourceAndMetadata.worker.js', () => {
return {
__esModule: true,
default: () => ({
parseHookNames: workerizedParseHookNamesMock,
parseSourceAndMetadata: workerizedParseSourceAndMetadataMock,
}),
};
});
Expand All @@ -912,11 +939,12 @@ describe('parseHookNames worker', () => {
.Component;

window.Worker = true;
// resets module so mocked worker instance can be updated

// Reset module so mocked worker instance can be updated.
jest.resetModules();
parseHookNames = require('../parseHookNames').parseHookNames;

await getHookNamesForComponent(Component);
expect(workerizedParseHookNamesMock).toHaveBeenCalledTimes(1);
expect(workerizedParseSourceAndMetadataMock).toHaveBeenCalledTimes(1);
});
});
17 changes: 15 additions & 2 deletions packages/react-devtools-extensions/src/background.js
Original file line number Diff line number Diff line change
Expand Up @@ -117,14 +117,27 @@ chrome.tabs.onUpdated.addListener((tabId, changeInfo, tab) => {
});

chrome.runtime.onMessage.addListener((request, sender) => {
if (sender.tab) {
const tab = sender.tab;
if (tab) {
const id = tab.id;
// This is sent from the hook content script.
// It tells us a renderer has attached.
if (request.hasDetectedReact) {
// We use browserAction instead of pageAction because this lets us
// display a custom default popup when React is *not* detected.
// It is specified in the manifest.
setIconAndPopup(request.reactBuildType, sender.tab.id);
setIconAndPopup(request.reactBuildType, id);
} else {
switch (request.payload?.type) {
bvaughn marked this conversation as resolved.
Show resolved Hide resolved
case 'fetch-file-with-cache-complete':
case 'fetch-file-with-cache-error':
// Forward the result of fetch-in-page requests back to the extension.
const devtools = ports[id]?.devtools;
if (devtools) {
devtools.postMessage(request);
}
break;
}
}
}
});
79 changes: 62 additions & 17 deletions packages/react-devtools-extensions/src/injectGlobalHook.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,40 +23,85 @@ let lastDetectionResult;
// (it will be injected directly into the page).
// So instead, the hook will use postMessage() to pass message to us here.
// And when this happens, we'll send a message to the "background page".
window.addEventListener('message', function(evt) {
if (evt.source !== window || !evt.data) {
window.addEventListener('message', function onMessage({data, source}) {
if (source !== window || !data) {
return;
}
if (evt.data.source === 'react-devtools-detector') {
lastDetectionResult = {
hasDetectedReact: true,
reactBuildType: evt.data.reactBuildType,
};
chrome.runtime.sendMessage(lastDetectionResult);
} else if (evt.data.source === 'react-devtools-inject-backend') {
const script = document.createElement('script');
script.src = chrome.runtime.getURL('build/react_devtools_backend.js');
document.documentElement.appendChild(script);
script.parentNode.removeChild(script);

switch (data.source) {
case 'react-devtools-detector':
lastDetectionResult = {
hasDetectedReact: true,
reactBuildType: data.reactBuildType,
};
chrome.runtime.sendMessage(lastDetectionResult);
break;
case 'react-devtools-extension':
if (data.payload?.type === 'fetch-file-with-cache') {
const url = data.payload.url;

const reject = value => {
chrome.runtime.sendMessage({
source: 'react-devtools-content-script',
payload: {
type: 'fetch-file-with-cache-error',
url,
value,
},
});
};

const resolve = value => {
chrome.runtime.sendMessage({
source: 'react-devtools-content-script',
payload: {
type: 'fetch-file-with-cache-complete',
url,
value,
},
});
};
bvaughn marked this conversation as resolved.
Show resolved Hide resolved

fetch(url, {cache: 'force-cache'}).then(
response => {
if (response.ok) {
response
.text()
.then(text => resolve(text))
.catch(error => reject(null));
} else {
reject(null);
}
},
error => reject(null),
);
}
break;
case 'react-devtools-inject-backend':
const script = document.createElement('script');
script.src = chrome.runtime.getURL('build/react_devtools_backend.js');
document.documentElement.appendChild(script);
script.parentNode.removeChild(script);
break;
}
});

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

const detectReact = `
window.__REACT_DEVTOOLS_GLOBAL_HOOK__.on('renderer', function(evt) {
window.__REACT_DEVTOOLS_GLOBAL_HOOK__.on('renderer', function({reactBuildType}) {
window.postMessage({
source: 'react-devtools-detector',
reactBuildType: evt.reactBuildType,
reactBuildType,
}, '*');
});
`;
Expand Down
85 changes: 84 additions & 1 deletion packages/react-devtools-extensions/src/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,27 @@ const LOCAL_STORAGE_SUPPORTS_PROFILING_KEY =

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

const cachedNetworkEvents = new Map();

// Cache JavaScript resources as the page loads them.
// This helps avoid unnecessary duplicate requests when hook names are parsed.
// Responses with a Vary: 'Origin' might not match future requests.
// This lets us avoid a possible (expensive) cache miss.
// For more info see: github.com/facebook/react/pull/22198
chrome.devtools.network.onRequestFinished.addListener(
function onRequestFinished(event) {
if (event.request.method === 'GET') {
switch (event.response.content.mimeType) {
case 'application/javascript':
case 'application/x-javascript':
case 'text/javascript':
cachedNetworkEvents.set(event.request.url, event);
break;
}
}
},
);

let panelCreated = false;

// The renderer interface can't read saved component filters directly,
Expand Down Expand Up @@ -212,20 +233,76 @@ function createPanelIfReactLoaded() {
}
};

// For some reason in Firefox, chrome.runtime.sendMessage() from a content script
// never reaches the chrome.runtime.onMessage event listener.
let fetchFileWithCaching = null;
if (isChrome) {
// Fetching files from the extension won't make use of the network cache
// for resources that have already been loaded by the page.
// This helper function allows the extension to request files to be fetched
// by the content script (running in the page) to increase the likelihood of a cache hit.
fetchFileWithCaching = url => {
const event = cachedNetworkEvents.get(url);
if (event != null) {
// If this resource has already been cached locally,
// skip the network queue (which might not be a cache hit anyway)
// and just use the cached response.
return new Promise(resolve => {
event.getContent(content => resolve(content));
});
}

// If DevTools was opened after the page started loading,
// we may have missed some requests.
// So fall back to a fetch() and hope we get a cached response.

return new Promise((resolve, reject) => {
function onPortMessage({payload, source}) {
if (source === 'react-devtools-content-script') {
switch (payload?.type) {
case 'fetch-file-with-cache-complete':
chrome.runtime.onMessage.removeListener(onPortMessage);
resolve(payload.value);
break;
case 'fetch-file-with-cache-error':
chrome.runtime.onMessage.removeListener(onPortMessage);
reject(payload.value);
break;
}
}
}

chrome.runtime.onMessage.addListener(onPortMessage);

chrome.devtools.inspectedWindow.eval(`
window.postMessage({
source: 'react-devtools-extension',
payload: {
type: 'fetch-file-with-cache',
url: "${url}",
},
});
`);
});
};
}

root = createRoot(document.createElement('div'));

render = (overrideTab = mostRecentOverrideTab) => {
mostRecentOverrideTab = overrideTab;
import('./parseHookNames').then(
({parseHookNames, purgeCachedMetadata}) => {
({parseHookNames, prefetchSourceFiles, purgeCachedMetadata}) => {
root.render(
createElement(DevTools, {
bridge,
browserTheme: getBrowserTheme(),
componentsPortalContainer,
enabledInspectedElementContextMenu: true,
fetchFileWithCaching,
loadHookNames: parseHookNames,
overrideTab,
prefetchSourceFiles,
profilerPortalContainer,
purgeCachedHookNamesMetadata: purgeCachedMetadata,
showTabBar: false,
Expand Down Expand Up @@ -366,6 +443,9 @@ function createPanelIfReactLoaded() {

// Re-initialize DevTools panel when a new page is loaded.
chrome.devtools.network.onNavigated.addListener(function onNavigated() {
// Clear cached requests when a new page is opened.
cachedNetworkEvents.clear();

// Re-initialize saved filters on navigation,
// since global values stored on window get reset in this case.
syncSavedPreferences();
Expand All @@ -382,6 +462,9 @@ function createPanelIfReactLoaded() {

// Load (or reload) the DevTools extension when the user navigates to a new page.
function checkPageForReact() {
// Clear cached requests when a new page is opened.
cachedNetworkEvents.clear();

syncSavedPreferences();
createPanelIfReactLoaded();
}
Expand Down
Loading