Skip to content

fix(deno_facade): allow non-MainModule resolution when referrer is "."#704

Merged
nyannyacha merged 1 commit into
supabase:mainfrom
0x5457:fix/cjs-require-esm
Jun 10, 2026
Merged

fix(deno_facade): allow non-MainModule resolution when referrer is "."#704
nyannyacha merged 1 commit into
supabase:mainfrom
0x5457:fix/cjs-require-esm

Conversation

@0x5457

@0x5457 0x5457 commented May 17, 2026

Copy link
Copy Markdown
Contributor

What kind of change does this PR introduce?

Bug fix

What is the current behavior?

EmbeddedModuleLoader::resolve rejects any resolution where referrer == "."
unless kind == ResolutionKind::MainModule:

if kind != ResolutionKind::MainModule {
    return Err(generic_error(format!(
        "Expected to resolve main module, got {:?} instead.",
        kind
    )));
}

This breaks Deno's CJS→ESM bridge. When a CommonJS module synchronously
require()s a pure-ESM module, loadESMFromCJS re-enters resolve with
ResolutionKind::Import and the same "." referrer, and the worker boots
with:

Error: Expected to resolve main module, got Import instead.
    at loadESMFromCJS (node:module:777)

fix #703

What is the new behavior?

After the fix, require("./message.mjs") and require("chalk") (a
"type": "module" package) both load correctly from a CJS entrypoint.

@nyannyacha

nyannyacha commented May 18, 2026

Copy link
Copy Markdown
Contributor

Hello @0x5457 😋
Thank you for submitting this.

Have you tried running deno compile in Deno v2.1.4?
The module loader implementation in the edge-runtime codebase reflects the EmbeddedModuleLoader implementation in the deno compile subcommand path of Deno v2.1.4 codebase, so it appears to behave consistently; however, deno run seems to work without error, which suggests an inconsistency.

I think this change is acceptable, but do you have any special comments regarding the above?

@0x5457

0x5457 commented May 18, 2026

Copy link
Copy Markdown
Contributor Author

Hi @nyannyacha, thanks for the review!

I haven't tried deno compile on v2.1.4 directly, but I noticed that upstream Deno hit the same issue and fixed it in denoland/deno#28379. That PR removes the exact same if kind != ResolutionKind::MainModule { ... } guard from EmbeddedModuleLoader::resolve in cli/rt/run.rs, its description explicitly mentions "fixes a bug where require esm wasn't working in deno compile", which matches what we're seeing here.

https://github.com/denoland/deno/pull/28379/changes#diff-c30a4785f54c02866be5d07ef0ce993d0518eeea81a85344ed953de6bbcf852cL156-L167

@nyannyacha nyannyacha force-pushed the fix/cjs-require-esm branch from 2cb65a5 to 1f34420 Compare June 9, 2026 07:25
@nyannyacha

Copy link
Copy Markdown
Contributor

Some tests seem a bit flaky due to third-party sources, but as the fix itself appears to be fine, so LTGM.
Thank you for your efforts! 😄

@nyannyacha nyannyacha merged commit 3baa26e into supabase:main Jun 10, 2026
6 of 9 checks passed
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.

Bug: require()-ing a pure-ESM module from CJS crashes the worker

2 participants