Skip to content

[DevTools] Handle case when cached network requests have null content in response #22282

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.

Sign up for GitHub

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

Closed
wants to merge 4 commits into from
Closed

[DevTools] Handle case when cached network requests have null content in response #22282

Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
297d27f
[DevTools] Handle case when cached network events have null content
Sep 9, 2021
dc8c512
[DevTools] Add DEBUG logging for fetching cached source files
Sep 10, 2021
ef1f293
[DevTools] Dont eagerly getContent for requests, only handle nullabil…
Sep 10, 2021
de9dbe5
Correctly wait for content for cached request to be loaded
Sep 10, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 65 additions & 19 deletions packages/react-devtools-extensions/src/main.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import Bridge from 'react-devtools-shared/src/bridge';
import Store from 'react-devtools-shared/src/devtools/store';
import {getBrowserName, getBrowserTheme} from './utils';
import {LOCAL_STORAGE_TRACE_UPDATES_ENABLED_KEY} from 'react-devtools-shared/src/constants';
import {__DEBUG__} from 'react-devtools-shared/src/constants';
import {
getAppendComponentStack,
getBreakOnConsoleErrors,
Expand All @@ -25,21 +26,28 @@ const LOCAL_STORAGE_SUPPORTS_PROFILING_KEY =

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

const cachedNetworkEvents = new Map();
const cachedRequests = 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) {
function onRequestFinished(request) {
if (request.request.method === 'GET') {
switch (request.response.content.mimeType) {
case 'application/javascript':
case 'application/x-javascript':
case 'text/javascript':
cachedNetworkEvents.set(event.request.url, event);
const url = request.request.url;
if (__DEBUG__) {
console.log(
'[main] onRequestFinished() Request finished for ',
url,
);
}
cachedRequests.set(url, request);
break;
}
}
Expand Down Expand Up @@ -242,21 +250,59 @@ function createPanelIfReactLoaded() {
// 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 (__DEBUG__) {
console.log('[main] fetchFileWithCaching() for ', url);
}
return new Promise((resolve, reject) => {
const request = cachedRequests.get(url);
let cachedContent = null;
if (request != null) {
if (__DEBUG__) {
console.log(
'[main] fetchFileWithCaching() Found a cached network request for ',
url,
);
}

request.getContent(content => {
if (content != null) {
cachedContent = content;
} else {
if (__DEBUG__) {
console.log(
'[main] fetchFileWithCaching() Invalid source file content returned by getContent() for ',
url,
);
}
}
});
}

// 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.
// If this request has already been cached locally,
// skip the network queue (which might not be a cache hit anyway)
// and use the cached response.
if (cachedContent != null) {
if (__DEBUG__) {
console.log(
'[main] fetchFileWithCaching() Reusing cached source file content for ',
url,
);
}
resolve(cachedContent);
return;
}

return new Promise((resolve, reject) => {
// If we didn't find a cached network request, or were unable to
// obtain valid content for the cached request, fall back to a fetch()
// and hope we get a cached response.
// We might miss caching some network requests if DevTools was opened
// after the page started loading.
if (__DEBUG__) {
console.log(
'[main] fetchFileWithCaching() Fetching from page: ',
url,
);
}
function onPortMessage({payload, source}) {
if (source === 'react-devtools-content-script') {
switch (payload?.type) {
Expand Down Expand Up @@ -442,7 +488,7 @@ 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();
cachedRequests.clear();

// Re-initialize saved filters on navigation,
// since global values stored on window get reset in this case.
Expand All @@ -461,7 +507,7 @@ 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();
cachedRequests.clear();

syncSavedPreferences();
createPanelIfReactLoaded();
Expand Down