Skip to content

Commit 24c2e27

Browse files
author
Brian Vaughn
authored
DevTools Suspense cache cleanup (#22275)
1 parent 4ce89a5 commit 24c2e27

File tree

5 files changed

+120
-87
lines changed

5 files changed

+120
-87
lines changed

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

Lines changed: 42 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -110,44 +110,51 @@ export function loadHookNames(
110110

111111
let didTimeout = false;
112112

113-
loadHookNamesFunction(hooksTree, fetchFileWithCaching).then(
114-
function onSuccess(hookNames) {
115-
if (didTimeout) {
116-
return;
117-
}
118-
119-
if (__DEBUG__) {
120-
console.log('[hookNamesCache] onSuccess() hookNames:', hookNames);
121-
}
122-
123-
if (hookNames) {
124-
const resolvedRecord = ((newRecord: any): ResolvedRecord<HookNames>);
125-
resolvedRecord.status = Resolved;
126-
resolvedRecord.value = hookNames;
127-
} else {
128-
const notFoundRecord = ((newRecord: any): RejectedRecord);
129-
notFoundRecord.status = Rejected;
130-
notFoundRecord.value = null;
131-
}
132-
133-
wake();
134-
},
135-
function onError(error) {
136-
if (didTimeout) {
137-
return;
138-
}
113+
const onSuccess = hookNames => {
114+
if (didTimeout) {
115+
return;
116+
}
117+
118+
if (__DEBUG__) {
119+
console.log('[hookNamesCache] onSuccess() hookNames:', hookNames);
120+
}
139121

140-
if (__DEBUG__) {
141-
console.log('[hookNamesCache] onError() error:', error);
142-
}
122+
if (hookNames) {
123+
const resolvedRecord = ((newRecord: any): ResolvedRecord<HookNames>);
124+
resolvedRecord.status = Resolved;
125+
resolvedRecord.value = hookNames;
126+
} else {
127+
const notFoundRecord = ((newRecord: any): RejectedRecord);
128+
notFoundRecord.status = Rejected;
129+
notFoundRecord.value = null;
130+
}
143131

144-
const thrownRecord = ((newRecord: any): RejectedRecord);
145-
thrownRecord.status = Rejected;
146-
thrownRecord.value = null;
132+
wake();
133+
};
147134

148-
wake();
149-
},
150-
);
135+
const onError = error => {
136+
if (didTimeout) {
137+
return;
138+
}
139+
140+
if (__DEBUG__) {
141+
console.log('[hookNamesCache] onError()');
142+
}
143+
144+
console.error(error);
145+
146+
const thrownRecord = ((newRecord: any): RejectedRecord);
147+
thrownRecord.status = Rejected;
148+
thrownRecord.value = null;
149+
150+
wake();
151+
};
152+
153+
const thenable = loadHookNamesFunction(hooksTree, fetchFileWithCaching);
154+
thenable.then(onSuccess, onError);
155+
if (typeof (thenable: any).catch === 'function') {
156+
(thenable: any).catch(onError);
157+
}
151158

152159
// Eventually timeout and stop trying to load names.
153160
let timeoutID = setTimeout(function onTimeout() {

packages/react-devtools-shared/src/hooks/__tests__/__source__/ContainingStringSourceMappingURL.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,17 @@ import React, {useState} from 'react';
1111

1212
// ?sourceMappingURL=([^\s'"]+)/gm
1313

14+
const abc = 'abc';
15+
const string =
16+
'sourceMappingURL=data:application/json;charset=utf-8;base64,' + abc;
17+
1418
export function Component() {
1519
const [count, setCount] = useState(0);
1620

1721
return (
1822
<div>
1923
<p>You clicked {count} times</p>
24+
<div>string: {string}</div>
2025
<button onClick={() => setCount(count + 1)}>Click me</button>
2126
</div>
2227
);

packages/react-devtools-shared/src/hooks/__tests__/parseHookNames-test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ describe('parseHookNames', () => {
136136
.Component;
137137

138138
fetchMock.mockIf(/.+$/, request => {
139-
// Since th custom hook contains only unnamed hooks, the source code should not be loaded.
139+
// Since the custom hook contains only unnamed hooks, the source code should not be loaded.
140140
if (request.url.endsWith('useCustom.js')) {
141141
throw Error(`Unexpected file request for "${request.url}"`);
142142
}

packages/react-devtools-shared/src/hooks/parseHookNames/loadSourceAndMetadata.js

Lines changed: 49 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,14 @@ function extractAndLoadSourceMapJSON(
148148
// Deduplicate fetches, since there can be multiple location keys per source map.
149149
const dedupedFetchPromises = new Map();
150150

151+
if (__DEBUG__) {
152+
console.log(
153+
'extractAndLoadSourceMapJSON() load',
154+
locationKeyToHookSourceAndMetadata.size,
155+
'source maps',
156+
);
157+
}
158+
151159
const setterPromises = [];
152160
locationKeyToHookSourceAndMetadata.forEach(hookSourceAndMetadata => {
153161
const sourceMapRegex = / ?sourceMappingURL=([^\s'"]+)/gm;
@@ -177,45 +185,52 @@ function extractAndLoadSourceMapJSON(
177185
const sourceMappingURL = sourceMappingURLMatch[1];
178186
const hasInlineSourceMap = sourceMappingURL.indexOf('base64,') >= 0;
179187
if (hasInlineSourceMap) {
180-
// TODO (named hooks) deduplicate parsing in this branch (similar to fetching in the other branch)
181-
// since there can be multiple location keys per source map.
182-
183-
// Web apps like Code Sandbox embed multiple inline source maps.
184-
// In this case, we need to loop through and find the right one.
185-
// We may also need to trim any part of this string that isn't based64 encoded data.
186-
const trimmed = ((sourceMappingURL.match(
187-
/base64,([a-zA-Z0-9+\/=]+)/,
188-
): any): Array<string>)[1];
189-
const decoded = withSyncPerformanceMark('decodeBase64String()', () =>
190-
decodeBase64String(trimmed),
191-
);
192-
193-
const sourceMapJSON = withSyncPerformanceMark(
194-
'JSON.parse(decoded)',
195-
() => JSON.parse(decoded),
196-
);
188+
try {
189+
// TODO (named hooks) deduplicate parsing in this branch (similar to fetching in the other branch)
190+
// since there can be multiple location keys per source map.
191+
192+
// Web apps like Code Sandbox embed multiple inline source maps.
193+
// In this case, we need to loop through and find the right one.
194+
// We may also need to trim any part of this string that isn't based64 encoded data.
195+
const trimmed = ((sourceMappingURL.match(
196+
/base64,([a-zA-Z0-9+\/=]+)/,
197+
): any): Array<string>)[1];
198+
const decoded = withSyncPerformanceMark(
199+
'decodeBase64String()',
200+
() => decodeBase64String(trimmed),
201+
);
197202

198-
if (__DEBUG__) {
199-
console.groupCollapsed(
200-
'extractAndLoadSourceMapJSON() Inline source map',
203+
const sourceMapJSON = withSyncPerformanceMark(
204+
'JSON.parse(decoded)',
205+
() => JSON.parse(decoded),
201206
);
202-
console.log(sourceMapJSON);
203-
console.groupEnd();
204-
}
205207

206-
// Hook source might be a URL like "https://4syus.csb.app/src/App.js"
207-
// Parsed source map might be a partial path like "src/App.js"
208-
if (sourceMapIncludesSource(sourceMapJSON, runtimeSourceURL)) {
209-
hookSourceAndMetadata.sourceMapJSON = sourceMapJSON;
208+
if (__DEBUG__) {
209+
console.groupCollapsed(
210+
'extractAndLoadSourceMapJSON() Inline source map',
211+
);
212+
console.log(sourceMapJSON);
213+
console.groupEnd();
214+
}
215+
216+
// Hook source might be a URL like "https://4syus.csb.app/src/App.js"
217+
// Parsed source map might be a partial path like "src/App.js"
218+
if (sourceMapIncludesSource(sourceMapJSON, runtimeSourceURL)) {
219+
hookSourceAndMetadata.sourceMapJSON = sourceMapJSON;
210220

211-
// OPTIMIZATION If we've located a source map for this source,
212-
// we'll use it to retrieve the original source (to extract hook names).
213-
// We only fall back to parsing the full source code is when there's no source map.
214-
// The source is (potentially) very large,
215-
// So we can avoid the overhead of serializing it unnecessarily.
216-
hookSourceAndMetadata.runtimeSourceCode = null;
221+
// OPTIMIZATION If we've located a source map for this source,
222+
// we'll use it to retrieve the original source (to extract hook names).
223+
// We only fall back to parsing the full source code is when there's no source map.
224+
// The source is (potentially) very large,
225+
// So we can avoid the overhead of serializing it unnecessarily.
226+
hookSourceAndMetadata.runtimeSourceCode = null;
217227

218-
break;
228+
break;
229+
}
230+
} catch (error) {
231+
// We've likely encountered a string in the source code that looks like a source map but isn't.
232+
// Maybe the source code contains a "sourceMappingURL" comment or soething similar.
233+
// In either case, let's skip this and keep looking.
219234
}
220235
} else {
221236
externalSourceMapURLs.push(sourceMappingURL);

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

Lines changed: 23 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -116,28 +116,34 @@ export function inspectElement(
116116
return null;
117117
}
118118

119-
inspectElementMutableSource({
119+
const onSuccess = ([inspectedElement: InspectedElementFrontend]) => {
120+
const resolvedRecord = ((newRecord: any): ResolvedRecord<InspectedElementFrontend>);
121+
resolvedRecord.status = Resolved;
122+
resolvedRecord.value = inspectedElement;
123+
wake();
124+
};
125+
126+
const onError = error => {
127+
if (newRecord.status === Pending) {
128+
const rejectedRecord = ((newRecord: any): RejectedRecord);
129+
rejectedRecord.status = Rejected;
130+
rejectedRecord.value = `Could not inspect element with id "${element.id}". Error thrown:\n${error.message}`;
131+
wake();
132+
}
133+
};
134+
135+
const thenable = inspectElementMutableSource({
120136
bridge,
121137
element,
122138
path,
123139
rendererID: ((rendererID: any): number),
124-
}).then(
125-
([inspectedElement: InspectedElementFrontend]) => {
126-
const resolvedRecord = ((newRecord: any): ResolvedRecord<InspectedElementFrontend>);
127-
resolvedRecord.status = Resolved;
128-
resolvedRecord.value = inspectedElement;
129-
wake();
130-
},
140+
});
141+
thenable.then(onSuccess, onError);
142+
143+
if (typeof (thenable: any).catch === 'function') {
144+
(thenable: any).catch();
145+
}
131146

132-
error => {
133-
if (newRecord.status === Pending) {
134-
const rejectedRecord = ((newRecord: any): RejectedRecord);
135-
rejectedRecord.status = Rejected;
136-
rejectedRecord.value = `Could not inspect element with id "${element.id}". Error thrown:\n${error.message}`;
137-
wake();
138-
}
139-
},
140-
);
141147
map.set(element, record);
142148
}
143149

0 commit comments

Comments
 (0)