Browse Source

ref: prefer for..of loops over indexed access (#84307)

This PR refactors traditional for loops which use an indexed access to
`for..of` loops, if the index was only used to access the item of the
collection that was iterated over.

`for..of` loops plays better with `noUncheckedIndexedAccess`, as they
avoids a no-null-assertion (!)

The [prefer-for-of](https://typescript-eslint.io/rules/prefer-for-of/)
rule of `typescript-eslint` was also enabled.

Note: These changes should be _mostly_ safe. There’s a slim chance that
we were looping over a non-iterable, like an object that had a `length`
property. [In one
case](https://github.com/getsentry/sentry/pull/84307/files#diff-4c51124da5e714594585261c1e0b0f02f205ef938748070260999a9588fd57c0R426-R427),
the types disallowed `for..of` because the element is not an `iterable`,
only an `indexable`. Iterating over a non-iterable will yield a runtime
error.

IF we had something that was `any` on type level, or was asserted to be
an array, when it was in fact an `indexable` at runtime, we would now
get an error with `for..of`.
Dominik Dorfmeister 1 month ago
parent
commit
6dda45322e

+ 0 - 1
eslint.config.mjs

@@ -432,7 +432,6 @@ export default typescript.config([
       '@typescript-eslint/consistent-type-definitions': 'off', // TODO(ryan953): Fix violations and delete this line
       '@typescript-eslint/no-empty-function': 'off', // TODO(ryan953): Fix violations and delete this line
       '@typescript-eslint/no-inferrable-types': 'off', // TODO(ryan953): Fix violations and delete this line
-      '@typescript-eslint/prefer-for-of': 'off', // TODO(ryan953): Fix violations and delete this line
 
       // Customization
       '@typescript-eslint/no-unused-vars': [

+ 3 - 3
scripts/extract-ios-device-names.ts

@@ -51,9 +51,9 @@ async function collectDefinitions(files: string[]): Promise<Mapping> {
       continue;
     }
 
-    for (let i = 0; i < content.length; i++) {
-      if (content[i].Identifier) {
-        definitions[content[i].Identifier] = content[i].Generation;
+    for (const c of content) {
+      if (c.Identifier) {
+        definitions[c.Identifier] = c.Generation;
       }
     }
   }

+ 4 - 5
static/app/components/events/interfaces/analyzeFrames.tsx

@@ -170,12 +170,11 @@ function satisfiesFunctionCondition(frame: Frame, suspect: SuspectFrame) {
   if (frame.function === null || frame.function === undefined) {
     return false;
   }
-  for (let index = 0; index < suspect.functions.length; index++) {
-    const matchFuction = suspect.functions[index]!;
+  for (const matchFunction of suspect.functions) {
     const match =
-      typeof matchFuction === 'string'
-        ? frame.function === matchFuction
-        : matchFuction.test(frame.function);
+      typeof matchFunction === 'string'
+        ? frame.function === matchFunction
+        : matchFunction.test(frame.function);
     if (match) {
       return true;
     }

+ 2 - 2
static/app/components/events/interfaces/spans/spanProfileDetails.tsx

@@ -115,8 +115,8 @@ export function useSpanProfileDetails(
   const maxNodes = useMemo(() => {
     // find the number of nodes with the minimum number of samples
     let hasMinCount = 0;
-    for (let i = 0; i < nodes.length; i++) {
-      if (nodes[i]!.count >= TOP_NODE_MIN_COUNT) {
+    for (const node of nodes) {
+      if (node.count >= TOP_NODE_MIN_COUNT) {
         hasMinCount += 1;
       } else {
         break;

+ 5 - 5
static/app/components/events/interfaces/spans/useSpanWaterfallModelFromTransaction.tsx

@@ -47,9 +47,9 @@ export function useSpanWaterfallModelFromTransaction(
   const totalCount: number = useMemo(() => {
     if (defined(data)) {
       const spans = Object.values(data);
-      for (let index = 0; index < spans.length; index++) {
-        if (spans[index].is_segment) {
-          return spans[index]['count()'];
+      for (const span of spans) {
+        if (span.is_segment) {
+          return span['count()'];
         }
       }
     }
@@ -60,8 +60,8 @@ export function useSpanWaterfallModelFromTransaction(
     const spanList_: AggregateSpanType[] = [];
     if (defined(data)) {
       const spans = Object.values(data);
-      for (let index = 0; index < spans.length; index++) {
-        const node = formatSpan(spans[index], totalCount);
+      for (const span of spans) {
+        const node = formatSpan(span, totalCount);
         if (node.is_segment === 1) {
           spanList_.unshift(node);
         } else {

+ 1 - 2
static/app/components/events/viewHierarchy/utils.tsx

@@ -113,8 +113,7 @@ export function getDeepestNodeAtPoint(
 
   vec2.scale(point, point, scale);
   const transformedPoint = vec2.transformMat3(vec2.create(), point, inverseMatrix);
-  for (let i = 0; i < nodes.length; i++) {
-    const node = nodes[i]!;
+  for (const node of nodes) {
     if (node.rect.contains(transformedPoint)) {
       clickedNode = node;
     }

+ 3 - 13
static/app/components/events/viewHierarchy/wireframe.tsx

@@ -142,19 +142,9 @@ function Wireframe({hierarchy, selectedNode, onNodeSelect, project}: WireframePr
         canvas.fillStyle = theme.gray100;
         canvas.strokeStyle = theme.gray300;
 
-        for (let i = 0; i < hierarchyData.nodes.length; i++) {
-          canvas.strokeRect(
-            hierarchyData.nodes[i]!.rect.x,
-            hierarchyData.nodes[i]!.rect.y,
-            hierarchyData.nodes[i]!.rect.width,
-            hierarchyData.nodes[i]!.rect.height
-          );
-          canvas.fillRect(
-            hierarchyData.nodes[i]!.rect.x,
-            hierarchyData.nodes[i]!.rect.y,
-            hierarchyData.nodes[i]!.rect.width,
-            hierarchyData.nodes[i]!.rect.height
-          );
+        for (const node of hierarchyData.nodes) {
+          canvas.strokeRect(node.rect.x, node.rect.y, node.rect.width, node.rect.height);
+          canvas.fillRect(node.rect.x, node.rect.y, node.rect.width, node.rect.height);
         }
       }
     },

+ 2 - 2
static/app/components/profiling/flamegraph/continuousFlamegraph.tsx

@@ -214,8 +214,8 @@ function findLongestMatchingFrame(
       longestFrame = frame;
     }
 
-    for (let i = 0; i < frame.children.length; i++) {
-      frames.push(frame.children[i]!);
+    for (const child of frame.children) {
+      frames.push(child);
     }
   }
 

+ 6 - 11
static/app/components/profiling/flamegraph/flamegraph.tsx

@@ -129,9 +129,7 @@ function convertProfileMeasurementsToUIFrames(
     values: [],
   };
 
-  for (let i = 0; i < measurement.values.length; i++) {
-    const value = measurement.values[i]!;
-
+  for (const value of measurement.values) {
     measurements.values.push({
       elapsed: value.elapsed_since_start_ns,
       value: value.value,
@@ -170,8 +168,8 @@ function findLongestMatchingFrame(
       longestFrame = frame;
     }
 
-    for (let i = 0; i < frame.children.length; i++) {
-      frames.push(frame.children[i]!);
+    for (const child of frame.children) {
+      frames.push(child);
     }
   }
 
@@ -445,8 +443,7 @@ function Flamegraph(): ReactElement {
         const measurements = profileGroup.measurements[key]!;
         const values: ProfileSeriesMeasurement['values'] = [];
 
-        for (let i = 0; i < measurements.values.length; i++) {
-          const value = measurements.values[i]!;
+        for (const value of measurements.values) {
           values.push({
             value: value.value,
             elapsed: value.elapsed_since_start_ns,
@@ -474,8 +471,7 @@ function Flamegraph(): ReactElement {
     if (memory_footprint) {
       const values: ProfileSeriesMeasurement['values'] = [];
 
-      for (let i = 0; i < memory_footprint.values.length; i++) {
-        const value = memory_footprint.values[i]!;
+      for (const value of memory_footprint.values) {
         values.push({
           value: value.value,
           elapsed: value.elapsed_since_start_ns,
@@ -493,8 +489,7 @@ function Flamegraph(): ReactElement {
     if (native_memory_footprint) {
       const values: ProfileSeriesMeasurement['values'] = [];
 
-      for (let i = 0; i < native_memory_footprint.values.length; i++) {
-        const value = native_memory_footprint.values[i]!;
+      for (const value of native_memory_footprint.values) {
         values.push({
           value: value.value,
           elapsed: value.elapsed_since_start_ns,

+ 2 - 2
static/app/components/profiling/flamegraph/flamegraphPreview.tsx

@@ -331,8 +331,8 @@ export function computePreviewConfigView(
       }
     }
 
-    for (let i = 0; i < frame.children.length; i++) {
-      frames.push(frame.children[i]!);
+    for (const child of frame.children) {
+      frames.push(child);
     }
   }
 

Some files were not shown because too many files changed in this diff