refactor[react-devtools-shared]: minor parsing improvements and modif…

…ications (facebook#27661) Had these stashed for some time, it includes: - Some refactoring to remove unnecessary `FlowFixMe`s and type castings via `any`. - Optimized version of parsing component names. We encode string names to utf8 and then pass it serialized from backend to frontend in a single array of numbers. Previously we would call `slice` to get the corresponding encoded string as a subarray and then parse each character. New implementation skips `slice` step and just receives `left` and `right` ranges for the string to parse. - Early `break` instead of `continue` when Store receives unexpected operation, like removing an element from the Store, which is not registered yet.
EdisonVan · Apr 15, 2024 · 4947795 · 4947795
1 parent 71ef6c9
commit 4947795
Show file tree

Hide file tree

Showing 6 changed files with 106 additions and 65 deletions.
diff --git a/packages/react-devtools-shared/src/backend/renderer.js b/packages/react-devtools-shared/src/backend/renderer.js
@@ -439,7 +439,6 @@ export function getInternalReactConstants(version: string): {
         return 'Cache';
       case ClassComponent:
       case IncompleteClassComponent:
-        return getDisplayName(resolvedType);
       case FunctionComponent:
       case IndeterminateComponent:
         return getDisplayName(resolvedType);

diff --git a/packages/react-devtools-shared/src/devtools/store.js b/packages/react-devtools-shared/src/devtools/store.js
@@ -27,7 +27,7 @@ import {
   setSavedComponentFilters,
   separateDisplayNameAndHOCs,
   shallowDiffers,
-  utfDecodeString,
+  utfDecodeStringWithRanges,
 } from '../utils';
 import {localStorageGetItem, localStorageSetItem} from '../storage';
 import {__DEBUG__} from '../constants';
@@ -450,7 +450,7 @@ export default class Store extends EventEmitter<{
   }
 
   // This build of DevTools supports the legacy profiler.
-  // This is a static flag, controled by the Store config.
+  // This is a static flag, controlled by the Store config.
   get supportsProfiling(): boolean {
     return this._supportsProfiling;
   }
@@ -467,7 +467,7 @@ export default class Store extends EventEmitter<{
   }
 
   // This build of DevTools supports the Timeline profiler.
-  // This is a static flag, controled by the Store config.
+  // This is a static flag, controlled by the Store config.
   get supportsTimeline(): boolean {
     return this._supportsTimeline;
   }
@@ -502,30 +502,58 @@ export default class Store extends EventEmitter<{
     }
 
     // Find which root this element is in...
-    let rootID;
     let root;
     let rootWeight = 0;
     for (let i = 0; i < this._roots.length; i++) {
-      rootID = this._roots[i];
-      root = ((this._idToElement.get(rootID): any): Element);
+      const rootID = this._roots[i];
+      root = this._idToElement.get(rootID);
+
+      if (root === undefined) {
+        this._throwAndEmitError(
+          Error(
+            `Couldn't find root with id "${rootID}": no matching node was found in the Store.`,
+          ),
+        );
+
+        return null;
+      }
+
       if (root.children.length === 0) {
         continue;
-      } else if (rootWeight + root.weight > index) {
+      }
+
+      if (rootWeight + root.weight > index) {
         break;
       } else {
         rootWeight += root.weight;
       }
     }
 
+    if (root === undefined) {
+      return null;
+    }
+
     // Find the element in the tree using the weight of each node...
     // Skip over the root itself, because roots aren't visible in the Elements tree.
-    let currentElement = ((root: any): Element);
+    let currentElement: Element = root;
     let currentWeight = rootWeight - 1;
+
     while (index !== currentWeight) {
       const numChildren = currentElement.children.length;
       for (let i = 0; i < numChildren; i++) {
         const childID = currentElement.children[i];
-        const child = ((this._idToElement.get(childID): any): Element);
+        const child = this._idToElement.get(childID);
+
+        if (child === undefined) {
+          this._throwAndEmitError(
+            Error(
+              `Couldn't child element with id "${childID}": no matching node was found in the Store.`,
+            ),
+          );
+
+          return null;
+        }
+
         const childWeight = child.isCollapsed ? 1 : child.weight;
 
         if (index <= currentWeight + childWeight) {
@@ -538,7 +566,7 @@ export default class Store extends EventEmitter<{
       }
     }
 
-    return ((currentElement: any): Element) || null;
+    return currentElement || null;
   }
 
   getElementIDAtIndex(index: number): number | null {
@@ -560,32 +588,31 @@ export default class Store extends EventEmitter<{
   getElementsWithErrorsAndWarnings(): Array<{id: number, index: number}> {
     if (this._cachedErrorAndWarningTuples !== null) {
       return this._cachedErrorAndWarningTuples;
-    } else {
-      const errorAndWarningTuples: ErrorAndWarningTuples = [];
-
-      this._errorsAndWarnings.forEach((_, id) => {
-        const index = this.getIndexOfElementID(id);
-        if (index !== null) {
-          let low = 0;
-          let high = errorAndWarningTuples.length;
-          while (low < high) {
-            const mid = (low + high) >> 1;
-            if (errorAndWarningTuples[mid].index > index) {
-              high = mid;
-            } else {
-              low = mid + 1;
-            }
-          }
+    }
 
-          errorAndWarningTuples.splice(low, 0, {id, index});
+    const errorAndWarningTuples: ErrorAndWarningTuples = [];
+
+    this._errorsAndWarnings.forEach((_, id) => {
+      const index = this.getIndexOfElementID(id);
+      if (index !== null) {
+        let low = 0;
+        let high = errorAndWarningTuples.length;
+        while (low < high) {
+          const mid = (low + high) >> 1;
+          if (errorAndWarningTuples[mid].index > index) {
+            high = mid;
+          } else {
+            low = mid + 1;
+          }
         }
-      });
 
-      // Cache for later (at least until the tree changes again).
-      this._cachedErrorAndWarningTuples = errorAndWarningTuples;
+        errorAndWarningTuples.splice(low, 0, {id, index});
+      }
+    });
 
-      return errorAndWarningTuples;
-    }
+    // Cache for later (at least until the tree changes again).
+    this._cachedErrorAndWarningTuples = errorAndWarningTuples;
+    return errorAndWarningTuples;
   }
 
   getErrorAndWarningCountForElementID(id: number): {
@@ -923,7 +950,11 @@ export default class Store extends EventEmitter<{
       const nextLength = operations[i];
       i++;
 
-      const nextString = utfDecodeString(operations.slice(i, i + nextLength));
+      const nextString = utfDecodeStringWithRanges(
+        operations,
+        i,
+        i + nextLength - 1,
+      );
       stringTable.push(nextString);
       i += nextLength;
     }
@@ -1035,7 +1066,7 @@ export default class Store extends EventEmitter<{
                 ),
               );
 
-              continue;
+              break;
             }
 
             parentElement.children.push(id);
@@ -1088,7 +1119,7 @@ export default class Store extends EventEmitter<{
                 ),
               );
 
-              continue;
+              break;
             }
 
             i += 1;
@@ -1126,7 +1157,7 @@ export default class Store extends EventEmitter<{
                   ),
                 );
 
-                continue;
+                break;
               }
 
               const index = parentElement.children.indexOf(id);
@@ -1172,7 +1203,17 @@ export default class Store extends EventEmitter<{
             }
           };
 
-          const root = ((this._idToElement.get(id): any): Element);
+          const root = this._idToElement.get(id);
+          if (root === undefined) {
+            this._throwAndEmitError(
+              Error(
+                `Cannot remove root "${id}": no matching node was found in the Store.`,
+              ),
+            );
+
+            break;
+          }
+
           recursivelyDeleteElements(id);
 
           this._rootIDToCapabilities.delete(id);
@@ -1194,7 +1235,7 @@ export default class Store extends EventEmitter<{
               ),
             );
 
-            continue;
+            break;
           }
 
           const children = element.children;
@@ -1279,7 +1320,7 @@ export default class Store extends EventEmitter<{
 
     this._revision++;
 
-    // Any time the tree changes (e.g. elements added, removed, or reordered) cached inidices may be invalid.
+    // Any time the tree changes (e.g. elements added, removed, or reordered) cached indices may be invalid.
     this._cachedErrorAndWarningTuples = null;
 
     if (haveErrorsOrWarningsChanged) {

diff --git a/packages/react-devtools-shared/src/devtools/views/Components/Element.js b/packages/react-devtools-shared/src/devtools/views/Components/Element.js
@@ -122,7 +122,7 @@ export default function Element({data, index, style}: Props): React.Node {
     isStrictModeNonCompliant,
     key,
     type,
-  } = ((element: any): ElementType);
+  } = element;
 
   // Only show strict mode non-compliance badges for top level elements.
   // Showing an inline badge for every element in the tree would be noisy.
@@ -173,17 +173,16 @@ export default function Element({data, index, style}: Props): React.Node {
             "
           </Fragment>
         )}
+
         {hocDisplayNames !== null && hocDisplayNames.length > 0 ? (
           <Badge
             className={styles.Badge}
             hocDisplayNames={hocDisplayNames}
             type={type}>
-            <DisplayName
-              displayName={hocDisplayNames[0]}
-              id={((id: any): number)}
-            />
+            <DisplayName displayName={hocDisplayNames[0]} id={id} />
           </Badge>
         ) : null}
+
         {showInlineWarningsAndErrors && errorCount > 0 && (
           <Icon
             type="error"

diff --git a/packages/react-devtools-shared/src/devtools/views/Components/HocBadges.js b/packages/react-devtools-shared/src/devtools/views/Components/HocBadges.js
@@ -25,12 +25,11 @@ export default function HocBadges({element}: Props): React.Node {
 
   return (
     <div className={styles.HocBadges}>
-      {hocDisplayNames !== null &&
-        hocDisplayNames.map(hocDisplayName => (
-          <div key={hocDisplayName} className={styles.Badge}>
-            {hocDisplayName}
-          </div>
-        ))}
+      {hocDisplayNames.map(hocDisplayName => (
+        <div key={hocDisplayName} className={styles.Badge}>
+          {hocDisplayName}
+        </div>
+      ))}
     </div>
   );
 }
diff --git a/packages/react-devtools-shared/src/devtools/views/Profiler/CommitTreeBuilder.js b/packages/react-devtools-shared/src/devtools/views/Profiler/CommitTreeBuilder.js
@@ -17,7 +17,7 @@ import {
   TREE_OPERATION_UPDATE_TREE_BASE_DURATION,
   TREE_OPERATION_UPDATE_ERRORS_OR_WARNINGS,
 } from 'react-devtools-shared/src/constants';
-import {utfDecodeString} from 'react-devtools-shared/src/utils';
+import {utfDecodeStringWithRanges} from 'react-devtools-shared/src/utils';
 import {ElementTypeRoot} from 'react-devtools-shared/src/frontend/types';
 import ProfilerStore from 'react-devtools-shared/src/devtools/ProfilerStore';
 
@@ -170,8 +170,10 @@ function updateTree(
   const stringTableEnd = i + stringTableSize;
   while (i < stringTableEnd) {
     const nextLength = operations[i++];
-    const nextString = utfDecodeString(
-      (operations.slice(i, i + nextLength): any),
+    const nextString = utfDecodeStringWithRanges(
+      operations,
+      i,
+      i + nextLength - 1,
     );
     stringTable.push(nextString);
     i += nextLength;

diff --git a/packages/react-devtools-shared/src/utils.js b/packages/react-devtools-shared/src/utils.js
@@ -116,7 +116,7 @@ export function getWrappedDisplayName(
   wrapperName: string,
   fallbackName?: string,
 ): string {
-  const displayName = (outerType: any).displayName;
+  const displayName = (outerType: any)?.displayName;
   return (
     displayName || `${wrapperName}(${getDisplayName(innerType, fallbackName)})`
   );
@@ -152,15 +152,14 @@ export function getUID(): number {
   return ++uidCounter;
 }
 
-export function utfDecodeString(array: Array<number>): string {
-  // Avoid spreading the array (e.g. String.fromCodePoint(...array))
-  // Functions arguments are first placed on the stack before the function is called
-  // which throws a RangeError for large arrays.
-  // See github.com/facebook/react/issues/22293
+export function utfDecodeStringWithRanges(
+  array: Array<number>,
+  left: number,
+  right: number,
+): string {
   let string = '';
-  for (let i = 0; i < array.length; i++) {
-    const char = array[i];
-    string += String.fromCodePoint(char);
+  for (let i = left; i <= right; i++) {
+    string += String.fromCodePoint(array[i]);
   }
   return string;
 }
@@ -216,8 +215,10 @@ export function printOperationsArray(operations: Array<number>) {
   const stringTableEnd = i + stringTableSize;
   while (i < stringTableEnd) {
     const nextLength = operations[i++];
-    const nextString = utfDecodeString(
-      (operations.slice(i, i + nextLength): any),
+    const nextString = utfDecodeStringWithRanges(
+      operations,
+      i,
+      i + nextLength - 1,
     );
     stringTable.push(nextString);
     i += nextLength;