Repository: facebook/react
Pull request: DevTools Suspense cache cleanup

Fixed a few things I noticed while testing #22263.

Commits can be reviewed independently.

Changed files:
filename: packages/react-devtools-shared/src/hookNamesCache.js
patch: @@ -110,44 +110,51 @@ export function loadHookNames(
 
     let didTimeout = false;
 
-    loadHookNamesFunction(hooksTree, fetchFileWithCaching).then(
-      function onSuccess(hookNames) {
-        if (didTimeout) {
-          return;
-        }
-
-        if (__DEBUG__) {
-          console.log('[hookNamesCache] onSuccess() hookNames:', hookNames);
-        }
-
-        if (hookNames) {
-          const resolvedRecord = ((newRecord: any): ResolvedRecord<HookNames>);
-          resolvedRecord.status = Resolved;
-          resolvedRecord.value = hookNames;
-        } else {
-          const notFoundRecord = ((newRecord: any): RejectedRecord);
-          notFoundRecord.status = Rejected;
-          notFoundRecord.value = null;
-        }
-
-        wake();
-      },
-      function onError(error) {
-        if (didTimeout) {
-          return;
-        }
+    const onSuccess = hookNames => {
+      if (didTimeout) {
+        return;
+      }
+
+      if (__DEBUG__) {
+        console.log('[hookNamesCache] onSuccess() hookNames:', hookNames);
+      }
 
-        if (__DEBUG__) {
-          console.log('[hookNamesCache] onError() error:', error);
-        }
+      if (hookNames) {
+        const resolvedRecord = ((newRecord: any): ResolvedRecord<HookNames>);
+        resolvedRecord.status = Resolved;
+        resolvedRecord.value = hookNames;
+      } else {
+        const notFoundRecord = ((newRecord: any): RejectedRecord);
+        notFoundRecord.status = Rejected;
+        notFoundRecord.value = null;
+      }
 
-        const thrownRecord = ((newRecord: any): RejectedRecord);
-        thrownRecord.status = Rejected;
-        thrownRecord.value = null;
+      wake();
+    };
 
-        wake();
-      },
-    );
+    const onError = error => {
+      if (didTimeout) {
+        return;
+      }
+
+      if (__DEBUG__) {
+        console.log('[hookNamesCache] onError()');
+      }
+
+      console.error(error);
+
+      const thrownRecord = ((newRecord: any): RejectedRecord);
+      thrownRecord.status = Rejected;
+      thrownRecord.value = null;
+
+      wake();
+    };
+
+    const thenable = loadHookNamesFunction(hooksTree, fetchFileWithCaching);
+    thenable.then(onSuccess, onError);
+    if (typeof (thenable: any).catch === 'function') {
+      (thenable: any).catch(onError);
+    }
 
     // Eventually timeout and stop trying to load names.
     let timeoutID = setTimeout(function onTimeout() {

filename: packages/react-devtools-shared/src/hooks/__tests__/__source__/ContainingStringSourceMappingURL.js
patch: @@ -11,12 +11,17 @@ import React, {useState} from 'react';
 
 // ?sourceMappingURL=([^\s'"]+)/gm
 
+const abc = 'abc';
+const string =
+  'sourceMappingURL=data:application/json;charset=utf-8;base64,' + abc;
+
 export function Component() {
   const [count, setCount] = useState(0);
 
   return (
     <div>
       <p>You clicked {count} times</p>
+      <div>string: {string}</div>
       <button onClick={() => setCount(count + 1)}>Click me</button>
     </div>
   );

filename: packages/react-devtools-shared/src/hooks/__tests__/parseHookNames-test.js
patch: @@ -136,7 +136,7 @@ describe('parseHookNames', () => {
       .Component;
 
     fetchMock.mockIf(/.+$/, request => {
-      // Since th custom hook contains only unnamed hooks, the source code should not be loaded.
+      // Since the custom hook contains only unnamed hooks, the source code should not be loaded.
       if (request.url.endsWith('useCustom.js')) {
         throw Error(`Unexpected file request for "${request.url}"`);
       }

filename: packages/react-devtools-shared/src/hooks/parseHookNames/loadSourceAndMetadata.js
patch: @@ -148,6 +148,14 @@ function extractAndLoadSourceMapJSON(
   // Deduplicate fetches, since there can be multiple location keys per source map.
   const dedupedFetchPromises = new Map();
 
+  if (__DEBUG__) {
+    console.log(
+      'extractAndLoadSourceMapJSON() load',
+      locationKeyToHookSourceAndMetadata.size,
+      'source maps',
+    );
+  }
+
   const setterPromises = [];
   locationKeyToHookSourceAndMetadata.forEach(hookSourceAndMetadata => {
     const sourceMapRegex = / ?sourceMappingURL=([^\s'"]+)/gm;
@@ -177,45 +185,52 @@ function extractAndLoadSourceMapJSON(
         const sourceMappingURL = sourceMappingURLMatch[1];
         const hasInlineSourceMap = sourceMappingURL.indexOf('base64,') >= 0;
         if (hasInlineSourceMap) {
-          // TODO (named hooks) deduplicate parsing in this branch (similar to fetching in the other branch)
-          // since there can be multiple location keys per source map.
-
-          // Web apps like Code Sandbox embed multiple inline source maps.
-          // In this case, we need to loop through and find the right one.
-          // We may also need to trim any part of this string that isn't based64 encoded data.
-          const trimmed = ((sourceMappingURL.match(
-            /base64,([a-zA-Z0-9+\/=]+)/,
-          ): any): Array<string>)[1];
-          const decoded = withSyncPerformanceMark('decodeBase64String()', () =>
-            decodeBase64String(trimmed),
-          );
-
-          const sourceMapJSON = withSyncPerformanceMark(
-            'JSON.parse(decoded)',
-            () => JSON.parse(decoded),
-          );
+          try {
+            // TODO (named hooks) deduplicate parsing in this branch (similar to fetching in the other branch)
+            // since there can be multiple location keys per source map.
+
+            // Web apps like Code Sandbox embed multiple inline source maps.
+            // In this case, we need to loop through and find the right one.
+            // We may also need to trim any part of this string that isn't based64 encoded data.
+            const trimmed = ((sourceMappingURL.match(
+              /base64,([a-zA-Z0-9+\/=]+)/,
+            ): any): Array<string>)[1];
+            const decoded = withSyncPerformanceMark(
+              'decodeBase64String()',
+              () => decodeBase64String(trimmed),
+            );
 
-          if (__DEBUG__) {
-            console.groupCollapsed(
-              'extractAndLoadSourceMapJSON() Inline source map',
+            const sourceMapJSON = withSyncPerformanceMark(
+              'JSON.parse(decoded)',
+              () => JSON.parse(decoded),
             );
-            console.log(sourceMapJSON);
-            console.groupEnd();
-          }
 
-          // Hook source might be a URL like "https://4syus.csb.app/src/App.js"
-          // Parsed source map might be a partial path like "src/App.js"
-          if (sourceMapIncludesSource(sourceMapJSON, runtimeSourceURL)) {
-            hookSourceAndMetadata.sourceMapJSON = sourceMapJSON;
+            if (__DEBUG__) {
+              console.groupCollapsed(
+                'extractAndLoadSourceMapJSON() Inline source map',
+              );
+              console.log(sourceMapJSON);
+              console.groupEnd();
+            }
+
+            // Hook source might be a URL like "https://4syus.csb.app/src/App.js"
+            // Parsed source map might be a partial path like "src/App.js"
+            if (sourceMapIncludesSource(sourceMapJSON, runtimeSourceURL)) {
+              hookSourceAndMetadata.sourceMapJSON = sourceMapJSON;
 
-            // OPTIMIZATION If we've located a source map for this source,
-            // we'll use it to retrieve the original source (to extract hook names).
-            // We only fall back to parsing the full source code is when there's no source map.
-            // The source is (potentially) very large,
-            // So we can avoid the overhead of serializing it unnecessarily.
-            hookSourceAndMetadata.runtimeSourceCode = null;
+              // OPTIMIZATION If we've located a source map for this source,
+              // we'll use it to retrieve the original source (to extract hook names).
+              // We only fall back to parsing the full source code is when there's no source map.
+              // The source is (potentially) very large,
+              // So we can avoid the overhead of serializing it unnecessarily.
+              hookSourceAndMetadata.runtimeSourceCode = null;
 
-            break;
+              break;
+            }
+          } catch (error) {
+            // We've likely encountered a string in the source code that looks like a source map but isn't.
+            // Maybe the source code contains a "sourceMappingURL" comment or soething similar.
+            // In either case, let's skip this and keep looking.
           }
         } else {
           externalSourceMapURLs.push(sourceMappingURL);

filename: packages/react-devtools-shared/src/inspectedElementCache.js
patch: @@ -116,28 +116,34 @@ export function inspectElement(
       return null;
     }
 
-    inspectElementMutableSource({
+    const onSuccess = ([inspectedElement: InspectedElementFrontend]) => {
+      const resolvedRecord = ((newRecord: any): ResolvedRecord<InspectedElementFrontend>);
+      resolvedRecord.status = Resolved;
+      resolvedRecord.value = inspectedElement;
+      wake();
+    };
+
+    const onError = error => {
+      if (newRecord.status === Pending) {
+        const rejectedRecord = ((newRecord: any): RejectedRecord);
+        rejectedRecord.status = Rejected;
+        rejectedRecord.value = `Could not inspect element with id "${element.id}". Error thrown:\n${error.message}`;
+        wake();
+      }
+    };
+
+    const thenable = inspectElementMutableSource({
       bridge,
       element,
       path,
       rendererID: ((rendererID: any): number),
-    }).then(
-      ([inspectedElement: InspectedElementFrontend]) => {
-        const resolvedRecord = ((newRecord: any): ResolvedRecord<InspectedElementFrontend>);
-        resolvedRecord.status = Resolved;
-        resolvedRecord.value = inspectedElement;
-        wake();
-      },
+    });
+    thenable.then(onSuccess, onError);
+
+    if (typeof (thenable: any).catch === 'function') {
+      (thenable: any).catch();
+    }
 
-      error => {
-        if (newRecord.status === Pending) {
-          const rejectedRecord = ((newRecord: any): RejectedRecord);
-          rejectedRecord.status = Rejected;
-          rejectedRecord.value = `Could not inspect element with id "${element.id}". Error thrown:\n${error.message}`;
-          wake();
-        }
-      },
-    );
     map.set(element, record);
   }