[APPS] Support importing .backend.ts files as RPC proxies from frontend code#320
Conversation
sdkennedy2
changed the title
This stack of pull requests is managed by Graphite. Learn more about stacking. |
🎉 All green!❄️ No new flaky tests detected 🔗 Commit SHA: 30d9c5d | Docs | Datadog PR Page | Give us feedback! |
sdkennedy2
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fcb5014b68
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } catch (error) { | ||
| log.error( | ||
| `Failed to parse exports from ${id}: ${error instanceof Error ? error.message : String(error)}`, | ||
| ); | ||
| return undefined; |
There was a problem hiding this comment.
Do not swallow export-analysis errors for .backend.* modules here. When extractExportedFunctions rejects (for example on default exports), this code logs and returns undefined, which leaves the original backend module untransformed; the frontend then bundles backend implementation code directly and no backend bundle is produced for that file. In practice this turns a configuration error into a silent runtime/security regression instead of a clear build failure.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d088b2594a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!exportNames) { | ||
| return undefined; |
There was a problem hiding this comment.
When parseExportNames yields null (including the explicit default-export error path), this branch returns undefined, so the .backend.* module is left untransformed in the frontend build instead of failing fast. In that case no backend function is registered, but the original backend source can still be bundled client-side, which is a silent and hard-to-debug failure mode for an invalid backend module shape. This should raise a build error rather than skipping transformation.
Useful? React with 👍 / 👎.
| const backendFiles = discoverBackendFiles(context.buildRoot, log); | ||
| const backendOutputs = new Map<string, string>(); | ||
| const hasBackend = backendFunctions.length > 0; | ||
| const hasBackend = backendFiles.length > 0; |
There was a problem hiding this comment.
This flag is computed from the initial glob scan only, but backend functions are actually populated later during transform. If transform discovers functions that the pre-scan misses (for example, due to scope/ignore mismatches), backendOutputs can be filled while hasBackend stays false, and the upload path skips adding backend/*.js assets. Using backendFunctions.length/backendOutputs.size at upload time avoids dropping built backend bundles.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5e0dda3366
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } catch (error) { | ||
| log.error( | ||
| `Failed to parse exports from ${id}: ${error instanceof Error ? error.message : String(error)}`, | ||
| ); | ||
| return null; |
There was a problem hiding this comment.
When extractExportedFunctions throws (for example on export default, which this change explicitly disallows), parseExportNames catches the error and returns null, so the caller in index.ts silently skips proxy generation and function registration. In that case the .backend.* module is left untransformed, no backend bundle is produced, and frontend code can end up executing raw backend implementation instead of executeBackendFunction. This should fail the build (or at least the module transform) rather than degrading into a silent fallback.
Useful? React with 👍 / 👎.
| id, | ||
| context.buildRoot, | ||
| ); | ||
| backendFunctions.push(...functions); |
There was a problem hiding this comment.
The transform hook appends discovered functions into a long-lived backendFunctions array every time a .backend.* module is transformed. In dev/HMR, the same file is re-transformed on edits, so this array accumulates duplicate entries indefinitely; then createDevServerMiddleware rebuilds a map from the full array on every request, making request-time work and memory usage grow with edit history. Replacing push-only accumulation with per-file replacement or dedup by encoded query name avoids this degradation.
Useful? React with 👍 / 👎.
sdkennedy2
changed the title
| export interface BackendFunction { | ||
| /** The BackendFunctionRef identifying this function */ | ||
| ref: BackendFunctionRef; | ||
| /** Absolute path to the .backend.ts source file */ | ||
| entryPath: string; | ||
| } | ||
|
|
||
| const EXTENSIONS = ['.ts', '.js', '.tsx', '.jsx']; | ||
| export interface BackendFileInfo { |
There was a problem hiding this comment.
I'm going to combine these into one type
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4e7c072f07
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| log.error( | ||
| `Failed to parse exports from ${id}: ${error instanceof Error ? error.message : String(error)}`, | ||
| ); | ||
| return null; |
There was a problem hiding this comment.
Returning null on export-analysis errors causes the transform hook to skip rewriting the .backend.* module, so unsupported patterns (for example export default) are silently shipped as normal frontend code instead of being blocked. In that case no backend function is registered, but the raw backend implementation is still importable by the client bundle, which can lead to incorrect runtime behavior and backend code leakage. This should surface as a hard error for .backend.* files rather than a soft skip.
Useful? React with 👍 / 👎.
| id, | ||
| context.buildRoot, | ||
| ); | ||
| backendFunctions.push(...functions); |
There was a problem hiding this comment.
This append-only push makes backend registrations accumulate across repeated transforms of the same file (common in Vite dev/HMR). If exports are renamed or removed, stale entries remain in backendFunctions, so /__dd/* lookups can still resolve deleted query names and fail later during bundling instead of returning a clean 404; the array also grows indefinitely over edits. Track functions per module id and replace old entries on re-transform.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 887f0698fc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| export function generateProxyModule(exports: ProxyExport[]): string { | ||
| const lines: string[] = []; | ||
|
|
||
| lines.push("import { executeBackendFunction } from '@datadog/apps-function-query';"); |
There was a problem hiding this comment.
Every transformed .backend.* module now imports executeBackendFunction from @datadog/apps-function-query, but this package is not provided by this repo's plugin dependencies, so builds fail as soon as a frontend imports a backend proxy unless the app independently installs that package. In a standard Vite build this surfaces as Rollup failed to resolve import "@datadog/apps-function-query", which blocks production builds for the new feature path.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Doesn't this mean we need to add @datadog/apps-function-query in all published's package.json?
(Well, at least @datadog/vite-plugin since we only support this one for now)
There was a problem hiding this comment.
I added @datadog/apps-function-query based on our conversation, but I think I'll want to bump the version of @datadog/apps-function-query to 0.0.2 and update this PR before landing it.
| if (node.type !== 'ExportNamedDeclaration') { | ||
| continue; |
There was a problem hiding this comment.
extractExportedFunctions silently ignores non-ExportNamedDeclaration nodes, so a backend file that uses export * from ... yields no discovered functions; the transform then returns undefined and leaves the original .backend source in the frontend graph instead of generating RPC proxies. This can accidentally ship backend implementation code to the client and bypass backend bundling for those exports, so unsupported export forms should throw explicitly (or be fully resolved) rather than falling through.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 49bf9b74ed
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (exportNames.length === 0) { | ||
| return undefined; |
There was a problem hiding this comment.
If a matched .backend.* module uses an unsupported export form (for example export * from './impl'), extractExportedFunctions yields no names and this branch returns undefined, so the file is not replaced by a proxy at all. That means frontend imports can bundle and execute the original backend implementation directly, while no encoded function ref is registered for the dev middleware/build pipeline. This should fail closed (throw an error) rather than silently passing through backend code.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 58266aa2a4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const functions = getBackendFunctions(); | ||
| if (functions.length > 0) { | ||
| backendOutDir = await buildBackendFunctions( |
There was a problem hiding this comment.
closeBundle reuses backendOutputs across runs but never resets it before calling buildBackendFunctions. If backend exports are renamed/removed between rebuilds (e.g., watch mode), stale map entries still reference files from the previous temp output directory, which is deleted in the finally block; handleUpload can then attempt to archive missing or stale backend artifacts. Resetting/recomputing backendOutputs per run avoids cross-build contamination.
Useful? React with 👍 / 👎.
| if (exportNames.length === 0) { | ||
| return undefined; |
There was a problem hiding this comment.
When a .backend.* module has no named exports, the transform handler returns early without updating the registry. After an HMR/edit where a file drops all exports, the old functions for that file remain registered, so requests can still resolve removed query names and later rebuilds may attempt to bundle functions that no longer exist. Update the file’s registry entry to an empty list before returning.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0d95641706
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (exportNames.length === 0) { | ||
| return undefined; |
There was a problem hiding this comment.
When transform.handler processes a .backend.* file with no named exports, it returns before updating the per-file registry. In watch/HMR scenarios, if a file previously exported backend functions and is edited to only type exports (or no exports), the old entries remain in getBackendFunctions(), so stale function refs can still be bundled/served and may later fail against removed symbols.
Useful? React with 👍 / 👎.
| if (decl.type === 'VariableDeclaration') { | ||
| return decl.declarations | ||
| .filter((d): d is typeof d & { id: Identifier } => d.id.type === 'Identifier') | ||
| .map((d) => d.id.name); |
There was a problem hiding this comment.
namesFromDeclaration treats every exported variable identifier as a backend function name, even when the initializer is not callable. For example, export const VERSION = '1' will generate a proxy callable and a backend bundle that executes VERSION(...args), which fails at runtime. Discovery should limit variable exports to callable forms (or error for non-functions) to avoid shipping broken RPC endpoints.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1399c1cf64
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ); | ||
| } | ||
| // handles: export { add, multiply } | ||
| names.push(spec.exported.name); |
There was a problem hiding this comment.
export { ... } bindings are callable
extractExportedFunctions currently registers every export specifier name as a backend function without checking what it refers to. That means const VERSION = '1'; export { VERSION }; is accepted, a proxy VERSION(...args) is generated, and backend execution fails at runtime because VERSION is not callable. Declaration exports already guard against non-functions, so specifier exports should be validated the same way (or rejected) to avoid shipping broken RPC wrappers.
Useful? React with 👍 / 👎.
| if (node.type !== 'ExportNamedDeclaration') { | ||
| continue; |
There was a problem hiding this comment.
export * in backend files
All non-ExportNamedDeclaration nodes are skipped, so a .backend.ts module using export * from './impl' yields no discovered functions. The transform path treats that as "no backend exports" and leaves the original module untransformed, so frontend imports don't get RPC proxies and backend code can pass through into the client bundle. ExportAllDeclaration should be handled explicitly with a build-time error instead of being ignored.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
@sdkennedy2 Maybe it's better to throw here for now, and not support this pattern? WDYT?
There was a problem hiding this comment.
It's most likely the same issue for export class MyClass....
There was a problem hiding this comment.
I'd be down to throw. This seems like a good idea.
There was a problem hiding this comment.
This should be fixed now because we throw when we receieve export * from './impl'
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9669dc867e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // handles: export { add, multiply } | ||
| names.push(spec.exported.name); |
There was a problem hiding this comment.
The export-specifier path currently trusts any identifier in export { ... } and registers it as a backend function without checking whether it is callable. In a .backend.* file, const VERSION = '1.0'; export { VERSION }; will pass discovery here, generate a proxy, and later fail at runtime when the backend virtual entry executes VERSION(...args). This should be rejected during discovery the same way non-function export const declarations are rejected.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
We name handle local declarations so that if you have:
const VERSION = '1.0'; export { VERSION }; It will correctly check if the VERSION variable declaration is callable. In this case we will throw.
The only area thats a little shaky is we allow the following but we don't verify that handler is callable. It seemed like a reasonable tradeoff though. Alternatively we could throw in this scenario.
import {handler} from '..';
export {handler};| if (exportNames.length === 0) { | ||
| // Clear any previously registered functions for this file | ||
| // so stale entries don't persist across HMR re-transforms. | ||
| setBackendFunctions(id, []); | ||
| return undefined; |
There was a problem hiding this comment.
When a .backend.* file has zero discovered exports, the transform clears the registry but returns undefined, which leaves the original module source in the frontend bundle instead of replacing it with a proxy/empty module. In practice, temporary edits or unsupported export forms can cause backend-only code to be shipped to the client unexpectedly; the hook should still return a transformed module (or throw) after clearing stale registry state.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
There seems to be a gap here (ref codex's review).
| const functions = getBackendFunctions(); | ||
| if (functions.length > 0) { | ||
| backendOutDir = await buildBackendFunctions( |
There was a problem hiding this comment.
closeBundle now reads a dynamic function set via getBackendFunctions(), but backendOutputs is reused across runs and never reset. In watch/HMR rebuilds, removing or renaming a backend export leaves old entries in the map, and those paths point to temp files deleted after the previous run, so handleUpload() can try to archive stale/missing backend files. Resetting backendOutputs before rebuilding avoids stale uploads and intermittent archive failures.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Given that we specifically support the dev mode, this looks legit too.
apps-runtime.ts imports from backend/client and runs in the browser, so it belongs in the DOM compilation context alongside the client code. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
sdkennedy2
force-pushed
the
sdkennedy2/replace-esbuild-acorn-with-transform-hook
branch
from
203664f to
32cc139
Compare
sdkennedy2
changed the base branch from
graphite-base/320
to
sdkennedy2/apps-function-query-migrate-runtime
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 30d9c5d300
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (decl.type === 'FunctionDeclaration' && decl.id) { | ||
| return [decl.id.name]; |
There was a problem hiding this comment.
When a .backend.ts export is a plain synchronous function, this accepts it as an RPC endpoint, but the frontend module is later replaced with an async proxy that returns executeBackendFunction(...). Since TypeScript still sees the original source signature, code like const total = add(1, 2) + 1 type-checks/bundles against a synchronous number return and then receives a Promise at runtime. Either reject non-async exports here or provide Promise-based frontend typings so callers are forced to await the RPC.
Useful? React with 👍 / 👎.
| const backendFunctions = discoverBackendFunctions(absoluteBackendDir, log); | ||
| const backendOutputs = new Map<string, string>(); | ||
| const hasBackend = backendFunctions.length > 0; | ||
| if (context.bundler.name !== 'vite') { |
There was a problem hiding this comment.
Let me know your thoughts on this. Alternatively I could move the transform into getVitePlugin, but it seems the current tranform code could work for other bundlers once we build out the remaining missing functionality.
gh-worker-dd-devflow-36fce6
Bot
added
mergequeue-status: queued
mergequeue-status: in_progress
and removed
mergequeue-status: queued
labels
gh-worker-dd-mergequeue-cf854d
Bot
deleted the
sdkennedy2/replace-esbuild-acorn-with-transform-hook
branch
gh-worker-dd-devflow-36fce6
Bot
added
mergequeue-status: done
and removed
mergequeue-status: in_progress
labels
2 participants
Motivation
Backend RPC in
@dd/apps-pluginwas tightly coupled to a fixedbackend/directory, one-function-per-file assumptions, and a separate runtime package path.This PR shifts backend function discovery to the bundler transform pipeline so functions can be declared with a
.backend.{ts,tsx,js,jsx}extension anywhere in the project, and the plugin generates frontend call proxies automatically. It also moves backend execution runtime ownership into the apps plugin.Changes
Transform-based backend discovery
.backend.ts|.tsx|.js|.jsxfiles.this.parse(code)AST-based export extraction.Proxy generation and query-name encoding
export async function <name>(...args)→globalThis.DD_APPS_RUNTIME.executeBackendFunction("<queryName>", args).encodeQueryName(sha256(relativePath).exportName) so frontend bundles do not leak backend source paths.Runtime injection and execution transport
globalThis.DD_APPS_RUNTIME.executeBackendFunctionin the page, so generated proxy modules call a single shared runtime API instead of importing a package.executeBackendFunctiondispatches calls based on context:postMessageto parent and waits for query response.POST /__dd/executeAction.@datadog/apps-function-query.Build and packaging integration
backend/<queryName>.js; frontend assets stay underfrontend/.QA Instructions
yarn workspace @dd/tests test:unit packages/plugins/appswindow.DD_APPS_RUNTIME.executeBackendFunctionexists.POST /__dd/executeActionreturns success in dev flow.Blast Radius
.backend.ts|.tsx|.js|.jsxand are no longer required under a fixedbackend/directory.backendDirbehavior/config is removed.@datadog/apps-function-queryis no longer required by the apps/vite plugin.globalThis.DD_APPS_RUNTIMEas the runtime entrypoint for backend execution calls.Documentation
backendDir/fixed path model to.backend.*