Skip to content

fix(prerender): make CJS compat plugin work with Vite 6 nodeRunnerEnv#1735

Closed
lisa-assistant wants to merge 1 commit into
cedarjs:mainfrom
lisa-assistant:lisa/fix-node-runner-cjs-compat
Closed

fix(prerender): make CJS compat plugin work with Vite 6 nodeRunnerEnv#1735
lisa-assistant wants to merge 1 commit into
cedarjs:mainfrom
lisa-assistant:lisa/fix-node-runner-cjs-compat

Conversation

@lisa-assistant
Copy link
Copy Markdown
Contributor

Problem

node-runner.ts imports cedarCjsCompatPlugin from @cedarjs/vite, but that plugin has two issues that cause it to silently skip transforming CJS files when used with a non-default Vite environment (nodeRunnerEnv):

  1. buildStart is not guaranteed to fire for non-default environments — so cjs-module-lexer never gets initialized, and lexerParse stays null.
  2. async transform can be skipped in non-default environments — CJS files pass through untransformed, causing 'module' is not defined errors at runtime.

The result: 15 of 16 node-runner tests fail.

Fix

Replaces the imported plugin with a self-contained cjsCompatPlugin local to node-runner.ts that:

  • Initializes cjs-module-lexer eagerly at module load (a module-level Promise awaited in createViteServer before the server is created), so it's always ready before any transform runs.
  • Uses a synchronous transform hook that works reliably in all Vite environments.
  • Falls back to a brace-counting parser (extractCjsNamedExports) for hand-written CJS with function values that cjs-module-lexer cannot statically detect (e.g. module.exports = { handler: function() {} }).

Test results

All 16 node-runner tests now pass, including:

  • uses cedarjsJobPathInjectorPlugin to handle job files without errors — imports @cedarjs/jobs (esbuild-compiled CJS with PrismaAdapter)
  • imports CommonJS module — hand-written CJS with function values

The cedarCjsCompatPlugin from @cedarjs/vite uses an async transform hook
and relies on buildStart for lexer initialization. Neither is reliable for
non-default Vite environments like nodeRunnerEnv — buildStart may not fire
and async transforms can be skipped, causing CJS files to pass through
untransformed and fail with 'module is not defined'.

This replaces the imported plugin with a self-contained cjsCompatPlugin
local to node-runner.ts that:
- Initializes cjs-module-lexer eagerly at module load (before createServer)
- Uses a synchronous transform hook that works in all Vite environments
- Falls back to a brace-counting parser for hand-written CJS with function
  values that cjs-module-lexer cannot statically detect

All 16 node-runner tests now pass, including the cedarjsJobPathInjectorPlugin
test that imports @cedarjs/jobs (esbuild-compiled CJS with PrismaAdapter).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@netlify
Copy link
Copy Markdown

netlify Bot commented May 10, 2026

👷 Deploy request for cedarjs pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit 4be8fea

@github-actions github-actions Bot added this to the next-release-patch milestone May 10, 2026
@nx-cloud
Copy link
Copy Markdown

nx-cloud Bot commented May 10, 2026

🤖 Nx Cloud AI Fix

Ensure the fix-ci command is configured to always run in your CI pipeline to get automatic fixes in future runs. For more information, please see https://nx.dev/ci/features/self-healing-ci


View your CI Pipeline Execution ↗ for commit 4be8fea

Command Status Duration Result
nx run-many -t test --minWorkers=1 --maxWorkers=4 ✅ Succeeded 1m 19s View ↗
nx run-many -t build:pack --exclude create-ceda... ✅ Succeeded 9s View ↗
nx run-many -t build ✅ Succeeded 25s View ↗
nx run-many -t test:types ✅ Succeeded 7s View ↗

☁️ Nx Cloud last updated this comment at 2026-05-10 09:19:49 UTC

@nx-cloud
Copy link
Copy Markdown

nx-cloud Bot commented May 10, 2026

🤖 Nx Cloud AI Fix

Ensure the fix-ci command is configured to always run in your CI pipeline to get automatic fixes in future runs. For more information, please see https://nx.dev/ci/features/self-healing-ci


View your CI Pipeline Execution ↗ for commit 4be8fea

Command Status Duration Result
nx run-many -t build ✅ Succeeded 15s View ↗

☁️ Nx Cloud last updated this comment at 2026-05-10 09:13:45 UTC

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 10, 2026

Greptile Summary

This PR fixes a real problem: buildStart and async transform hooks don't reliably fire for Vite's non-default nodeRunnerEnv, causing CJS modules to pass through untransformed. The solution — eagerly initializing cjs-module-lexer at module load and switching to a synchronous transform hook — correctly addresses those two root causes.

  • The local cjsCompatPlugin correctly handles eager lexer init and synchronous transforms, and the extractCjsNamedExports brace-counting fallback works for hand-written CJS with function values.
  • The new plugin drops two behaviors from the original cedarCjsCompatPlugin: (1) the __esModule flag check that unwraps module.exports.default for TypeScript-/Babel-compiled packages with a default export, causing silent wrong-value default imports; (2) the UMD pattern guard that threw an explicit error instead of silently producing broken output.
  • The PR description refers to "Vite 6 nodeRunnerEnv" but packages/vite/package.json pins vite at 7.3.2 — the project overview doc (2026-03-26-cedarjs-project-overview.md) correctly lists Vite 7 and remains factually accurate; only the PR description has the stale version reference.

Confidence Score: 3/5

The transform fix is sound for the tested cases, but the missing __esModule unwrapping silently breaks default imports of TypeScript- or Babel-compiled CJS packages that carry a .default property.

The eager-init and synchronous-transform changes correctly solve the stated problem, and the brace-counting fallback adds useful coverage. However, any TypeScript-compiled npm package that has __esModule: true and a default export will have its import Foo from '...' silently return the wrong value — the full module.exports wrapper object rather than the actual export. This regression is not covered by the 16 tests.

packages/prerender/src/graphql/node-runner.ts — specifically the cjsCompatPlugin transform output section and named-export filtering logic.

Important Files Changed

Filename Overview
packages/prerender/src/graphql/node-runner.ts Replaces the shared cedarCjsCompatPlugin with a local cjsCompatPlugin that eagerly initializes cjs-module-lexer and uses a synchronous transform hook to fix Vite non-default environment compatibility, but omits the __esModule default-export unwrapping, silently breaking default imports of TypeScript-compiled CJS modules with a default export.

Reviews (1): Last reviewed commit: "fix(prerender): make CJS compat plugin w..." | Re-trigger Greptile

Comment on lines +192 to +213
const namedExportLines = [...namedExports]
.map(
(name) =>
`export const ${name} = __cjs_result__[${JSON.stringify(name)}]`,
)
.join('\n')

return {
code: `
import { createRequire as __createRequire__ } from 'node:module'
const require = __createRequire__(${filePath})
const module = { exports: {} }
const exports = module.exports
const __dirname = ${dirPath}
const __filename = ${filePath}
;(function() {
${code}
}).call(module.exports)
const __cjs_result__ = module.exports
export default __cjs_result__
${namedExportLines}
`,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Missing __esModule default-export unwrapping

The original cedarCjsCompatPlugin detects the __esModule: true flag (set by TypeScript's compiler and Babel when a module has a real default export) and unwraps module.exports.default before emitting export default. The new plugin always emits export default __cjs_result__, so a consumer that does import Foo from './ts-compiled-module' on a TypeScript-compiled CJS file with a default export will receive the entire module.exports wrapper object ({ __esModule: true, default: ActualFoo, ... }) instead of ActualFoo. Because exports.default = ActualFoo matches the heuristic regex, the file is transformed, and the regression is silent.

Comment on lines +176 to +186
if (lexerParse) {
try {
const { exports } = lexerParse(code)
for (const e of exports) {
if (/^[a-zA-Z_$][a-zA-Z0-9_$]*$/.test(e) && e !== 'default') {
namedExports.add(e)
}
}
} catch {
// Ignore — extractCjsNamedExports result is still used
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 __esModule leaks into named exports

The named-export filter (e !== 'default') does not exclude __esModule. Both cjs-module-lexer and extractCjsNamedExports may detect it, causing the generated wrapper to emit export const __esModule = __cjs_result__["__esModule"]. The original plugin explicitly filtered __esModule out of the named-export list with e !== '__esModule' && e !== 'default'.

Comment on lines +160 to +169
transform(code, id) {
// Only handle plain .js / .cjs files — TypeScript and JSX are already
// transformed by Vite's esbuild plugin and will be valid ESM.
if (!/\.[cm]?js$/.test(id)) {
return null
}

// Quick heuristic: skip files that don't look like CJS
if (!/\bmodule\.exports\b|\bexports\.\w+/.test(code)) {
return null
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 No UMD guard — silently produces broken output

The original plugin detected UMD wrappers (typeof module.exports === 'object', typeof define === 'function', etc.) and threw an explicit, actionable error. The new plugin skips that check entirely: a UMD file passes the heuristic (module.exports is present), gets wrapped in the CJS shim, and produces broken runtime output with no diagnostic message. Adding the same UMD regex checks from the original plugin before the transform would preserve the fail-fast behavior.

@lisa-assistant
Copy link
Copy Markdown
Contributor Author

Closing — the upstream implementation in @cedarjs/vite works correctly once the package is built. My local test failures were because I hadn't rebuilt @cedarjs/vite after #1717 merged (cedarCjsCompatPlugin was in source but not in dist). CI builds all packages before running tests so it never exhibited the problem. Sorry for the noise.

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.

1 participant