Skip to content

fix: subpath exports resolution for CJS files in dual-format packages #227

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

Open
RizonSunny wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: subpath exports resolution for CJS files in dual-format packages #227

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
17 changes: 8 additions & 9 deletions lib/follow.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,19 +73,19 @@ export function follow(x: string, opts: FollowOptions) {
} else if (opts.extensions) {
extensions = [opts.extensions as string];
} else {
extensions = ['.js', '.json', '.node'];
extensions = ['.js', '.json', '.node', '.mjs', '.cjs'];
Copy link
Copy Markdown
Member

@robertsLando robertsLando Apr 7, 2026

Choose a reason for hiding this comment

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

This extensions array is now duplicated in three places (walker.ts:80, follow.ts:76, resolver.ts:152). Please extract it to a shared constant (e.g. export MODULE_RESOLVE_EXTENSIONS from common.ts) and import it everywhere.

}

const result = resolveModule(x, {
basedir: opts.basedir || process.cwd(),
extensions,
});

// Only use ESM resolution result if it's an actual ESM package
// For CJS packages, fall through to standard CommonJS resolution
// to ensure all callbacks (catchReadFile, catchPackageFilter) are handled correctly
if (result.isESM) {
// This is a real ESM package, handle it here
// Use the exports-aware resolver result whenever it succeeds,
// regardless of whether the resolved file is ESM or CJS.
// The "exports" field is a package-level feature, not ESM-only —
// dual-format packages use it to point require() to .cjs files.
if (result.resolvedViaExports) {
if (opts.catchReadFile) {
// Find the package.json for this resolved module
let currentDir = path.dirname(result.resolved);
Expand Down Expand Up @@ -132,13 +132,12 @@ export function follow(x: string, opts: FollowOptions) {
}
}

// ESM package resolved successfully
// Exports-aware resolver succeeded
resolve(result.resolved);
return;
}

// CJS package - fall through to standard CommonJS resolution
// to handle all callbacks properly
// No exports field found - fall through to standard CommonJS resolution
} catch (_error) {
// ESM resolution failed - fall through to standard CommonJS resolution
}
Expand Down
5 changes: 4 additions & 1 deletion lib/resolver.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ interface ResolveOptions {
interface ResolveResult {
resolved: string;
isESM: boolean;
resolvedViaExports: boolean;
}

/**
Expand Down Expand Up @@ -149,7 +150,7 @@ export function resolveModule(
specifier: string,
options: ResolveOptions,
): ResolveResult {
Copy link
Copy Markdown
Member

@robertsLando robertsLando Apr 7, 2026

Choose a reason for hiding this comment

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

Same here — import the shared MODULE_RESOLVE_EXTENSIONS constant instead of hardcoding.

const { basedir, extensions = ['.js', '.json', '.node'] } = options;
const { basedir, extensions = ['.js', '.json', '.node', '.mjs', '.cjs'] } = options;

// First, try ESM-style resolution with exports field
const esmResolved = tryResolveESM(specifier, basedir);
Expand All @@ -158,6 +159,7 @@ export function resolveModule(
return {
resolved: esmResolved,
isESM: isESMFile(esmResolved),
resolvedViaExports: true,
};
}

Expand All @@ -170,5 +172,6 @@ export function resolveModule(
return {
resolved,
isESM: false, // CJS resolution
resolvedViaExports: false,
};
}
13 changes: 2 additions & 11 deletions lib/walker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import { pc } from './colors';
import { follow } from './follow';
import { log, wasReported } from './log';
import * as detector from './detector';
import { transformESMtoCJS, rewriteMjsRequirePaths } from './esm-transformer';
import { transformESMtoCJS } from './esm-transformer';
Copy link
Copy Markdown
Member

@robertsLando robertsLando Apr 7, 2026

Choose a reason for hiding this comment

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

Please keep the rewriteMjsRequirePaths import and usage — it's needed for .mjs files that get transformed to CJS. The packer renames .mjs → .js in the snapshot, and without this rewrite, require('./foo.mjs') paths generated by esbuild will point to files that no longer exist.

import {
ConfigDictionary,
FileRecord,
Expand Down Expand Up @@ -77,7 +77,7 @@ const win32 = process.platform === 'win32';

// Extensions to try when resolving modules
// Includes .mjs to support ESM files that get transformed to .js
const MODULE_RESOLVE_EXTENSIONS = ['.js', '.json', '.node', '.mjs'];
const MODULE_RESOLVE_EXTENSIONS = ['.js', '.json', '.node', '.mjs', '.cjs'];

/**
* Checks if a module is a core module
Expand Down Expand Up @@ -1121,15 +1121,6 @@ class Walker {
const derivatives2: Derivative[] = [];
stepDetect(record, marker, derivatives2);
await this.stepDerivatives(record, marker, derivatives2);

// After dependencies are resolved, rewrite .mjs require paths to .js
// since the packer renames .mjs files to .js in the snapshot
if (record.wasTransformed && record.body) {
record.body = Buffer.from(
rewriteMjsRequirePaths(record.body.toString('utf8')),
'utf8',
);
}
}
}

Expand Down
Loading