Added an LRU cache and eviction strategy for source code/ASTs

Author: Brian Vaughn

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

@@ -266,7 +266,7 @@ describe('parseHookNames', () => {
       ); // external source map
     });
 
-    // TODO (named hooks) Inline require (e.g. require("react").useState()) isn't supported yet
+    // TODO Inline require (e.g. require("react").useState()) isn't supported yet.
     // Maybe this isn't an important use case to support,
     // since inline requires are most likely to exist in compiled source (if at all).
     xit('should work for inline requires', async () => {

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

@@ -69,14 +69,6 @@ function getFilteredHookASTNodes(
   potentialHooksFound: Array<NodePath>,
   source: string,
 ): Array<NodePath> {
-  // Remove targetHook from potentialHooks array since its purpose is served.
-  // Also to clean the potentialHooks array for further filtering member nodes down the line.
-  const hookIdx = potentialHooksFound.indexOf(potentialReactHookASTNode);
-  if (hookIdx !== -1) {
-    // TODO (named hooks) Should we be doing this? Is it necessary?
-    // potentialHooksFound.splice(hookIdx, 1);
-  }
-
   let nodesAssociatedWithReactHookASTNode: NodePath[] = [];
   if (nodeContainsHookVariableName(potentialReactHookASTNode)) {
     // made custom hooks to enter this, always
@@ -275,7 +267,9 @@ function isHook(node: Node): boolean {
       isPascalCaseNameSpace.test(obj.name)
     );
   } else {
-    // TODO (named hooks) Handle inline require statements e.g. require("useStable")(...)
+    // TODO Possibly handle inline require statements e.g. require("useStable")(...)
+    // This does not seem like a high priority, since inline requires are probably
+    // not common and are also typically in compiled code rather than source code.
 
     return false;
   }

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

@@ -11,6 +11,7 @@
 
 import {parse} from '@babel/parser';
 import {enableHookNameParsing} from 'react-devtools-feature-flags';
+import LRU from 'lru-cache';
 import {SourceMapConsumer} from 'source-map';
 import {getHookName, isNonDeclarativePrimitiveHook} from './astUtils';
 import {sourceMapsAreAppliedToErrors} from './ErrorTester';
@@ -21,20 +22,22 @@ import type {
   HookSource,
   HooksTree,
 } from 'react-debug-tools/src/ReactDebugHooks';
-import type {HookNames} from 'react-devtools-shared/src/types';
+import type {HookNames, LRUCache} from 'react-devtools-shared/src/types';
 import type {Thenable} from 'shared/ReactTypes';
 import type {SourceConsumer, SourceMap} from './astUtils';
 
 const SOURCE_MAP_REGEX = / ?sourceMappingURL=([^\s'"]+)/gm;
 const ABSOLUTE_URL_REGEX = /^https?:\/\//i;
 const MAX_SOURCE_LENGTH = 100_000_000;
 
+type AST = mixed;
+
 type HookSourceData = {|
   // Generated by react-debug-tools.
   hookSource: HookSource,
 
   // AST for original source code; typically comes from a consumed source map.
-  originalSourceAST: mixed,
+  originalSourceAST: AST | null,
 
   // Source code (React components or custom hooks) containing primitive hook calls.
   // If no source map has been provided, this code will be the same as runtimeSourceCode.
@@ -54,6 +57,32 @@ type HookSourceData = {|
   sourceMapContents: SourceMap | null,
 |};
 
+type CachedMetadata = {|
+  originalSourceAST: AST,
+  originalSourceCode: string,
+  sourceConsumer: SourceConsumer | null,
+|};
+
+// On large trees, encoding takes significant time.
+// Try to reuse the already encoded strings.
+const fileNameToMetadataCache: LRUCache<string, CachedMetadata> = new LRU({
+  max: 50,
+  dispose: (fileName: string, metadata: CachedMetadata) => {
+    if (__DEBUG__) {
+      console.log(
+        'fileNameToHookSourceData.dispose() Evicting cached metadata for "' +
+          fileName +
+          '"',
+      );
+    }
+
+    const sourceConsumer = metadata.sourceConsumer;
+    if (sourceConsumer !== null) {
+      sourceConsumer.destroy();
+    }
+  },
+});
+
 export default async function parseHookNames(
   hooksTree: HooksTree,
 ): Thenable<HookNames | null> {
@@ -85,28 +114,44 @@ export default async function parseHookNames(
       throw Error('Hook source code location not found.');
     } else {
       if (!fileNameToHookSourceData.has(fileName)) {
-        fileNameToHookSourceData.set(fileName, {
+        const hookSourceData: HookSourceData = {
           hookSource,
           originalSourceAST: null,
           originalSourceCode: null,
           runtimeSourceCode: null,
           sourceConsumer: null,
           sourceMapURL: null,
           sourceMapContents: null,
-        });
+        };
+
+        // If we've already loaded source/source map info for this file,
+        // we can skip reloading it (and more importantly, re-parsing it).
+        const metadata = fileNameToMetadataCache.get(fileName);
+        if (metadata != null) {
+          if (__DEBUG__) {
+            console.groupCollapsed(
+              'parseHookNames() Found cached metadata for file "' +
+                fileName +
+                '"',
+            );
+            console.log(metadata);
+            console.groupEnd();
+          }
+
+          hookSourceData.originalSourceAST = metadata.originalSourceAST;
+          hookSourceData.originalSourceCode = metadata.originalSourceCode;
+          hookSourceData.sourceConsumer = metadata.sourceConsumer;
+        }
+
+        fileNameToHookSourceData.set(fileName, hookSourceData);
       }
     }
   }
 
-  // TODO (named hooks) Call .destroy() on SourceConsumers after we're done to free up memory.
-
-  // TODO (named hooks) Replace Array of hook names with a Map() of hook ID to name to better support mapping nested hook to name.
-  // This is a little tricky though, since custom hooks don't have IDs.
-  // We may need to add an ID for these too, in the backend?
-
   return loadSourceFiles(fileNameToHookSourceData)
     .then(() => extractAndLoadSourceMaps(fileNameToHookSourceData))
     .then(() => parseSourceAST(fileNameToHookSourceData))
+    .then(() => updateLruCache(fileNameToHookSourceData))
     .then(() => findHookNames(hooksList, fileNameToHookSourceData));
 }
 
@@ -129,6 +174,11 @@ function extractAndLoadSourceMaps(
 ): Promise<*> {
   const promises = [];
   fileNameToHookSourceData.forEach(hookSourceData => {
+    if (hookSourceData.originalSourceAST !== null) {
+      // Use cached metadata.
+      return;
+    }
+
     const runtimeSourceCode = ((hookSourceData.runtimeSourceCode: any): string);
     const sourceMappingURLs = runtimeSourceCode.match(SOURCE_MAP_REGEX);
     if (sourceMappingURLs == null) {
@@ -360,6 +410,11 @@ async function parseSourceAST(
 
   const promises = [];
   fileNameToHookSourceData.forEach(hookSourceData => {
+    if (hookSourceData.originalSourceAST !== null) {
+      // Use cached metadata.
+      return;
+    }
+
     const {runtimeSourceCode, sourceMapContents} = hookSourceData;
     if (sourceMapContents !== null) {
       // Parse and extract the AST from the source map.
@@ -386,9 +441,9 @@ async function parseSourceAST(
               console.groupEnd();
             }
 
-            // Save the original source and parsed AST for later.
-            // TODO (named hooks) Cache this across components, per source/file name.
             hookSourceData.originalSourceCode = originalSourceCode;
+
+            // TODO Parsing should ideally be done off of the main thread.
             hookSourceData.originalSourceAST = parse(originalSourceCode, {
               sourceType: 'unambiguous',
               plugins: ['jsx', 'typescript'],
@@ -399,6 +454,8 @@ async function parseSourceAST(
     } else {
       // There's no source map to parse here so we can just parse the original source itself.
       hookSourceData.originalSourceCode = runtimeSourceCode;
+
+      // TODO Parsing should ideally be done off of the main thread.
       hookSourceData.originalSourceAST = parse(runtimeSourceCode, {
         sourceType: 'unambiguous',
         plugins: ['jsx', 'typescript'],
@@ -420,3 +477,25 @@ function flattenHooksList(
     }
   }
 }
+
+function updateLruCache(
+  fileNameToHookSourceData: Map<string, HookSourceData>,
+): Promise<*> {
+  fileNameToHookSourceData.forEach(
+    ({originalSourceAST, originalSourceCode, sourceConsumer}, fileName) => {
+      // Only set once to avoid triggering eviction/cleanup code.
+      if (!fileNameToMetadataCache.has(fileName)) {
+        if (__DEBUG__) {
+          console.log('updateLruCache() Caching metada for "' + fileName + '"');
+        }
+
+        fileNameToMetadataCache.set(fileName, {
+          originalSourceAST,
+          originalSourceCode: ((originalSourceCode: any): string),
+          sourceConsumer,
+        });
+      }
+    },
+  );
+  return Promise.resolve();
+}

packages/react-devtools-shared/src/devtools/views/Components/Badge.js

@@ -26,7 +26,7 @@ export default function Badge({
   type,
   children,
 }: Props) {
-  if (hocDisplayNames === null) {
+  if (hocDisplayNames === null || hocDisplayNames.length === 0) {
     return null;
   }
 

packages/react-devtools-shared/src/devtools/views/Components/Components.css

@@ -68,4 +68,5 @@
   justify-content: center;
   font-size: var(--font-size-sans-large);
   color: var(--color-dim);
+  border-left: 1px solid var(--color-border);
 }

packages/react-devtools-shared/src/devtools/views/Components/InspectedElement.css

@@ -64,5 +64,5 @@
   padding: 0.25rem;
   color: var(--color-dimmer);
   font-style: italic;
+  border-left: 1px solid var(--color-border);
 }
-

packages/react-devtools-shared/src/devtools/views/Components/InspectedElementErrorBoundary.css

@@ -1,3 +1,3 @@
 .Wrapper {
-  border-left: 1px solid var(--color-border);
+  height: 100%;
 }

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

@@ -71,11 +71,11 @@ export function loadHookNames(
 
   const map = getRecordMap();
 
-  // TODO (named hooks) Make sure caching layer works and we aren't re-parsing ASTs.
   let record = map.get(element);
   if (record) {
-    // TODO (named hooks) Do we need to update the Map to use new hooks list as keys?
-    // or wil these be stable between inspections as a component updates?
+    // TODO Do we need to update the Map to use new the hooks list objects as keys
+    // or will these be stable between inspections as a component updates?
+    // It seems like they're stable.
   } else {
     const callbacks = new Set();
     const wakeable: Wakeable = {

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

@@ -81,3 +81,9 @@ export type ComponentFilter =
 
 export type HookName = string | null;
 export type HookNames = Map<HooksNode, HookName>;
+
+export type LRUCache<K, V> = {|
+  get: (key: K) => V,
+  has: (key: K) => boolean,
+  set: (key: K, value: V) => void,
+|};

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

@@ -47,13 +47,17 @@ import {
 } from 'react-devtools-shared/src/types';
 import {localStorageGetItem, localStorageSetItem} from './storage';
 import {meta} from './hydration';
+
 import type {ComponentFilter, ElementType} from './types';
+import type {LRUCache} from 'react-devtools-shared/src/types';
 
 const cachedDisplayNames: WeakMap<Function, string> = new WeakMap();
 
 // On large trees, encoding takes significant time.
 // Try to reuse the already encoded strings.
-const encodedStringCache = new LRU({max: 1000});
+const encodedStringCache: LRUCache<string, Array<number>> = new LRU({
+  max: 1000,
+});
 
 export function alphaSortKeys(
   a: string | number | Symbol,