code review

Author: mdjastrzebski

codemods/v14-async-functions/scripts/codemod.ts

@@ -157,7 +157,7 @@ export default async function transform(
     }
 
     if (
-      !isFunctionAlreadyAsync(containingFunction, rootNode) &&
+      !isFunctionAlreadyAsync(containingFunction) &&
       !functionsToMakeAsync.has(containingFunction.id())
     ) {
       functionsToMakeAsync.set(containingFunction.id(), containingFunction);
@@ -179,7 +179,7 @@ export default async function transform(
         }
 
         if (
-          !isFunctionAlreadyAsync(containingFunction, rootNode) &&
+          !isFunctionAlreadyAsync(containingFunction) &&
           !functionsToMakeAsync.has(containingFunction.id())
         ) {
           functionsToMakeAsync.set(containingFunction.id(), containingFunction);
@@ -207,7 +207,15 @@ export default async function transform(
   return rootNode.commitEdits(edits);
 }
 
-function parseCustomRenderFunctionsFromOptions(options: any): Set<string> {
+interface CodemodOptions {
+  params?: {
+    customRenderFunctions?: string | number | boolean;
+  };
+}
+
+function parseCustomRenderFunctionsFromOptions(
+  options?: CodemodOptions,
+): Set<string> {
   const customRenderFunctionsParam = options?.params?.customRenderFunctions
     ? String(options.params.customRenderFunctions)
     : '';
@@ -319,6 +327,17 @@ function extractImportedFunctionNames(
   return { importedFunctions, specifiersToRemove };
 }
 
+/**
+ * Removes duplicate import specifiers from import statements.
+ * Handles complex comma placement scenarios when removing specifiers:
+ * - Trailing commas after the specifier
+ * - Leading commas before the specifier
+ * - Edge cases with single vs multiple specifiers
+ *
+ * @param specifiersToRemove - Array of specifiers to remove with their import statements
+ * @param rootNode - The root AST node (used to get full text for comma detection)
+ * @param edits - Array to collect edit operations
+ */
 function removeDuplicateImportSpecifiers(
   specifiersToRemove: Array<{ specifier: SgNode<TSX>; importStmt: SgNode<TSX> }>,
   rootNode: SgNode<TSX>,
@@ -493,7 +512,8 @@ function findFireEventMethodCalls(
             }
           }
         } catch {
-          // Skip if field() is not available
+          // Skip nodes where field() is not available or AST structure doesn't match expectations.
+          // This is expected for malformed or edge-case AST structures and should be silently ignored.
         }
       }
     }
@@ -528,14 +548,28 @@ function findScreenMethodCalls(rootNode: SgNode<TSX>): SgNode<TSX>[] {
           }
         }
       } catch {
-        // Skip if field() is not available
+        // Skip nodes where field() is not available or AST structure doesn't match expectations.
+        // This is expected for malformed or edge-case AST structures and should be silently ignored.
       }
     }
   }
 
   return functionCalls;
 }
 
+/**
+ * Tracks variables assigned from render() calls to identify renderer result objects.
+ * This helps identify calls like `renderer.rerender()` or `renderer.unmount()` that need to be made async.
+ *
+ * Handles various assignment patterns:
+ * - Direct assignment: `const renderer = render(...)`
+ * - Destructured assignment: `const { rerender } = render(...)`
+ * - Assignment expressions: `renderer = render(...)`
+ *
+ * @param rootNode - The root AST node to search within
+ * @param importedFunctions - Set of imported function names (must include 'render')
+ * @returns Set of variable names that represent renderer results
+ */
 function trackVariablesAssignedFromRender(
   rootNode: SgNode<TSX>,
   importedFunctions: Set<string>,
@@ -646,7 +680,8 @@ function findRendererMethodCalls(
             }
           }
         } catch {
-          // Skip if field() is not available
+          // Skip nodes where field() is not available or AST structure doesn't match expectations.
+          // This is expected for malformed or edge-case AST structures and should be silently ignored.
         }
       }
     }
@@ -671,6 +706,21 @@ function findRendererMethodCalls(
   return functionCalls;
 }
 
+/**
+ * Tracks variables assigned from renderHook() calls to identify hook result objects.
+ * Similar to trackVariablesAssignedFromRender but handles renderHook-specific patterns.
+ *
+ * Handles various assignment patterns:
+ * - Direct assignment: `const result = renderHook(...)`
+ * - Destructured assignment: `const { rerender, unmount } = renderHook(...)`
+ * - Renamed destructuring: `const { rerender: rerenderHook } = renderHook(...)`
+ *
+ * @param rootNode - The root AST node to search within
+ * @param importedFunctions - Set of imported function names (must include 'renderHook')
+ * @returns Object containing:
+ *   - renderHookVariables: Set of all variable names representing hook results
+ *   - renderHookMethodVariables: Set of renamed method variables (e.g., rerenderHook)
+ */
 function trackVariablesAssignedFromRenderHook(
   rootNode: SgNode<TSX>,
   importedFunctions: Set<string>,
@@ -824,7 +874,8 @@ function findRenderHookMethodCalls(
             }
           }
         } catch {
-          // Skip if field() is not available
+          // Skip nodes where field() is not available or AST structure doesn't match expectations.
+          // This is expected for malformed or edge-case AST structures and should be silently ignored.
         }
       }
     }
@@ -849,6 +900,20 @@ function findRenderHookMethodCalls(
   return functionCalls;
 }
 
+/**
+ * Automatically detects custom render functions by analyzing the code structure.
+ * A custom render function is identified as:
+ * - A function/const that starts with 'render' (e.g., renderWithProviders, renderWithTheme)
+ * - Called from within test functions
+ * - Contains calls to the base render() function from RNTL
+ * - Defined at the top level (not nested inside other functions)
+ *
+ * This helps identify custom render wrappers that should be transformed.
+ *
+ * @param rootNode - The root AST node to search within
+ * @param importedFunctions - Set of imported function names (must include 'render')
+ * @returns Set of custom render function names that were auto-detected
+ */
 function findAutoDetectedCustomRenderFunctions(
   rootNode: SgNode<TSX>,
   importedFunctions: Set<string>,
@@ -1116,7 +1181,8 @@ function findRNTLFunctionCallsInNode(
             }
           }
         } catch {
-          // Skip if field() is not available
+          // Skip nodes where field() is not available or AST structure doesn't match expectations.
+          // This is expected for malformed or edge-case AST structures and should be silently ignored.
         }
       }
     }
@@ -1151,7 +1217,7 @@ function transformRNTLCallsInsideCustomRender(
   }
 
   if (needsAsync && !customRenderFunctionsToMakeAsync.has(funcNode.id())) {
-    const isAsync = isFunctionAlreadyAsync(funcNode, rootNode);
+    const isAsync = isFunctionAlreadyAsync(funcNode);
     if (!isAsync) {
       customRenderFunctionsToMakeAsync.set(funcNode.id(), funcNode);
     }
@@ -1172,15 +1238,30 @@ function addAwaitBeforeCall(functionCall: SgNode<TSX>, edits: Edit[]): void {
   });
 }
 
-function isFunctionAlreadyAsync(func: SgNode<TSX>, rootNode: SgNode<TSX>): boolean {
+/**
+ * Checks if a function is already marked as async using AST-based detection.
+ * This is more reliable than string matching and handles edge cases better.
+ */
+function isFunctionAlreadyAsync(func: SgNode<TSX>): boolean {
   if (func.is('arrow_function')) {
+    // For arrow functions, check if 'async' is a direct child
     const children = func.children();
     return children.some((child) => child.text() === 'async');
-  } else {
-    const funcStart = func.range().start.index;
-    const textBefore = rootNode.text().substring(Math.max(0, funcStart - 10), funcStart);
-    return textBefore.trim().endsWith('async');
+  } else if (func.is('function_declaration') || func.is('function_expression')) {
+    // For function declarations/expressions, check for async modifier
+    // The async keyword appears before the 'function' keyword
+    const children = func.children();
+    const functionKeywordIndex = children.findIndex((child) => child.text() === 'function');
+    if (functionKeywordIndex > 0) {
+      // Check if any child before 'function' is 'async'
+      return children
+        .slice(0, functionKeywordIndex)
+        .some((child) => child.text() === 'async');
+    }
+    // Also check if the first child is 'async'
+    return children.length > 0 && children[0].text() === 'async';
   }
+  return false;
 }
 
 function addAsyncKeywordToFunction(func: SgNode<TSX>, edits: Edit[]): void {
@@ -1212,6 +1293,19 @@ function addAsyncKeywordToFunction(func: SgNode<TSX>, edits: Edit[]): void {
   }
 }
 
+/**
+ * Finds the containing test function (test, it, beforeEach, etc.) for a given node.
+ * Traverses up the AST tree to find the nearest test function that contains the node.
+ *
+ * Handles various test patterns:
+ * - Direct test functions: test(), it()
+ * - Test modifiers: test.skip(), it.only()
+ * - Test.each patterns: test.each(), it.each()
+ * - Hooks: beforeEach(), afterEach(), beforeAll(), afterAll()
+ *
+ * @param node - The AST node to find the containing test function for
+ * @returns The containing test function node, or null if not found
+ */
 function findContainingTestFunction(node: SgNode<TSX>): SgNode<TSX> | null {
   let current: SgNode<TSX> | null = node;
 
@@ -1244,7 +1338,8 @@ function findContainingTestFunction(node: SgNode<TSX>): SgNode<TSX> | null {
                     }
                   }
                 } catch {
-                  // Skip if field() is not available
+                  // Skip nodes where field() is not available or AST structure doesn't match expectations.
+                  // This is expected for malformed or edge-case AST structures and should be silently ignored.
                 }
               }
               if (funcNode.is('call_expression')) {
@@ -1262,7 +1357,8 @@ function findContainingTestFunction(node: SgNode<TSX>): SgNode<TSX> | null {
                     }
                   }
                 } catch {
-                  // Skip if field() is not available
+                  // Skip nodes where field() is not available or AST structure doesn't match expectations.
+                  // This is expected for malformed or edge-case AST structures and should be silently ignored.
                 }
               }
             }
@@ -1287,7 +1383,8 @@ function findContainingTestFunction(node: SgNode<TSX>): SgNode<TSX> | null {
                   }
                 }
               } catch {
-                // Skip if field() is not available
+                // Skip nodes where field() is not available or AST structure doesn't match expectations.
+                // This is expected for malformed or edge-case AST structures and should be silently ignored.
               }
             }
             if (funcNode.is('call_expression')) {
@@ -1305,7 +1402,8 @@ function findContainingTestFunction(node: SgNode<TSX>): SgNode<TSX> | null {
                   }
                 }
               } catch {
-                // Skip if field() is not available
+                // Skip nodes where field() is not available or AST structure doesn't match expectations.
+                // This is expected for malformed or edge-case AST structures and should be silently ignored.
               }
             }
           }

codemods/v14-update-deps/README.md

@@ -5,8 +5,8 @@ This codemod updates your `package.json` file to prepare for React Native Testin
 - ✅ Removing `@types/react-test-renderer` (no longer needed)
 - ✅ Removing `react-test-renderer` (replaced by universal-test-renderer)
 - ✅ Moving `@testing-library/react-native` from `dependencies` to `devDependencies` if present
-- ✅ Adding `@testing-library/react-native@^14.0.0-alpha` to `devDependencies` if not present
-- ✅ Updating `@testing-library/react-native` to `^14.0.0-alpha` (latest alpha version)
+- ✅ Adding `@testing-library/react-native@^14.0.0-alpha.5` to `devDependencies` if not present
+- ✅ Updating `@testing-library/react-native` to `^14.0.0-alpha.5`
 - ✅ Adding `universal-test-renderer@0.10.1` to `devDependencies` (always added)
 
 ## What it does
@@ -17,9 +17,9 @@ This codemod automatically updates your `package.json` dependencies to match the
 
 2. **Moves RNTL to devDependencies**: If `@testing-library/react-native` is in `dependencies`, it's moved to `devDependencies`
 
-3. **Ensures RNTL is present**: If `@testing-library/react-native` is not present, it's added to `devDependencies` with version `^14.0.0-alpha`
+3. **Ensures RNTL is present**: If `@testing-library/react-native` is not present, it's added to `devDependencies` with version `^14.0.0-alpha.5`
 
-4. **Updates RNTL version**: Updates `@testing-library/react-native` to `^14.0.0-alpha` to get the latest alpha version
+4. **Updates RNTL version**: Updates `@testing-library/react-native` to `^14.0.0-alpha.5`
 
 5. **Adds universal-test-renderer**: Always adds `universal-test-renderer@0.10.1` to `devDependencies`
 
@@ -56,7 +56,7 @@ npx codemod@latest run @testing-library/react-native-v14-update-deps --target ./
 ```json
 {
   "devDependencies": {
-    "@testing-library/react-native": "^14.0.0-alpha",
+    "@testing-library/react-native": "^14.0.0-alpha.5",
     "universal-test-renderer": "0.10.1"
   }
 }
@@ -79,7 +79,7 @@ npx codemod@latest run @testing-library/react-native-v14-update-deps --target ./
 ```json
 {
   "devDependencies": {
-    "@testing-library/react-native": "^14.0.0-alpha",
+    "@testing-library/react-native": "^14.0.0-alpha.5",
     "universal-test-renderer": "0.10.1"
   }
 }
@@ -103,7 +103,7 @@ npx codemod@latest run @testing-library/react-native-v14-update-deps --target ./
 {
   "devDependencies": {
     "some-other-package": "^1.0.0",
-    "@testing-library/react-native": "^14.0.0-alpha",
+    "@testing-library/react-native": "^14.0.0-alpha.5",
     "universal-test-renderer": "0.10.1"
   }
 }
@@ -121,7 +121,7 @@ npx codemod@latest run @testing-library/react-native-v14-update-deps --target ./
    pnpm install
    ```
 
-2. **Version resolution**: The codemod sets `@testing-library/react-native` to `^14.0.0-alpha`, which will resolve to the latest alpha version available. You can manually update this to a specific version if needed.
+2. **Version resolution**: The codemod sets `@testing-library/react-native` to `^14.0.0-alpha.5`. You can manually update this to a different version if needed.
 
 3. **Always adds packages**: The codemod always ensures both `@testing-library/react-native` and `universal-test-renderer` are present in `devDependencies`, even if they weren't there before.
 
@@ -138,7 +138,7 @@ npm test
 
 1. **Package manager**: The codemod updates `package.json` but doesn't run the package manager install command. You need to run `npm install` / `yarn install` / `pnpm install` manually after the codemod completes.
 
-2. **Version pinning**: If you have a specific alpha version pinned, the codemod will update it to the range `^14.0.0-alpha`. You may want to review and adjust the version after running the codemod.
+2. **Version pinning**: If you have a specific alpha version pinned, the codemod will update it to `^14.0.0-alpha.5`. You may want to review and adjust the version after running the codemod.
 
 3. **Workspace projects**: For monorepos with multiple `package.json` files, the codemod will process each one individually.
 

codemods/v14-update-deps/scripts/codemod.ts

@@ -55,9 +55,11 @@ export default async function transform(
 
     return null;
   } catch (error) {
-    const errorMessage = error instanceof Error ? error.message : String(error);
-    console.error(`Error processing ${filename}:`, errorMessage);
-    return null;
+    // Re-throw error to let the codemod platform handle it
+    // This provides better error reporting than silently returning null
+    throw new Error(
+      `Error processing ${filename}: ${error instanceof Error ? error.message : String(error)}`,
+    );
   }
 }
 

codemods/v14-update-deps/scripts/test.js

@@ -2,8 +2,10 @@
 
 /**
  * Test script for the package.json update codemod
+ * Note: This test script uses console.log for test output, which is acceptable for test scripts
  */
 
+/* eslint-disable no-console */
 import { readFileSync, writeFileSync, existsSync, readdirSync } from 'fs';
 import { join, dirname } from 'path';
 import { fileURLToPath } from 'url';

codemods/v14-update-deps/tsconfig.json

@@ -0,0 +1,17 @@
+{
+  "compilerOptions": {
+    "module": "NodeNext",
+    "moduleResolution": "NodeNext",
+    "types": ["@codemod.com/jssg-types"],
+    "allowImportingTsExtensions": true,
+    "noEmit": true,
+    "verbatimModuleSyntax": true,
+    "erasableSyntaxOnly": true,
+    "strict": true,
+    "strictNullChecks": true,
+    "noImplicitReturns": true,
+    "noFallthroughCasesInSwitch": true,
+    "noUncheckedIndexedAccess": true
+  },
+  "exclude": ["tests"]
+}