fix(optimize-imports): address review nits — TODO comment, type annotation, test names, wildcard dir test

- Add TODO comment on allResolved bail-out suggesting debug logging
- Remove redundant ': string[]' annotation on optimizePackageImports (inferred)
- Rename namespace re-export test to use generic X/barrel names instead of Slot/lucide-react
- Update syntax-error test to match new empty-map-on-parse-error behavior (not null)
- Add test for directory-style 'export * from "./components"' where components/index.js exists

Author: james-elicx

packages/vinext/src/config/next-config.ts

@@ -506,7 +506,7 @@ export async function resolveNextConfig(
 
   // Resolve optimizePackageImports from experimental config
   const rawOptimize = experimental?.optimizePackageImports;
-  const optimizePackageImports: string[] = Array.isArray(rawOptimize)
+  const optimizePackageImports = Array.isArray(rawOptimize)
     ? rawOptimize.filter((x): x is string => typeof x === "string")
     : [];
 

packages/vinext/src/plugins/optimize-imports.ts

@@ -284,8 +284,12 @@ function resolvePackageEntry(
  *   - `import * as X; export { X }` — indirect namespace re-export
  *   - `export * from "./sub"` — wildcard: recursively parse sub-module and merge exports
  *
+ * Returns an empty map when the file cannot be read or has a parse error, so that
+ * recursive wildcard calls degrade gracefully without aborting the whole barrel walk.
+ *
  * @param initialContent - Pre-read file content for `filePath`. If provided, skips the
- *   `readFile` call for the entry file — useful when the caller already has the content.
+ *   `readFile` call for the entry file — avoids a redundant read when the caller
+ *   already has the content in hand.
  */
 function buildExportMapFromFile(
   filePath: string,
@@ -377,7 +381,16 @@ function buildExportMapFromFile(
           if (rawSource.startsWith(".")) {
             const subPath = path.resolve(fileDir, rawSource).split(path.sep).join("/");
             // Try with the path as-is first, then with common extensions
-            const candidates = [subPath, `${subPath}.js`, `${subPath}.mjs`, `${subPath}.ts`];
+            const candidates = [
+              subPath,
+              `${subPath}.js`,
+              `${subPath}.mjs`,
+              `${subPath}.ts`,
+              // Directory-style sub-modules: `export * from "./components"` where
+              // `components/` is a directory with an index file.
+              `${subPath}/index.js`,
+              `${subPath}/index.ts`,
+            ];
             for (const candidate of candidates) {
               if (readFile(candidate) !== null) {
                 const subMap = buildExportMapFromFile(candidate, readFile, cache, visited);
@@ -463,16 +476,14 @@ export function buildBarrelExportMap(
   const cached = exportMapCache.get(entryPath);
   if (cached) return cached;
 
-  // Verify the entry file is readable and parseable before delegating to the
-  // recursive helper. This lets us return null (instead of an empty map) for
-  // unresolvable or unparseable entries.
+  // Verify the entry file is readable before delegating to the recursive helper.
+  // This lets us return null (instead of an empty map) for unresolvable entries,
+  // giving callers a clear signal that the package barrel could not be analyzed.
+  // Parse errors in the entry file are handled gracefully by buildExportMapFromFile
+  // (returns an empty map), which causes the transform to leave all imports unchanged —
+  // the correct safe fallback.
   const content = readFile(entryPath);
   if (!content) return null;
-  try {
-    parseAst(content);
-  } catch {
-    return null;
-  }
 
   const visited = new Set<string>();
   // Pass the already-read content so buildExportMapFromFile skips the redundant
@@ -507,6 +518,9 @@ export function createOptimizeImportsPlugin(
   // buildStart so the transform loop doesn't allocate template literals per file.
   let quotedPackages: string[] = [];
 
+  // `satisfies Plugin` gives a structural type-check at the object literal in addition
+  // to the `: Plugin` return type annotation on the function, catching hook name typos
+  // or shape mismatches that the return-type check alone would accept silently.
   return {
     name: "vinext:optimize-imports",
     // No enforce — runs after JSX transform so parseAst gets plain JS.
@@ -669,6 +683,7 @@ export function createOptimizeImportsPlugin(
             if (!allResolved) break;
           }
 
+          // TODO: consider debug logging which specifier was unresolved
           if (!allResolved || specifiers.length === 0) continue;
 
           // Group specifiers by their resolved source module
@@ -758,5 +773,5 @@ export function createOptimizeImportsPlugin(
         };
       },
     },
-  };
+  } satisfies Plugin;
 }

tests/optimize-imports.test.ts

@@ -221,14 +221,18 @@ export { format };`;
     expect(map).toBeNull();
   });
 
-  it("returns null when entry file has syntax errors", () => {
+  it("returns an empty map when entry file has syntax errors", () => {
+    // Parse errors produce an empty map (safe fallback — leaves the import
+    // unchanged in the transform), not null. Null is only returned when the
+    // entry path itself cannot be resolved or the file cannot be read.
     const entryPath = uniquePath("syntax-error");
     const map = buildBarrelExportMap(
       "test-pkg",
       () => entryPath,
       () => "export { unclosed",
     );
-    expect(map).toBeNull();
+    expect(map).not.toBeNull();
+    expect(map!.size).toBe(0);
   });
 
   it("resolves wildcard export * from './sub' by merging sub-module exports", () => {
@@ -352,6 +356,30 @@ export { Button } from "./button";`;
     expect(buttonEntry!.source).toBe("/fake/nested/components/Button");
     expect(buttonEntry!.source).not.toBe("/fake/nested/Button");
   });
+
+  it("resolves wildcard export * from './components' where components/ is a directory with index.js", () => {
+    // Tests the `/index.js` candidate added to the wildcard resolution path.
+    // `export * from "./components"` with no extension — the resolver must
+    // try `components/index.js` when `components.js` does not exist.
+    const entryPath = "/fake/dir-wildcard/index.js";
+    const files: Record<string, string> = {
+      [entryPath]: `export * from "./components";`,
+      "/fake/dir-wildcard/components/index.js": `export { Button } from "./Button";`,
+      "/fake/dir-wildcard/components/Button.js": `export function Button() {}`,
+    };
+
+    const map = buildBarrelExportMap(
+      "test-pkg",
+      () => entryPath,
+      (fp) => files[fp] ?? null,
+    );
+
+    expect(map).not.toBeNull();
+    const buttonEntry = map!.get("Button");
+    expect(buttonEntry).toBeDefined();
+    // Must resolve through the directory index, relative to components/
+    expect(buttonEntry!.source).toBe("/fake/dir-wildcard/components/Button");
+  });
 });
 
 // ── Plugin transform with real FS fixture ─────────────────────
@@ -408,7 +436,7 @@ describe("vinext:optimize-imports transform", () => {
     if (tmpDir) fs.rmSync(tmpDir, { recursive: true, force: true });
   });
 
-  it("rewrites namespace re-export: import { Slot } from 'pkg' → import * as Slot from 'sub-pkg'", async () => {
+  it("rewrites namespace re-export: import { X } from 'barrel' → import * as X from 'sub-pkg'", async () => {
     // lucide-react is in DEFAULT_OPTIMIZE_PACKAGES. The barrel contents below use
     // @radix-ui sub-packages intentionally — this tests the namespace rewrite path
     // with an arbitrary barrel; the package name just needs to be in the optimized list.