Skip to content

build: add rollup plugin for compile-time ESM/CJS code branching #21715

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
timfish merged 5 commits into from
Jun 24, 2026
+50 −34
Merged

build: add rollup plugin for compile-time ESM/CJS code branching #21715

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6019e74
build: Bundle time ESM/CJS code
timfish Jun 23, 2026
5624872
Lint
timfish Jun 23, 2026
e9506b4
improve whitespace
timfish Jun 23, 2026
0a4b3f2
Merge branch 'develop' into timfish/build/bundle-time-cjs-esm
timfish Jun 24, 2026
ddd1e16
Merge branch 'develop' into timfish/build/bundle-time-cjs-esm
timfish Jun 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 16 additions & 19 deletions dev-packages/rollup-utils/npmHelpers.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { defineConfig } from 'rollup';
import {
makeDebugBuildStatementReplacePlugin,
makeEsbuildPlugin,
makeEsmCjsReplacePlugin,
makeNodeResolvePlugin,
makeProductionReplacePlugin,
makeRrwebBuildPlugin,
Expand Down Expand Up @@ -129,40 +130,36 @@ export function makeNPMConfigVariants(baseConfig, options = {}) {

if (emitCjs) {
if (splitDevProd) {
variantSpecificConfigs.push({ output: { format: 'cjs', dir: path.join(baseConfig.output.dir, 'cjs/dev') } });
variantSpecificConfigs.push({
output: { format: 'cjs', dir: path.join(baseConfig.output.dir, 'cjs/dev') },
plugins: [makeEsmCjsReplacePlugin('cjs')],
});
variantSpecificConfigs.push({
output: { format: 'cjs', dir: path.join(baseConfig.output.dir, 'cjs/prod') },
plugins: [makeProductionReplacePlugin()],
plugins: [makeProductionReplacePlugin(), makeEsmCjsReplacePlugin('cjs')],
});
} else {
variantSpecificConfigs.push({ output: { format: 'cjs', dir: path.join(baseConfig.output.dir, 'cjs') } });
variantSpecificConfigs.push({
output: { format: 'cjs', dir: path.join(baseConfig.output.dir, 'cjs') },
plugins: [makeEsmCjsReplacePlugin('cjs')],
});
}
}

if (emitEsm) {
if (splitDevProd) {
variantSpecificConfigs.push({
output: {
format: 'esm',
dir: path.join(baseConfig.output.dir, 'esm/dev'),
plugins: [makePackageNodeEsm()],
},
output: { format: 'esm', dir: path.join(baseConfig.output.dir, 'esm/dev') },
plugins: [makePackageNodeEsm(), makeEsmCjsReplacePlugin('esm')],
});
variantSpecificConfigs.push({
output: {
format: 'esm',
dir: path.join(baseConfig.output.dir, 'esm/prod'),
plugins: [makePackageNodeEsm()],
},
plugins: [makeProductionReplacePlugin()],
output: { format: 'esm', dir: path.join(baseConfig.output.dir, 'esm/prod') },
plugins: [makePackageNodeEsm(), makeProductionReplacePlugin(), makeEsmCjsReplacePlugin('esm')],
});
} else {
variantSpecificConfigs.push({
output: {
format: 'esm',
dir: path.join(baseConfig.output.dir, 'esm'),
plugins: [makePackageNodeEsm()],
},
output: { format: 'esm', dir: path.join(baseConfig.output.dir, 'esm') },
plugins: [makePackageNodeEsm(), makeEsmCjsReplacePlugin('esm')],
});
}
}
Expand Down
36 changes: 27 additions & 9 deletions dev-packages/rollup-utils/plugins/npmPlugins.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,20 +120,38 @@ export function makeDebugBuildStatementReplacePlugin() {
return plugin;
}

export function makeProductionReplacePlugin() {
// Markers use the `/*! ... */` legal-comment syntax so esbuild preserves them through
// transpile. We still run as a `transform` (per-module) hook rather than `renderChunk`:
// the block typically uses imports declared at the module top, and stripping it before
// rollup analyses module-graph imports lets those now-unused imports be tree-shaken away.
// The plugin sort order in utils.mjs pins this before `esbuild`.
const pattern =
/\/\*! rollup-include-development-only \*\/[\s\S]*?\/\*! rollup-include-development-only-end \*\/\s*/g;
// Markers use the `/*! ... */` legal-comment syntax so esbuild preserves them through
// transpile. We still run as a `transform` (per-module) hook rather than `renderChunk`:
// the block typically uses imports declared at the module top, and stripping it before
// rollup analyses module-graph imports lets those now-unused imports be tree-shaken away.
// The plugin sort order in utils.mjs pins this before `esbuild`.
const REMOVE_DEV_BLOCK =
/\/\*! rollup-include-development-only \*\/[\s\S]*?\/\*! rollup-include-development-only-end \*\/\s*/g;

export function makeProductionReplacePlugin() {
return {
name: 'remove-dev-mode-blocks',
transform(code) {
if (!code.includes('rollup-include-development-only')) return null;
return { code: code.replace(pattern, ''), map: null };
return { code: code.replace(REMOVE_DEV_BLOCK, ''), map: null };
},
};
}

const REMOVE_CJS_BLOCK = /\/\*! rollup-include-cjs-only \*\/[\s\S]*?\/\*! rollup-include-cjs-only-end \*\/\s*/g;
const REMOVE_ESM_BLOCK = /\/\*! rollup-include-esm-only \*\/[\s\S]*?\/\*! rollup-include-esm-only-end \*\/\s*/g;
const STRIP_CJS_MARKERS = /[ \t]*\/\*! rollup-include-cjs-only(?:-end)? \*\/[ \t]*\r?\n?/g;
const STRIP_ESM_MARKERS = /[ \t]*\/\*! rollup-include-esm-only(?:-end)? \*\/[ \t]*\r?\n?/g;

export function makeEsmCjsReplacePlugin(type) {
const removeBlock = type === 'esm' ? REMOVE_CJS_BLOCK : REMOVE_ESM_BLOCK;
const stripMarkers = type === 'esm' ? STRIP_ESM_MARKERS : STRIP_CJS_MARKERS;

return {
name: 'remove-esm-cjs-mode-blocks',
transform(code) {
if (!code.includes('rollup-include-')) return null;
return { code: code.replace(removeBlock, '').replace(stripMarkers, ''), map: null };

@cursor cursor Bot Jun 24, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

ESM/CJS strip runs after esbuild

Medium Severity

The new remove-esm-cjs-mode-blocks transform is not pinned before esbuild in mergePlugins, unlike remove-dev-mode-blocks. Format-specific blocks are stripped only after transpilation, so top-level imports used solely in a removed branch can remain in the module graph and ship in the wrong format bundle.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit ddd1e16. Configure here.

@timfish timfish Jun 24, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

we're not using esbuild

},
};
}
Expand Down
13 changes: 7 additions & 6 deletions packages/node-core/src/utils/detection.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,13 @@ import { NODE_MAJOR, NODE_MINOR } from '../nodeVersion';

/** Detect CommonJS. */
export function isCjs(): boolean {
try {
// oxlint-disable-next-line typescript/prefer-optional-chain
return typeof module !== 'undefined' && typeof module.exports !== 'undefined';
} catch {
return false;
}
/*! rollup-include-cjs-only */
return true;
/*! rollup-include-cjs-only-end */

/*! rollup-include-esm-only */
return false;
/*! rollup-include-esm-only-end */

@cursor cursor Bot Jun 24, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Source isCjs always true

Medium Severity

Replacing runtime detection with two unconditional return statements means any execution of the TypeScript source without the Rollup strip step always hits the first return true. Unit tests and other tooling that import from src (for example Vitest in packages/node-core) no longer mirror ESM behavior for isCjs() or callers like supportsEsmLoaderHooks().

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit ddd1e16. Configure here.

@timfish timfish Jun 24, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Look at the PR. These blocks are removed at build time.

}

let hasWarnedAboutNodeVersion: boolean | undefined;
Expand Down
Loading