fix(ai-mcp): add shims to bin bundle for CJS dynamic-require compat#754
fix(ai-mcp): add shims to bin bundle for CJS dynamic-require compat#754citizen204 wants to merge 1 commit into
Conversation
json-schema-to-typescript (inlined via noExternal) uses dynamic require() calls internally. When bundled into an ESM output the global require is absent, causing the CLI to crash on startup with "Dynamic require of 'fs' is not supported" before it can parse any args. shims: true makes tsup inject a createRequire-based shim at the top of the bundle so CJS dependencies can call require() safely from within the ESM output. Fixes TanStack#753
📝 WalkthroughWalkthroughThe AI-MCP CLI bundling configuration is updated to enable CommonJS shim injection in the tsup bundler, allowing ESM-bundled dependencies that use dynamic ChangesBundle configuration for ESM compatibility
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/ai-mcp/tsup.bin.config.ts`:
- Around line 13-15: The comment claiming "shims: true injects createRequire" is
incorrect; update the inline comment in tsup config to explain that shims only
provide ESM/Node global compatibility and do not polyfill dynamic require, and
then add a tsup injection/banner that defines a require using Node's
createRequire (e.g., use createRequire(import.meta.url) to set a global require
helper) so json-schema-to-typescript's dynamic require() calls will work in the
ESM bundle; specifically modify the tsup config where "shims: true" is set to
replace the comment and add an "inject" or "banner" entry that imports
createRequire from "module" and assigns const require =
createRequire(import.meta.url) (or attaches it to globalThis), ensuring the
bundle provides a working require at runtime.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3852e1cb-0669-4731-9370-6d95a4e67a37
📒 Files selected for processing (1)
packages/ai-mcp/tsup.bin.config.ts
| // json-schema-to-typescript uses CJS dynamic require(); shims injects | ||
| // createRequire so those calls work inside this ESM bundle. | ||
| shims: true, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify tsup shims option documentation and behavior
# Check tsup documentation for the shims option
echo "=== Checking tsup package.json for version ==="
cat package.json | jq -r '.devDependencies.tsup // .dependencies.tsup // "not found"'
echo -e "\n=== Searching for tsup shims usage in the codebase ==="
rg -n --type=ts 'shims.*true' -g '!node_modules' -C2Repository: TanStack/ai
Length of output: 553
🌐 Web query:
What does the tsup shims option do for ESM bundles with dynamic require calls?
💡 Result:
The shims option in tsup is designed to provide compatibility polyfills when building code that assumes the presence of Node.js-specific global variables or module system features in formats where they are not natively available [1][2]. Specifically, it injects code to support dirname and filename in ESM bundles (which natively only provide import.meta.url) and provides import.meta.url in CJS bundles [1][2]. While the shims option is intended to bridge these gaps, it is not a comprehensive solution for "dynamic require" errors in ESM bundles [3][4]. 1. Purpose of Shims: When enabled (shims: true), tsup automatically injects polyfills via esbuild's inject configuration [1]. These polyfills define dirname and filename using node:path and node:url utilities within ESM [1]. 2. Limitations with Dynamic Require: Enabling shims does not automatically provide a global require function in ESM [5]. If your code performs dynamic require() calls (a CJS-specific feature), it will fail in an ESM environment because ESM does not natively support require [3][4]. 3. Recommended Workarounds: - Manual Shim Injection: To support dynamic require in ESM, developers must often manually create a require function using createRequire from the node:module package [5][3]. - Configuration Pattern: A common approach involves using tsup's inject option to point to a custom shim file that defines globalThis.require = createRequire(import.meta.url) [5]. Alternatively, some developers use the banner option to prepend this definition to the generated ESM output [3]. In summary, the shims option handles specific variable compatibility (dirname/filename) but does not resolve dynamic require limitations in ESM [1][2]. Resolving dynamic require calls typically requires manual injection of a createRequire polyfill [5][3].
Citations:
- 1: https://readoss.com/en/egoist/tsup/the-esbuild-build-pipeline-orchestration-plugins-and-output-processing
- 2: https://tsup.egoist.dev/
- 3: Dynamic require of "react" is not supported egoist/tsup#927
- 4: Overcome path and fs dynamic import error egoist/tsup#1130
- 5: https://nibras.co/fixing-tsup-dynamic-require-errors/
Fix tsup shims explanation/mechanism: shims: true usually doesn’t provide createRequire for ESM dynamic require()
packages/ai-mcp/tsup.bin.config.ts comments claim shims: true “inject[s] createRequire” so CJS require() calls from json-schema-to-typescript work in the ESM bundle, but tsup’s shims option is primarily for ESM/Node global compatibility (e.g., dirname/filename) and is not generally a solution for ESM “Dynamic require” failures. To address that crash, use tsup’s inject/banner to define a require via createRequire(import.meta.url), and update the inline comments to match the actual mechanism.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/ai-mcp/tsup.bin.config.ts` around lines 13 - 15, The comment
claiming "shims: true injects createRequire" is incorrect; update the inline
comment in tsup config to explain that shims only provide ESM/Node global
compatibility and do not polyfill dynamic require, and then add a tsup
injection/banner that defines a require using Node's createRequire (e.g., use
createRequire(import.meta.url) to set a global require helper) so
json-schema-to-typescript's dynamic require() calls will work in the ESM bundle;
specifically modify the tsup config where "shims: true" is set to replace the
comment and add an "inject" or "banner" entry that imports createRequire from
"module" and assigns const require = createRequire(import.meta.url) (or attaches
it to globalThis), ensuring the bundle provides a working require at runtime.
1 participant
Summary
@tanstack/ai-mcp0.1.0 ships its CLI as an ESM bundle withjson-schema-to-typescriptinlined vianoExternal. That library usesrequire()internally, which is not available in ESM-output bundles — so the CLI crashes on startup with:before it can print help or run codegen.
Changes
packages/ai-mcp/tsup.bin.config.ts: addshims: truetsup'sshimsoption injects acreateRequire(import.meta.url)shim at the top of the ESM bundle, making CJS dependencies'require()calls work correctly. This is the standard fix for CJS-in-ESM bundling with tsup.Fixes #753
Changes
packages/ai-mcp/tsup.bin.config.ts: addshims: trueSummary by CodeRabbit