Skip to content

fix(routing): bundle CommonJS dependencies of compiled-binary API routes - #5477

Closed
kojiwakayama wants to merge 29 commits into
mainfrom
fix/4177-bundle-route-cjs-deps
Closed

kojiwakayama wants to merge 29 commits into
mainfrom
fix/4177-bundle-route-cjs-deps

Conversation

@kojiwakayama

@kojiwakayama kojiwakayama commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

Summary

In a compiled binary, API routes now bundle the project's CommonJS dependencies with esbuild. Before, a hand-written runtime CommonJS loader loaded them, and this PR removes that loader. deno compile binaries cannot load a CommonJS file from disk at runtime: require there reads only embedded files, and importing a .cjs file evaluates it as ESM. Bundling gives Node's resolution and interop from esbuild. ESM dependencies still load from file:// URLs.

This replaces the approach in #5472, which is closed unmerged.

  • The module-level require that bundled code calls at runtime serves any built-in through process.getBuiltinModule, including a computed id. Any other package id throws MODULE_NOT_FOUND, so optional dependencies probed in try still fall back.
  • Each dependency file that mentions __dirname, __filename or require.resolve is parsed once by the bundler, not matched as text. In a CommonJS file, unbound __dirname and __filename become constants, and each require.resolve("literal") becomes the path Node returns. No wrapper is added, so arguments, a hashbang and a "use strict" prologue are unchanged. An extensionless file, which Node's require() loads as CommonJS, gets the same treatment. A .js file counts as ESM by its package type (looked up no further than node_modules, as in Node) or by the parser's verdict on its syntax.
  • In an ESM dependency file, import.meta.url (with the query it was imported with), filename and dirname point at the file itself, and import.meta.resolve("literal") resolves at build time. A computed or detached import.meta.resolve calls a resolver bound to the file. That resolver looks up a package name or #import through the runtime require with the import condition and keeps a target's query.
  • At runtime, a computed require finds a file in Node's order: the path, then with .js, .json or .node, then the directory's package.json main, then its index. JSON loads from anywhere in the project, with a byte order mark dropped, and so does a package.json. A JavaScript file loads only from the project's dependencies, as CommonJS with its own require and Node's module object, so require(path.join(__dirname, "dialects", name + ".js")) works. A package name is looked up in node_modules from the requiring file upwards (from the project root for route code), through exports (including * patterns, null blocks, array fallbacks past invalid targets, percent-escaped targets with any query or fragment dropped; targets cannot leave the package, and a map mixing subpath and condition keys is refused) or main. On Windows, .\\ and ..\\ ids are relative. The nearest copy of a package that refuses the path ends the lookup, as in Node. A #name goes through the package's imports. require.resolve returns the real path and takes Node's paths option. A native addon throws ERR_DLOPEN_FAILED, and an ES module (.mjs, or .js or .ts in a type: module package) throws ERR_REQUIRE_ESM. A .ts or .cts file runs with its types stripped, as in Node 24. A malformed package scope throws ERR_INVALID_PACKAGE_CONFIG, and pkg/ loads the package root. Project JavaScript never loads this way. Bare prefix-only names such as test are not built-ins.
  • A require the bundler leaves in a bundled dependency file, such as a computed id, runs with the runtime require of that file's directory, as in Node. A file that uses direct eval declares require, __dirname and __filename for its own directory.
  • Bundled CommonJS dependency files and dependency JSON run through the runtime require's module cache, as Node's one cache. The route prologue routes esbuild's __commonJS helper through it. A module is cached before its body runs, so a cycle through a computed require sees it partly loaded. It leaves the cache if its body throws, and a computed and a literal require of one file share it either way round. require.cache exposes it, and deleting an entry makes the next load, literal or computed, run the file again. An unknown helper shape fails the build.
  • A dependency file's top-level arguments holds Node's five wrapper values. A file Node parses as CommonJS (.cjs, .cts, or "type": "commonjs") that has ES module syntax throws a SyntaxError when it runs. A file with an extension Node has no loader for runs as CommonJS, even in a "type": "module" scope. A file that reassigns require, plainly or with a compound assignment, gets its own binding. A dependency ES module has no require binding.
  • The runtime require follows Node's package rules: a package's own name resolves through its exports before node_modules, #imports stop at node_modules, and require.resolve keeps the paths option (relative entries resolve from the working directory, invalid values throw), require.resolve.paths() and module.paths. import.meta.resolve returns a package-map target that does not exist yet.
  • A with { type: "json" } import of a dependency's JSON gets the module require() caches, as in Node 24; one with a query or fragment is a JSON module of its own. onLoad callbacks now receive the import attributes (OnLoadArgs.with).
  • Runtime modules record parent and children, each bundled module has its own require, and a failed load leaves both the cache and its parent's children. A malformed package.json of a required package or directory throws ERR_INVALID_PACKAGE_CONFIG. Direct eval is recognized in every spelling. Also, and JSON a computed require loads first gets a complete module record. A malformed package scope fails every id resolved from it with ERR_INVALID_PACKAGE_CONFIG, import.meta.resolve included. With paths, absolute, #import and self ids resolve from the caller first. A linked workspace package's lookup stops at the copy that refuses an id.
  • A dependency module's import.meta has a null prototype and main: false; a POSIX file URL with a host throws ERR_INVALID_FILE_URL_HOST; assigning require in an ES module throws a ReferenceError.
  • Each dependency file is prepared the same way every time, so an unchanged route rebuilds to the same module and keeps its cache entry.
  • .cts files are CommonJS TypeScript, and .jsx and .ts files are prepared like .js.
  • A dependency reached from project code through the import map, the @/ alias or a relative node_modules path is prepared as a dependency.
  • The runtime helper names are chosen so they never collide with a route's own declarations.
  • A linked @scope directory counts as a dependency directory.
  • Project code's built-in imports resolve after the import map, so a project can map a built-in name to its own module.
  • External import rewrites (built-ins, ESM packages, veryfront) apply only to parsed import specifiers. Package strings that read like imports stay as written.
  • Native addons (.node files) fail the build with a clear message that names only the file. A package whose name ends in .node still bundles.
  • A package that loads from disk as ESM stays external only for import and import(), and only where it resolves to the project's own copy; a dependency's nested copy is bundled for it. A dependency's import of a subpath names the file the package's exports map it to. A require() of it from a bundled dependency bundles its CommonJS export, so a dual package works from both sides.
  • Dependency files resolve their own imports as Node 24 does. They read the node and module-sync conditions and main, never module. node-addons stays off, as under --no-addons, since the binary cannot load addons. Extensionless relative imports try .js, .json, .node, then the directory's index, rather than a TypeScript or .mjs sibling. The project's import map and @/ alias don't apply. Workspace packages linked from node_modules to a directory inside the project count as dependencies.
  • An installed dependency gets the trust it has on every other runtime, where it loads unchecked: it is not held to the route-code import rules, which reject ordinary package code such as computed property reads in mime-types, jsonwebtoken and semver. Dependency files still load only from inside the project.
  • Removes generateCompiledBinaryRequireShim, the require() rewrite of CommonJS dependency imports, and the destructuring helper it used, plus their tests. Symlink containment stays covered by the source snapshot tests.

Only the compiled binary changes. Deno source runs, Node and Bun are unchanged.

Behaviour changes in the compiled binary

  • A missing package now fails the route build with a project-relative message (app/api/x/route.ts:1:16: Could not resolve "pg"). It no longer fails at runtime.
  • Each route gets its own copy of a bundled package. The old runtime loader also cached per route, so this is not a regression.
  • A linked workspace package's package imports skip the project's tsconfig paths, as in Node.
  • module.parent is the module that first required a dependency, as in Node, and undefined for one the route imports.
  • createRequire from node:module in a dependency is the runtime's own, with a cache apart from the bundle's.

Not in this PR

Hosted runtimes still do not install tenant package.json dependencies. That is separate work.

Test plan

  • compiled-cjs-bundling.test.ts (unit) covers built-in names, the package-type lookup stopping at node_modules, Node's extension order, prefix-only built-in ids, and helper naming.
  • tests/integration/semantic-unit-boundary/.../compiled-cjs-bundling.test.ts bundles and imports real packages. It covers an exports map, built-in/JSON/optional requires, __dirname, and a route-level require binding. It also covers require.resolve (relative, self, built-in, missing), a computed runtime require of in-project JSON, rejection of a path outside the project, ESM dependencies getting no CommonJS bindings, and rejection of a native addon. Newer cases cover:
    • strings and comments that look like require.resolve or export;
    • arguments in a CommonJS file;
    • a linked workspace package;
    • Node's resolution order (.mjs beside .json, .ts beside .js, a module-only package);
    • a route's own require binding with nothing bundled;
    • cached JSON from a computed require;
    • a computed built-in require;
    • an extensionless main, and an extensionless main or default index beside a .ts sibling;
    • dependency import.meta (computed, detached, built-in and bare);
    • a package named client.node;
    • a dual package required from a bundled dependency;
    • a dependency that requires a computed path (a dialect loader);
    • one module shared by a bundled and a computed require, JSON included;
    • a workspace package's imports past the project's tsconfig paths;
    • a query in a dependency module's import.meta.url;
    • a computed relative require in a bundled dependency;
    • import.meta.resolve of a package's own #import;
    • a dependency's nested copy of an external ESM package, and an external subpath its exports remap;
    • a dependency that uses direct eval;
    • a dependency reached through a project resolver;
    • a module cycle through a computed require, require.cache deletion with a literal re-require, and a top-level return;
    • Node's wrapper arguments, module-sync, forced CommonJS syntax, runtime TypeScript and a BOM-prefixed package scope;
    • a direct-eval dependency beside an external dynamic import;
    • reassigned require, an unknown-extension main in a module scope, package self-reference, #imports scope ending at node_modules, require.resolve paths, a not-yet-built #import target, a JSON import attribute, and typeof require in an ES module;
    • the module graph (parent, children), a complete JSON cache entry, a malformed package scope, caller-scoped ids with paths, import.meta's shape, file URL hosts, require assignment in an ES module, and a linked workspace package whose nearest copy refuses an id;
    • a lazy require from a bundled module, failed loads, malformed target packages and directories, and (eval)(x).
      Each new case was checked to fail with its fix reverted.
  • Hosted-like run of the built binary: /app-style project, no node_modules, VERYFRONT_HOST_ALLOW_PROJECT_EXECUTION=1, serve --mode=production.
    • The SSR page returned 200.
    • An API route using node:crypto, node:path and a relative TS module returned 200 in 0.03 s.
    • A route importing the declared but uninstalled pg failed closed with Could not resolve "pg". On main this route failed on the first request with MODULE_NOT_FOUND.
  • New compiled-binary e2e case: an API route importing a CommonJS dependency and an ESM dependency. The CommonJS dependency covers directory and extensionless requires and computed reads. Binary e2e passed in CI on the first commit.
  • Built bin/veryfront locally and ran serve --mode=production:
    • The fixture route returned 200.
    • A dependency doing a computed require of fs returned 200 (rebuilt on d0532c2).
    • A dialect loader using require(path.join(__dirname, ...)) returned 200, and its exported 'from "fs"' string came back unchanged.
    • A dependency that requires one file and one JSON file both statically and by computed path got the same objects (rebuilt on 48640ee).
    • A route using real pg, jsonwebtoken, lodash, mime-types and semver (1,253 .js files in node_modules) returned 200. The cold build took 0.11–0.15 s, against 0.11 s before the parse and resolution changes; warm requests take 5 ms.
    • A route importing an uninstalled package failed closed in 0.01 s.
    • A route importing one dependency by a relative path and by @/node_modules/..., and declaring its own __vf_require_from, returned 200 with one shared module (rebuilt on dd15aac).
    • On 3d4664b, a dependency with a computed require of a .cts file, top-level arguments, and a require.cache deletion and re-require returned 200. The values matched Node's (types stripped, five arguments, the file ran again). The real-package route cold-built in 0.12 s.
    • On 2706f75, a route covering the round-10 cases returned Node's values: a swapped require, a .foo main with five arguments, MODULE_NOT_FOUND for a scope-less #config, a separate JSON import object, typeof require "undefined" in ESM, and the not-yet-built target URL. All other routes still returned 200.
    • On c006f1c, a route covering the round-11 cases returned Node's values: children ["a.js","b.js","d.json"], a complete JSON cache entry, import.meta with a null prototype and main: false, ERR_INVALID_FILE_URL_HOST and a ReferenceError for an ES module require assignment. All other routes still returned 200.
    • On 4a652bd, a route with a lazy require, a failed load, malformed packages and (eval)("require") returned Node's values, and all other routes still returned 200.
  • deno task test:file on the loader, rewriter, loader-helpers, import-rewriter core and new tests: all pass. The semantic test audit passes.

Closes #5471.

Posted by Claude Code (AI agent) on behalf of the PR author.

A compiled binary cannot load a CommonJS file from disk at runtime, so the
route build now bundles a project's CommonJS dependencies with esbuild's Node
resolution and interop instead of loading them through a hand-written runtime
CommonJS loader. ESM dependencies still load from file:// URLs.

- built-ins a bundled module requires are imported statically and bound to
  the module-level require; any other id throws MODULE_NOT_FOUND
- __dirname/__filename are defined per bundled CommonJS file
- native addons fail the build with a clear message
- bundled dependency files pass the same project boundary and import scan as
  route code

Refs veryfront/veryfront-issue-inbox#4177, #5471
@kojiwakayama

Copy link
Copy Markdown
Contributor Author

@codex review

Posted by Claude Code (AI agent) on behalf of the PR author.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 6, 2026 •

Copy link
Copy Markdown
Codex Review Summary: ✅ Completed

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-07T06:57:53.286104Z 0a2a12c Manual request
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

github-actions Bot commented Nov 6, 2026 •

Copy link
Copy Markdown

📦 Client bundle boundary

Entrypoint Modules Source size Server leaks
src/index.client.ts 66 366 KiB ✅ 0
src/react/components/Head.tsx 5 74 KiB ✅ 0
src/react/router/index.tsx 5 74 KiB ✅ 0
src/react/context/index.tsx 5 74 KiB ✅ 0
src/react/fonts/index.ts 6 81 KiB ✅ 0
src/react/components/ui/index.ts 111 717 KiB ✅ 0
src/react/components/ui/adapter/contract.ts 1 18 KiB ✅ 0
src/react/components/ui/icons/index.ts 3 17 KiB ✅ 0
src/chat/index.ts 305 2046 KiB ✅ 0
src/chat/ag-ui.ts 61 459 KiB ✅ 0
src/chat/protocol.ts 1 11 KiB ✅ 0
src/chat/types.ts 46 246 KiB ✅ 0
src/chat/message-prep.ts 68 517 KiB ✅ 0
src/markdown/index.ts 6 30 KiB ✅ 0
src/mdx/index.ts 2 2 KiB ✅ 0
src/agent/identity-contracts.ts 1 3 KiB ✅ 0
src/schemas/index.ts 43 220 KiB ✅ 0
src/platform/env.ts 5 53 KiB ✅ 0
src/workflow/claude-code/react/index.ts 9 91 KiB ✅ 0
src/events/index.ts 31 567 KiB ✅ 0
src/events/ag-ui/public.ts 30 561 KiB ✅ 0
src/run-events/index.ts 54 321 KiB ✅ 0

A server module in a client graph aborts hydration in the browser. New leaks fail CI; known leaks are tracked in scripts/lint/client-bundle-baseline.json to burn down.

@github-actions

github-actions Bot commented Nov 6, 2026

Copy link
Copy Markdown

@codex review

Copy link
Copy Markdown
Contributor

Score: 84/100. Replaces the runtime CJS loader in compiled binaries with esbuild bundling. The approach is sound, scoped to the compiled binary, and well tested.

  • Strength: Non-compiled runtimes are unchanged. Every new branch is gated on compiled, and the new logic is isolated in compiled-cjs-bundling.ts.
  • Strength: The built-in require handling is careful. The table is prototype-less, unknown ids throw MODULE_NOT_FOUND so optional try requires still fall back, hashbangs are preserved, and helper names avoid collisions. Native addons fail the build with a clear message.
  • Strength: Test coverage is good. Unit tests cover the helpers, there is a real bundle-and-import test (exports map with the node condition, JSON, __dirname, optional require, route-level require), and there is a compiled-binary e2e case. Dependency files still go through the project-boundary checks.
  • Concern (minor): commonJSPathPrelude and DECLARES_PATH_BINDING are regex heuristics. A comment or string containing __dirname, or a var __dirname in a comment, can add a prelude that is not needed or skip one that is. Consider noting this limitation in a comment, or using esbuild's define or a banner approach if it proves flaky.
  • Concern (minor): mainFields: ["main","module"] with conditions: ["node"] prefers CJS entry points even for dual packages. That is intentional here, but it changes which build of a dual package runs in compiled binaries. Mention it in the release notes.
  • Concern (minor): The old shim and its tests remain as dead code, which the description acknowledges. Please track the follow-up issue so they are removed.

I read the diff only and did not run the tests. Verify that CI passes, including the compiled-binary integration suite.


Generated by Claude Code

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6f63222a64

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/routing/api/module-loader/loader.ts Resolved
Comment thread src/routing/api/module-loader/loader.ts Resolved
@codecov

codecov Bot commented Nov 6, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 89.05207% with 164 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/routing/api/module-loader/loader.ts 51.93% 79 Missing and 8 partials ⚠️
...routing/api/module-loader/compiled-cjs-bundling.ts 94.21% 20 Missing and 56 partials ⚠️
...tensions/ext-bundler-esbuild/src/plugin-adapter.ts 0.00% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

Files under the project's node_modules that a compiled-binary route bundles
now resolve their own imports with esbuild's Node resolution (directory
indexes, extensionless and .json requires) instead of the project's import
map, @/ alias and adapter resolver, and are not held to the route-code
import rules, which rejected ordinary package code such as computed property
reads (mime-types, jsonwebtoken, semver). Elsewhere these dependencies load
at runtime unscanned; they still load only from inside the project.

Refs veryfront/veryfront-issue-inbox#4177
…nary routes

The route build now bundles CommonJS dependencies, so the generated runtime
loader (generateCompiledBinaryRequireShim), the require() rewrite of CommonJS
dependency imports and its destructuring helper are unused. Remove them and
their tests. Project-boundary symlink containment stays covered by the source
snapshot tests.

Move the esbuild bundling tests to tests/integration/semantic-unit-boundary,
since they write temp files; the unit file keeps the pure helpers.

Refs veryfront/veryfront-issue-inbox#4177
@github-actions

github-actions Bot commented Nov 6, 2026

Copy link
Copy Markdown

@codex review

@github-actions

github-actions Bot commented Nov 6, 2026

Copy link
Copy Markdown

@codex review

@kojiwakayama

Copy link
Copy Markdown
Contributor Author

@codex review

Posted by Claude Code (AI agent) on behalf of the PR author.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Swish!

Reviewed commit: a7753e278d

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Wrap a CommonJS dependency in a function that passes __dirname and
__filename, so a hashbang and a "use strict" prologue stay in effect
and a file that declares one of them still reads the other. ESM
dependency files (by extension, package type or syntax) are left alone.

Resolve literal require.resolve() calls at build time, and serve
runtime require()/require.resolve() for built-ins and for existing
files inside the project; anything else throws MODULE_NOT_FOUND.
@github-actions

github-actions Bot commented Nov 6, 2026

Copy link
Copy Markdown

@codex review

@kojiwakayama

Copy link
Copy Markdown
Contributor Author

@codex review

Posted by Claude Code (AI agent) on behalf of the PR author.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 22b529dd7d

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/routing/api/module-loader/loader.ts Resolved
Comment thread src/routing/api/module-loader/loader.ts Resolved
The Bun shard has no Deno global; use node:fs/promises instead.
@kojiwakayama

Copy link
Copy Markdown
Contributor Author

@codex review

Posted by Claude Code (AI agent) on behalf of the PR author.

@github-actions

github-actions Bot commented Nov 6, 2026

Copy link
Copy Markdown

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a41c0b042b

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/routing/api/module-loader/loader.ts Resolved
Bun defines __dirname and __filename in ESM modules too.
@kojiwakayama

Copy link
Copy Markdown
Contributor Author

@codex review

Posted by Claude Code (AI agent) on behalf of the PR author.

@github-actions

github-actions Bot commented Nov 6, 2026

Copy link
Copy Markdown

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 52d7ee7db3

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

…e does

Parse each CommonJS dependency file with the bundler instead of matching
its text: ESM syntax comes from the parser, and unbound __dirname,
__filename and require.resolve("literal") are replaced through define.
No wrapper function is added, so arguments, the hashbang and the
directive prologue are untouched.

Inside dependencies, resolve extensionless relative imports in Node's
order (.js, .json, .node), read only the main field, and stop the
package-type lookup at node_modules. Workspace packages linked from
node_modules count as dependencies. Prefix-only built-ins such as
"test" no longer resolve without the node: prefix.
…iles

Bundled dependency files now run through the runtime require's cache:
cached before the body runs, evicted on throw or on deletion. Top-level
arguments carry Node's five wrapper values. Forced CommonJS files with
ES module syntax throw a SyntaxError. The build reads module-sync. The
runtime require strips TypeScript types, reads BOM-prefixed and rejects
malformed package scopes, accepts pkg/ root ids, evicts unreadable
files, and resolves import.meta.resolve packages with import
conditions. Windows backslash require ids resolve at build time, and
package target suffixes are kept.
@github-actions

github-actions Bot commented Nov 7, 2026

Copy link
Copy Markdown

@codex review

@kojiwakayama

Copy link
Copy Markdown
Contributor Author

@codex review 3d4664b

Posted by Claude Code (AI agent) on behalf of the PR author.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3d4664b5d0

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/routing/api/module-loader/loader.ts Resolved
…me has it

Bun has no stripTypeScriptTypes; there the runtime require refuses the
file with ERR_UNKNOWN_FILE_EXTENSION instead of running it.
@github-actions

github-actions Bot commented Nov 7, 2026

Copy link
Copy Markdown

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 34e7bf1438

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

…endencies

- A dependency that assigns require, plainly or compoundly, gets its own binding.
- Files with extensions Node has no loader for are prepared as CommonJS.
- #imports stop at node_modules; a package resolves its own name first.
- import.meta.resolve returns package-map targets that do not exist yet.
- require.resolve keeps Node's paths API, require.resolve.paths and module.paths.
- An ES module JSON import with type json stays apart from require()'s module.
- ES module dependencies see no require binding.
- Generated code writes paths through codeString.
@github-actions

github-actions Bot commented Nov 7, 2026

Copy link
Copy Markdown

@codex review

@kojiwakayama

Copy link
Copy Markdown
Contributor Author

@codex review 2706f75

Posted by Claude Code (AI agent) on behalf of the PR author.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2706f75bec

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

…ort.meta

- A malformed package scope fails every id resolved from it, as Node's
  self-reference lookup does, and import.meta.resolve reports it.
- JSON a computed require loads first gets a complete module record.
- Modules record their parent and children.
- require.resolve with paths resolves absolute, #imports and self ids from
  the caller first.
- A linked workspace lookup stops at the copy that refuses an id.
- A type json import shares require()'s module unless it has a query.
- An ES module's require is undeclared, so assigning it throws.
- import.meta has a null prototype and main: false; file URLs with a host
  are rejected on POSIX.
@github-actions

github-actions Bot commented Nov 7, 2026

Copy link
Copy Markdown

@codex review

@kojiwakayama

Copy link
Copy Markdown
Contributor Author

@codex review c006f1c

Posted by Claude Code (AI agent) on behalf of the PR author.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c006f1c5e1

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

…bad packages

- A bundled module's computed requires use its own require, so a call it
  makes after its body ran still names it as the parent.
- A failed load leaves its parent's children as well as the cache.
- A malformed package.json of a required package or directory throws
  ERR_INVALID_PACKAGE_CONFIG instead of MODULE_NOT_FOUND.
- Direct eval is detected from the parsed output, so (eval)(x) and
  commented spellings count.
@github-actions

github-actions Bot commented Nov 7, 2026

Copy link
Copy Markdown

@codex review

@kojiwakayama

Copy link
Copy Markdown
Contributor Author

@codex review 4a652bd

Posted by Claude Code (AI agent) on behalf of the PR author.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4a652bd135

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

…ir dependency

- A literal require in a bundled CommonJS dependency is marked as the
  module's own, so one a callback makes after the body has finished still
  names that module as its parent and child.
- require() and require.resolve() reject a non-string id with Node's
  TypeError ERR_INVALID_ARG_TYPE, and require('') with
  ERR_INVALID_ARG_VALUE, instead of MODULE_NOT_FOUND.
- An import() the bundler cannot follow resolves from the dependency file,
  as import.meta.resolve there would; one that does not resolve rejects.
@github-actions

github-actions Bot commented Nov 7, 2026

Copy link
Copy Markdown

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4a079aa73f

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +1912 to +1913
if (onlyHelperCalls(code, BUNDLED_REQUIRE_FROM)) {
code = code.replace(new RegExp(`\\b${BUNDLED_REQUIRE_FROM}\\b`, "g"), requireFrom);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Avoid rewriting helper-shaped text in strings

When route code contains the exact text __vf_require_from("..., for example inside a tagged template such as String.raw\__vf_require_from("x")`, onlyHelperCalls()` counts that textual occurrence as a generated call. This fast path then replaces the identifier globally, including inside the template, so the compiled route exports different string data than the source supplied. Use the parser-based free-reference rewrite for this case as well, or make the fast-path detection syntax-aware rather than matching raw text.

Useful? React with 👍 / 👎.

try {
// The bundler names files relative to its working directory, with
// symlinks followed; this keys them the way computed requires do.
const path = host.realpathSync(host.join(bundleRoot!, key));

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Map suffixed CommonJS imports to their real cache path

When the same CommonJS file is imported with a query or fragment, esbuild includes that suffix in the generated CommonJS module key, so passing the key directly to realpathSync looks for a nonexistent filename such as a.cjs?x. bundledPath() then returns null and leaves each suffixed import in esbuild's separate helper cache instead of the shared runtime cache; Node 24.15 ignores the suffix for the underlying CommonJS load, so import "./a.cjs?x", import "./a.cjs?y", and require("./a.cjs") all evaluate once and return the same exports object. Recover the suffix-free filesystem path before cache lookup so state and object identity do not depend on the import spelling.

AGENTS.md reference: AGENTS.md:L20-L22

Useful? React with 👍 / 👎.

return BUILTINS.has(specifier) ? specifier : undefined;
}

const NATIVE_ADDON = /\.node$/i;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep Node extension matching case-sensitive

On case-sensitive hosts, Node treats an explicitly required file such as entry.NODE as an unknown extension and runs it through the CommonJS JavaScript loader, but this case-insensitive pattern classifies it as a native addon and rejects the build. The same case folding appears in the runtime .node/.mjs/.mts checks, so uppercase extensions can also receive ERR_DLOPEN_FAILED or ERR_REQUIRE_ESM instead of executing as CommonJS. Match Node's exact lowercase loader extensions on POSIX rather than applying /i across these sibling paths.

AGENTS.md reference: AGENTS.md:L23-L25

Useful? React with 👍 / 👎.

return url.href;
};
return (specifier) => {
const id = String(specifier);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3 Badge Reject Symbol module specifiers during string coercion

When a bundled dependency calls detached import.meta.resolve(Symbol("x")) or import(Symbol("x")), String(specifier) converts the symbol to the package-like text Symbol(x) and the helper reports ERR_MODULE_NOT_FOUND. Native Node uses the ECMAScript abstract string conversion for module specifiers, which rejects symbols with a TypeError; code can therefore misclassify this programming error as an absent optional package. Perform specifier coercion with the same Symbol-rejecting semantics as the native APIs.

Useful? React with 👍 / 👎.

The runtime require() and import.meta.resolve() of a bundled dependency
now throw what Node 24 throws instead of a generic MODULE_NOT_FOUND:
ERR_PACKAGE_PATH_NOT_EXPORTED for a subpath exports does not define,
ERR_PACKAGE_IMPORT_NOT_DEFINED for an undefined #import,
ERR_INVALID_PACKAGE_CONFIG for exports mixing subpaths and conditions,
and ERR_INVALID_PACKAGE_TARGET for a target that leaves its package.
Optional-dependency code that catches MODULE_NOT_FOUND no longer hides a
broken package.
@github-actions

github-actions Bot commented Nov 7, 2026

Copy link
Copy Markdown

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0a2a12cfe3

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +639 to +644
const jsExtension = /\.(?:jsx?|ts)$/i.test(filePath);
// Node's require() loads a file without an extension, or with one it has
// no loader for, as CommonJS, whatever its package declares; a package
// `main` may name one.
const extensionless = !KNOWN_EXTENSION.test(filePath);
if (!cjsExtension && !extensionless && !(jsExtension && packageType !== "module")) return null;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Treat JSX extensions as unknown CommonJS files

When a dependency explicitly requires a lowercase .jsx or .tsx file, Node treats that extension as unknown and executes the file as CommonJS even inside a "type": "module" package. Including jsx here makes .jsx follow the package type, while the sibling KNOWN_EXTENSION pattern makes .tsx skip the unknown-extension path entirely. As a result, CommonJS bindings such as __dirname are not prepared, and TypeScript syntax in .tsx is transpiled successfully where Node throws a SyntaxError. Keep both extensions on the unknown-extension CommonJS path.

AGENTS.md reference: AGENTS.md:L23-L25

Useful? React with 👍 / 👎.

Comment on lines +891 to +893
for (const entry of [...computed].sort((a, b) => b.s - a.s)) {
result = result.slice(0, entry.s) + `${marker}(${result.slice(entry.s, entry.e)})` +
result.slice(entry.e);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep computed CommonJS imports inside the bundled loader

When a delayed computed import(id) resolves to a CommonJS .js or .cjs dependency, this rewrite feeds its file URL to the native import() rather than the bundle's CommonJS runtime. In a compiled Deno binary that bypasses the shared cache and attempts to load the CommonJS file from disk, which is precisely the unsupported path this change is intended to replace and can evaluate it as ESM or fail. The new integration coverage exercises only plugin.mjs and a type: module package, so it misses this distinct target type; computed imports resolving to CommonJS must return the bundled module's namespace instead.

AGENTS.md reference: AGENTS.md:L26-L28

Useful? React with 👍 / 👎.

@sonarqubecloud

sonarqubecloud Bot commented Nov 7, 2026

Copy link
Copy Markdown

@kojiwakayama

Copy link
Copy Markdown
Contributor Author

Replaced by #5484. It loads CommonJS dependencies through the runtime's own createRequire, which works in Deno 2.9 compiled binaries, instead of a custom loader. The open review threads here no longer apply.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Hosted runtime: CommonJS npm packages fail (__vf_interopDefault / require is not defined), app console output missing in logs

3 participants