fix(compile): wasm files might not be a part of module graph

[CertainLach]

Fixes: #31456

deno compile analyzes all --included files as module graph roots, however wasm files might not be used in module graph at all, and instead fetch()ed and WebAssembly.instantiated, in which case their imports should not be analyzed.

It should not break imported wasm files, as they will be added to module graph as a dependency anyway.

[coderabbitai]

Walkthrough

Internal helper predicates in the compile module are renamed for clarity: is_module_graph_module becomes is_module_graph_root_module and is_module_graph_media_type becomes is_module_graph_root_media_type. The root-media-type logic is extended to explicitly handle Wasm as non-root-eligible with accompanying comments. All call sites are updated to reference the renamed predicates. Existing behavior for non-file schemes remains unchanged. The match arms are expanded to differentiate which media types qualify as root-eligible, reflecting the new naming convention.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title directly addresses the main change: preventing wasm files from being treated as module graph roots, which matches the core functionality fix in the PR.
Description check	✅ Passed	The description explains the issue (wasm imports being resolved when they shouldn't be), references the linked issue, and clarifies that the fix prevents analysis of wasm files not part of the module graph.
Linked Issues check	✅ Passed	The PR changes directly address issue #31456 by modifying root-media-type logic to exclude wasm files from module graph analysis, preventing unwanted import resolution.
Out of Scope Changes check	✅ Passed	The changes are focused on renaming predicates and adjusting wasm handling in the compile logic, all aligned with fixing wasm file module graph treatment.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 76068b2 and cc9e180.

📒 Files selected for processing (1)

• cli/tools/compile.rs (3 hunks)

🧰 Additional context used

📓 Path-based instructions (2)

cli/tools/**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

CLI tools should be implemented in cli/tools/<tool> or cli/tools/<tool>/mod.rs

Files:

• cli/tools/compile.rs

**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.rs: For debugging Rust code, set breakpoints in IDE debuggers (VS Code with rust-analyzer, IntelliJ IDEA) or use lldb directly
Use eprintln!() or dbg!() macros for debug prints in Rust code

Files:

• cli/tools/compile.rs

🧠 Learnings (1)

📚 Learning: 2025-11-24T16:19:37.808Z

Learnt from: CR
Repo: denoland/deno PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:19:37.808Z
Learning: Applies to cli/module_loader.rs : Module loading and resolution is handled in `cli/module_loader.rs`

Applied to files:

• cli/tools/compile.rs

🧬 Code graph analysis (1)

cli/tools/compile.rs (1)

libs/resolver/emit.rs (3)

• media_type (395-395)
• media_type (413-415)
• media_type (435-437)

🔇 Additional comments (3)

cli/tools/compile.rs (3)

349-354: Helper rename and root-only semantics look correct

Constraining is_module_graph_root_module to “root-ness” and delegating only file URLs to media-type inspection matches how get_module_roots_and_include_paths uses it. Non-file includes still become roots, and file includes are filtered by media type as intended.

---

356-381: Wasm exclusion from module graph roots aligns with compile behavior

The media-type filter now explicitly excludes MediaType::Wasm from being a graph root while still allowing all JS/TS variants as roots. That ensures .wasm files provided via --include or discovered under included directories are treated as side assets unless actually imported, which is exactly what’s needed to avoid resolving their internal imports.

---

427-432: Include handling now avoids unnecessary wasm graph roots

Using is_module_graph_root_module for each --include and is_module_graph_root_media_type inside analyze_path means:

• Direct .wasm includes no longer become module roots.
• .wasm files under included directories are skipped as roots but remain available via include_paths.

This should prevent the reported “import not a dependency” errors for runtime-instantiated wasm without affecting wasm modules that are actually imported.

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[CertainLach]

Renamed for proper semantics: Wasm files might be a module graph modules, but they are never a module graph roots

This function is only used to determine roots

[CertainLach]

I don't have an issue with json files, but they should not be part of module graph unless being imported either.

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

cli/tools/compile.rs (1)

356-381: Convert doc comments (///) to regular comments (//) in the MediaType::Wasm match arm to avoid compiler warnings

The /// lines before MediaType::Wasm are doc comments inside a function body. Rust's compiler warns about unused_doc_comments because doc comments are only for documentation items (functions, structs, etc.), not statements. Use regular comments instead.

Suggested change:

-      /// Wasm is never a module graph root, and might be used outside of an import statement,
-      /// fetched() wasm files have their dependencies specified explicitly on instantiation.
+      // Wasm is never a module graph root, and might be used outside of an import statement.
+      // fetch()'d wasm files have their dependencies specified explicitly on instantiation.
       MediaType::Wasm => false,

🧹 Nitpick comments (1)

cli/tools/compile.rs (1)

349-355: Confirm intended behavior for non-file wasm --include URLs

The new helpers correctly prevent local .wasm files (and wasm under included directories) from becoming module graph roots while still being present in include_paths, which addresses the reported issue for file-based includes.

However, is_module_graph_root_module returns true for all non-file schemes, so something like --include=https://example.com/foo.wasm would still make that wasm a graph root and subject to import analysis. That preserves existing behavior but means the fix is scoped to file-based includes only.

If you also want remote or other non-file .wasm includes to avoid module-graph import analysis, you could consider delegating to is_module_graph_root_media_type(MediaType::from_specifier(url)) for non-file schemes as well (or special-case wasm there).

Please double-check that limiting the change to file-based wasm includes matches the desired behavior for deno compile --include and that there are no user scenarios relying on remote wasm includes being treated as module graph roots.

Also applies to: 427-433

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between efa4da8 and 76068b2.

📒 Files selected for processing (1)

• cli/tools/compile.rs (3 hunks)

🧰 Additional context used

📓 Path-based instructions (2)

cli/tools/**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

CLI tools should be implemented in cli/tools/<tool> or cli/tools/<tool>/mod.rs

Files:

• cli/tools/compile.rs

**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.rs: For debugging Rust code, set breakpoints in IDE debuggers (VS Code with rust-analyzer, IntelliJ IDEA) or use lldb directly
Use eprintln!() or dbg!() macros for debug prints in Rust code

Files:

• cli/tools/compile.rs

🧠 Learnings (1)

📚 Learning: 2025-11-24T16:19:37.808Z

Learnt from: CR
Repo: denoland/deno PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:19:37.808Z
Learning: Applies to cli/module_loader.rs : Module loading and resolution is handled in `cli/module_loader.rs`

Applied to files:

• cli/tools/compile.rs

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)

• GitHub Check: test release linux-x86_64
• GitHub Check: test debug windows-x86_64
• GitHub Check: test debug linux-aarch64
• GitHub Check: test debug linux-x86_64
• GitHub Check: test debug macos-aarch64
• GitHub Check: test debug macos-x86_64
• GitHub Check: lint debug windows-x86_64
• GitHub Check: lint debug macos-x86_64
• GitHub Check: lint debug linux-x86_64
• GitHub Check: build libs

[CertainLach]

Was testing this change, and have encountered that fetching wasm doesn't work, as it seems like fetch() with file urls does not support reading files from DenoRtSys

This does not affect utility of this patch for cases when the wasm files are only used for frontend asset storage purposes, and if someone wants to use wasm file in deno itself - it is still possible by using WebAssembly.instantiate(new WebAssembly.Module(await Deno.readFile(...)))

[dsherret]

The two_times_compile_include_all test is now failing. Can you update the expectation?

Also, can you add a test for this in the tests/specs/compile folder? Just add a folder like some of the other tests, then run cargo test specs::compile to run the tests in that folder.