Merge pull request #468 from DataDog/sdkennedy2/apps-parse-backend-module-source

[APPS] fix(apps): build backend functions under Vite 8 / Rolldown

Co-authored-by: sdkennedy2 <scott.kennedy@datadoghq.com>

Author: gh-worker-dd-mergequeue-cf854d[bot]

packages/plugins/apps/src/index.test.ts

@@ -41,17 +41,24 @@ function extractViteTransform(plugins: PluginOptions[]) {
 }
 
 function emitModuleParsed(
-    config: { plugins?: Array<{ moduleParsed?: (moduleInfo: unknown) => void }> },
+    config: {
+        plugins?: Array<{
+            moduleParsed?: (this: { parse: typeof parseAst }, moduleInfo: unknown) => void;
+        }>;
+    },
     id: string,
     code: string,
     importedIds: string[] = [],
 ) {
     for (const plugin of config.plugins ?? []) {
-        plugin.moduleParsed?.({
-            id,
-            ast: parseAst(code),
-            importedIds,
-        });
+        plugin.moduleParsed?.call(
+            { parse: parseAst },
+            {
+                id,
+                code,
+                importedIds,
+            },
+        );
     }
 }
 

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

@@ -6,40 +6,58 @@ import { parseAst } from 'rollup/parseAst';
 
 import { createBackendModuleGraphCollector } from './backend-module-graph-collector';
 
+type FakeModuleInfo = { id: string; code: string | null };
+
+/**
+ * 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.');
+    }
+
+    const parse = jest.fn(parseAst);
+    const callHook = (moduleInfo: object) => Reflect.apply(hook, { parse }, [moduleInfo]);
+
+    return { callHook, parse };
+};
+
+const getEmit = (collector: ReturnType<typeof createBackendModuleGraphCollector>) => {
+    const { callHook, parse } = getModuleParsedHook(collector);
+
+    const emit = (moduleInfo: FakeModuleInfo, importedIds: string[] = []) => {
+        const importedIdResolutions = importedIds.map((id) => ({ id }));
+        callHook({ ...moduleInfo, importedIds, importedIdResolutions });
+    };
+
+    return { emit, parse };
+};
+
 describe('Backend Functions - backend module graph collector', () => {
-    test('Should collect parsed local module records from Rollup moduleParsed hooks', () => {
+    test('Should collect parsed local module records from moduleParsed hooks', () => {
         const collector = createBackendModuleGraphCollector('/project');
-        const moduleParsed = collector.plugin.moduleParsed as (moduleInfo: unknown) => void;
-
-        moduleParsed({
-            id: '/project/src/backend/actions.backend.js?import',
-            ast: parseAst(`
-                import { getEcho } from './helpers/http.js';
-                export function run() {
-                    return getEcho();
-                }
-            `),
-            importedIds: ['/project/src/backend/helpers/http.js?import'],
-            importedIdResolutions: [{ id: '/project/src/backend/helpers/http.js?import' }],
-        });
-        moduleParsed({
-            id: '/project/node_modules/package/index.js',
-            ast: parseAst('export const value = true;'),
-            importedIds: [],
-            importedIdResolutions: [],
-        });
-        moduleParsed({
-            id: '\0virtual-helper.js',
-            ast: parseAst('export const value = true;'),
-            importedIds: [],
-            importedIdResolutions: [],
-        });
-        moduleParsed({
-            id: 'virtual:dd-backend-dev:example.js',
-            ast: parseAst('export const value = true;'),
-            importedIds: [],
-            importedIdResolutions: [],
-        });
+        const { emit, parse } = getEmit(collector);
+
+        emit(
+            {
+                id: '/project/src/backend/actions.backend.js?import',
+                code: `
+                    import { getEcho } from './helpers/http.js';
+                    export function run() {
+                        return getEcho();
+                    }
+                `,
+            },
+            ['/project/src/backend/helpers/http.js?import'],
+        );
+        emit({ id: '/project/node_modules/package/index.js', code: 'export const value = true;' });
+        emit({ id: '\0virtual-helper.js', code: 'export const value = true;' });
+        emit({ id: 'virtual:dd-backend-dev:example.js', code: 'export const value = true;' });
+        emit({ id: '/project/src/backend/external.js', code: null });
 
         expect([...collector.getModuleRecords().keys()]).toEqual([
             '/project/src/backend/actions.backend.js',
@@ -54,5 +72,38 @@ describe('Backend Functions - backend module graph collector', () => {
                 },
             ],
         });
+        // Filtering happens before the parse, so the skipped modules above never
+        // reach the parser. Without that ordering every `node_modules` module in
+        // the backend graph would be parsed just to be discarded.
+        expect(parse).toHaveBeenCalledTimes(1);
+    });
+
+    test('Should collect records under a bundler that does not support ModuleInfo#ast', () => {
+        const collector = createBackendModuleGraphCollector('/project');
+        const { callHook } = getModuleParsedHook(collector);
+
+        // 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([...collector.getModuleRecords().keys()]).toEqual([
+            '/project/src/backend/actions.backend.ts',
+        ]);
     });
 });

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

@@ -2,13 +2,14 @@
 // This product includes software developed at Datadog (https://www.datadoghq.com/).
 // Copyright 2019-Present Datadog, Inc.
 
-import type { BaseNode } from 'estree';
 import type { ModuleInfo } from 'rollup';
 import type { Plugin } from 'vite';
 
 import {
     createParsedModuleRecord,
     type ParsedModuleRecord,
+    shouldTraverseCollectedModule,
+    unsupportedModuleGraphDependency,
 } from '../backend/ast-parsing/module-graph';
 
 const VIRTUAL_MODULE_ID_RE = /^(?:\0|virtual:)/;
@@ -30,12 +31,49 @@ export function createBackendModuleGraphCollector(buildRoot: string): BackendMod
                     return;
                 }
 
+                // `createParsedModuleRecord` applies this same predicate, but
+                // only after the AST exists. Checking it here keeps us from
+                // parsing every `node_modules` module just to discard it.
+                if (!shouldTraverseCollectedModule(moduleId, buildRoot)) {
+                    return;
+                }
+
+                // 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`.
+                //
+                // No TypeScript-capable parser is needed because `moduleParsed`
+                // runs after `transform`, so types and JSX are already compiled
+                // away. Note that `this.parse` is the bundler's parser but not
+                // its parser *configuration* — Rollup binds no options to it,
+                // while its own module parse passes `{ jsx }`. Our nested build
+                // never enables `jsx`, so the two agree today; fail closed rather
+                // than silently if they ever diverge, since a module we cannot
+                // parse may hide a connection ID.
+                let parsed;
+                try {
+                    parsed = this.parse(moduleInfo.code);
+                } catch (error) {
+                    const reason = error instanceof Error ? error.message : String(error);
+                    throw unsupportedModuleGraphDependency(
+                        moduleId,
+                        `unparseable module source (${reason})`,
+                    );
+                }
+
                 const record = createParsedModuleRecord(
                     moduleId,
                     buildRoot,
-                    moduleInfo.ast as BaseNode,
+                    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;
                 }

packages/plugins/apps/src/vite/dev-server.test.ts

@@ -113,17 +113,24 @@ function mockBuildResult(code: string) {
 }
 
 function emitModuleParsed(
-    config: { plugins?: Array<{ moduleParsed?: (moduleInfo: unknown) => void }> },
+    config: {
+        plugins?: Array<{
+            moduleParsed?: (this: { parse: typeof parseAst }, moduleInfo: unknown) => void;
+        }>;
+    },
     id: string,
     code: string,
     importedIds: string[] = [],
 ) {
     for (const plugin of config.plugins ?? []) {
-        plugin.moduleParsed?.({
-            id,
-            ast: parseAst(code),
-            importedIds,
-        });
+        plugin.moduleParsed?.call(
+            { parse: parseAst },
+            {
+                id,
+                code,
+                importedIds,
+            },
+        );
     }
 }
 

packages/plugins/apps/src/vite/index.test.ts

@@ -48,16 +48,23 @@ function mockBuildResult() {
 }
 
 function emitModuleParsed(
-    config: { plugins?: Array<{ moduleParsed?: (moduleInfo: unknown) => void }> },
+    config: {
+        plugins?: Array<{
+            moduleParsed?: (this: { parse: typeof parseAst }, moduleInfo: unknown) => void;
+        }>;
+    },
     id: string,
     code: string,
 ) {
     for (const plugin of config.plugins ?? []) {
-        plugin.moduleParsed?.({
-            id,
-            ast: parseAst(code),
-            importedIds: [],
-        });
+        plugin.moduleParsed?.call(
+            { parse: parseAst },
+            {
+                id,
+                code,
+                importedIds: [],
+            },
+        );
     }
 }