Skip to content

fix(compile): resolve JSON requires from ESM module paths - #10874

Closed
proggeramlug wants to merge 1 commit into
mainfrom
fix/10758-mongodb-json-require
Closed

proggeramlug wants to merge 1 commit into
mainfrom
fix/10758-mongodb-json-require

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

Closes #10758

MongoDB is compiled from its ESM-shaped TypeScript source, but client_metadata.ts retains a literal require("../../../package.json"). Perry previously added the JSON file to the graph while leaving the call to the entry-relative global require, producing MODULE_NOT_FOUND.

This replaces resolved JSON requires with the JSON loader default binding only for unwrapped ESM modules. CJS-wrapped modules retain their per-module require/cache path.

Tests:

  • cargo test -p perry --bin perry replaces_resolved_json_require_in_unwrapped_esm_module
  • cargo test -p perry --test source_graph_export_regressions esm_module_resolves_static_json_require_from_its_own_directory -- --nocapture (remote Linux, matching static runtime)
  • MongoDB 7.5.0's full 162-module source graph passed collection and codegen without the original package.json resolution failure; final linking was blocked by an unrelated stale optional runtime archive on the shared remote host.

Summary by CodeRabbit

  • Bug Fixes
    • Fixed static relative JSON require() calls in ESM-shaped TypeScript so they resolve from the requiring module’s directory.
    • Packages can now read their own package.json metadata without encountering MODULE_NOT_FOUND during setup.
  • Tests
    • Added regression coverage confirming correct package version resolution from ESM source graphs.

@coderabbitai

coderabbitai Bot commented Sep 21, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

Understand this PR’s impact

Explore downstream dependencies and potential security impact with Blast Radius.

View blast radius →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: eb54c7da-302f-405f-bbf1-b1db874cd616

📥 Commits

Reviewing files that changed from the base of the PR and between 34f8a0e and b7081db.

📒 Files selected for processing (1)
  • crates/perry/src/commands/compile/collect_modules.rs

Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review.


📝 Walkthrough

Walkthrough

The compiler now replaces selected static relative JSON require() calls in unwrapped ESM-shaped TypeScript with default imports. The change includes transform coverage and a MongoDB source-graph regression test.

Changes

Static JSON require resolution

Layer / File(s) Summary
JSON require transformation
crates/perry/src/commands/compile/collect_modules.rs, crates/perry/src/commands/compile/collect_modules/static_require_transform.rs
The transform accepts a JSON-replacement flag. Unwrapped modules pass the flag, and resolved .json requires become generated default-import bindings.
Regression validation and changelog
crates/perry/tests/source_graph_export_regressions.rs, crates/perry/tests/source_graph_export_regressions/issue_10758.rs, changelog.d/10758-esm-static-json-require.md
Tests cover a nested MongoDB package graph and assert the resolved package version. The changelog documents the behavior.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant collect_module_one
  participant static_require_transform
  participant package_json
  collect_module_one->>static_require_transform: transform unwrapped ESM module
  static_require_transform->>package_json: resolve relative JSON target
  static_require_transform->>static_require_transform: generate default import binding
  static_require_transform-->>collect_module_one: return source with replaced binding
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely identifies the main change: resolving JSON requires from ESM module paths.
Description check ✅ Passed The description explains the issue, the implementation, the related issue, and the tests performed. It does not use the template headings or include the checklist confirmations, but it contains the re…
Linked Issues check ✅ Passed Issue #10758 requires JSON require() resolution for MongoDB's ESM-shaped source and regression coverage for the real module-graph failure. The PR passes !was_cjs_wrapped to the static-require tran…
Out of Scope Changes check ✅ Passed The changes modify JSON static-require transformation, select behavior by module wrapping state, add regression coverage for issue #10758, and add a related changelog entry. Each change supports the l…
Docstring Coverage ✅ Passed Docstring coverage is 85.71% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 4 files.
✨ Finishing Touches 💡 1
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@proggeramlug
proggeramlug force-pushed the fix/10758-mongodb-json-require branch from 34f8a0e to b7081db Compare September 21, 2026 10:45
proggeramlug pushed a commit that referenced this pull request Sep 21, 2026
The file was at exactly 2000 on main, so any PR adding a line fails the gate;
#10874/#10867 add one between them. collect_module_one is a single ~1890-line
walk under 15 already-peeled siblings, so the next split is phase surgery, not
a move of independent arms -- same rationale as the #1435 allowlist block.
@proggeramlug

Copy link
Copy Markdown
Contributor Author

Landed via merge train 252 (#10892) as v0.5.1632 — merge commit 841b605c97.

Carried with #10866, #10876, #10867, #10874 and #10862, which all touch overlapping surfaces.

Train-side work: #10867 and #10874 each add a module declaration to the same regression-test index (additive — both kept, sorted by issue); #10862's version-bump commits were excluded, since contributor PRs do not bump; and collect_modules.rs was allowlisted because it sat at exactly 2000 lines on main, so the one line these PRs add between them failed the cap.

Validation: 173 gap fixtures across seven areas, zero regressions, re-run against the current base after v0.5.1631 changed canonical_dir's path resolution — relevant because two of these PRs are about module resolution. All ratchets, cargo fmt, and cargo check --workspace --all-targets under -D warnings clean.

Closing here rather than merging — a train lands the commits directly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant