Address review: non-mutating test helper, drop as-cast, split stranded comment

Author: sdkennedy2

packages/plugins/apps/src/vite/backend-module-graph-collector.test.ts

@@ -6,29 +6,30 @@ import { parseAst } from 'rollup/parseAst';
 
 import { createBackendModuleGraphCollector } from './backend-module-graph-collector';
 
-type FakeModuleInfo = { id: string; code?: string | null };
+type FakeModuleInfo = { id: string; code: string | null };
 
 /**
- * Invokes the hook with a plugin context exposing `parse`, which is where it gets
- * its parser. `rollup/parseAst` is what Rollup's real context uses.
+ * Calls the `moduleParsed` hook with a plugin context exposing `parse`, which is
+ * where it takes its parser from. `rollup/parseAst` is what Rollup's real
+ * context supplies. The hook reads only the few `ModuleInfo` fields these fakes
+ * model, so building a complete one would be noise.
  */
+const getModuleParsedHook = (collector: ReturnType<typeof createBackendModuleGraphCollector>) => {
+    const hook = collector.plugin.moduleParsed;
+    if (typeof hook !== 'function') {
+        throw new Error('Expected "moduleParsed" to be a function hook.');
+    }
+
+    return (moduleInfo: object) => Reflect.apply(hook, { parse: parseAst }, [moduleInfo]);
+};
+
 const getEmit = (collector: ReturnType<typeof createBackendModuleGraphCollector>) => {
-    const moduleParsed = collector.plugin.moduleParsed as (
-        this: { parse: typeof parseAst },
-        moduleInfo: unknown,
-    ) => void;
+    const callHook = getModuleParsedHook(collector);
 
-    // Fills the remaining fields in place rather than spreading into a new
-    // object: a spread would drop (or trigger) an `ast` getter, and one case
-    // below depends on that getter surviving intact.
-    return (moduleInfo: FakeModuleInfo, importedIds: string[] = []) =>
-        moduleParsed.call(
-            { parse: parseAst },
-            Object.assign(moduleInfo, {
-                importedIds,
-                importedIdResolutions: importedIds.map((id) => ({ id })),
-            }),
-        );
+    return (moduleInfo: FakeModuleInfo, importedIds: string[] = []) => {
+        const importedIdResolutions = importedIds.map((id) => ({ id }));
+        callHook({ ...moduleInfo, importedIds, importedIdResolutions });
+    };
 };
 
 describe('Backend Functions - backend module graph collector', () => {
@@ -70,23 +71,28 @@ describe('Backend Functions - backend module graph collector', () => {
 
     test('Should collect records under a bundler that does not support ModuleInfo#ast', () => {
         const collector = createBackendModuleGraphCollector('/project');
-        const emit = getEmit(collector);
+        const callHook = getModuleParsedHook(collector);
 
-        // Rolldown, the bundler Vite 8 uses by default, keeps `ast` on its
-        // Rollup-compat object but stubs the getter to throw. Reading the
-        // property at all is the failure, so it must throw rather than be absent.
-        const moduleInfo: FakeModuleInfo = Object.defineProperty(
-            { id: '/project/src/backend/actions.backend.ts', code: 'export const id = "conn-1";' },
-            'ast',
-            {
-                get() {
-                    throw new Error('UNSUPPORTED: ModuleInfo#ast');
-                },
-                enumerable: true,
+        // Rolldown, Vite 8's default bundler, keeps `ast` on its Rollup-compat
+        // object but stubs the getter to throw. Reading the property at all is
+        // the failure, so it has to throw rather than be absent — which is also
+        // why this is assembled in place instead of going through `getEmit`,
+        // whose spread would trigger the getter during setup.
+        const moduleInfo = {
+            id: '/project/src/backend/actions.backend.ts',
+            code: 'export const id = "conn-1";',
+            importedIds: [],
+            importedIdResolutions: [],
+        };
+        Object.defineProperty(moduleInfo, 'ast', {
+            get() {
+                throw new Error('UNSUPPORTED: ModuleInfo#ast');
             },
-        );
+            enumerable: true,
+        });
+
+        callHook(moduleInfo);
 
-        expect(() => emit(moduleInfo)).not.toThrow();
         expect([...collector.getModuleRecords().keys()]).toEqual([
             '/project/src/backend/actions.backend.ts',
         ]);

packages/plugins/apps/src/vite/backend-module-graph-collector.ts

@@ -37,27 +37,27 @@ export function createBackendModuleGraphCollector(buildRoot: string): BackendMod
                     return;
                 }
 
-                // Parse the source instead of reading `moduleInfo.ast`: Rolldown,
-                // the bundler Vite 8 uses by default, stubs that getter to throw
-                // `UNSUPPORTED: ModuleInfo#ast`. `code` is null for external and
-                // synthetic modules.
-                //
-                // `this.parse` is the bundler's own parser, which already parsed
-                // this exact source to compute `importedIds` — so whatever the
-                // bundler accepted parses here too, and no TypeScript-capable
-                // parser is needed (`moduleParsed` runs after `transform`, so
-                // types and JSX are already gone).
+                // External and synthetic modules have no source to parse.
                 if (typeof moduleInfo.code !== 'string') {
                     return;
                 }
 
+                // Parse the source instead of reading `moduleInfo.ast`: Rolldown,
+                // the bundler Vite 8 uses by default, stubs that getter to throw
+                // `UNSUPPORTED: ModuleInfo#ast`. Using the context's own parser
+                // is also correct by construction — it already accepted this
+                // exact source to compute `importedIds` — and needs no
+                // TypeScript support, since `moduleParsed` runs after
+                // `transform`, so types and JSX are already gone.
                 const parsed = this.parse(moduleInfo.code);
                 const record = createParsedModuleRecord(
                     moduleId,
                     buildRoot,
                     parsed,
                     getStaticDependencyIds(moduleInfo).map(normalizeViteModuleId),
                 );
+                // Only null when the traversal predicate rejects, which the guard
+                // above already covered; kept to narrow the nullable return type.
                 if (!record) {
                     return;
                 }