Skip to content

fix(extensions): fall back to ESM import when createRequire is intercepted - #866

Open
MartinGuerraGit wants to merge 1 commit into
Gentleman-Programming:mainfrom
MartinGuerraGit:fix/pi-pretty-esm-import
Open

MartinGuerraGit wants to merge 1 commit into
Gentleman-Programming:mainfrom
MartinGuerraGit:fix/pi-pretty-esm-import

Conversation

@MartinGuerraGit

@MartinGuerraGit MartinGuerraGit commented Sep 10, 2026

Copy link
Copy Markdown

Closes #238

PR Type

  • Bug fix
  • New feature
  • Documentation only
  • Code refactoring
  • Maintenance/tooling
  • Breaking change

Summary

  • extensions/pi-pretty.ts no longer hard-fails when the Pi compiled binary intercepts createRequire: if the package-name require throws, it falls back to an ESM import() of the same installed dependency.
  • Keeps the existing real-path createRequire behavior as the primary path (pnpm symlink installs keep working as before).
  • No dependency, manifest, or packaging changes.

Changes

File Change
extensions/pi-pretty.ts Lazy-load the pi-pretty module with try/require primary + ESM-import fallback; unwrap function-or-default interop in one helper

Test Plan

  • No shell scripts modified (shellcheck N/A)
  • Manually tested the affected functionality: pi -p "hi" from a repo that consumes gentle-pi as a linked package now starts with all extensions on Pi 0.85.1 (previously aborted with Failed to load extension ... pi-pretty.ts ... ResolveMessage: Cannot find module '@heyhuynhgiabuu/pi-pretty'); plain Node import of the wrapper also verified
  • Repo tests for the touched area pass: tests/package-manifest.test.ts + tests/quiet-tool-rendering.test.ts (95 pass, 0 fail)
  • Full pnpm test left to CI

Contributor Checklist

Summary by CodeRabbit

  • Bug Fixes
    • Improved extension loading compatibility across different runtime environments.
    • Added fallback handling for module loading when the primary loading method is unavailable.
    • Added validation to provide an error when the loaded extension does not expose the expected interface.

@coderabbitai

coderabbitai Bot commented Sep 10, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: b9d138fc-bb4f-4c1e-9726-db63c1723013

📥 Commits

Reviewing files that changed from the base of the PR and between 1c62427 and dc786ed.

📒 Files selected for processing (1)
  • extensions/pi-pretty.ts

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


📝 Walkthrough

Walkthrough

The pi-pretty extension now loads asynchronously. It first uses requireFromRealPackage, falls back to ESM import on failure, validates the exported function, and awaits the loader from the default export.

Changes

pi-pretty loading

Layer / File(s) Summary
Module loader and extension integration
extensions/pi-pretty.ts
The extension adds helpers for module loading and export validation. It falls back from requireFromRealPackage to ESM import and awaits the validated function from the default export.

Estimated code review effort: 2 (Simple) | ~10 minutes

Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant ExtensionEntry
  participant loadPiPrettyExtension
  participant requireFromRealPackage
  participant ESMImport
  ExtensionEntry->>loadPiPrettyExtension: await load
  loadPiPrettyExtension->>requireFromRealPackage: require pi-pretty
  requireFromRealPackage-->>loadPiPrettyExtension: module or error
  alt require fails
    loadPiPrettyExtension->>ESMImport: import pi-pretty
    ESMImport-->>loadPiPrettyExtension: module
  end
  loadPiPrettyExtension-->>ExtensionEntry: validated extension function
Loading

Suggested reviewers: alan-thegentleman

Merge Risk: ⚪ Minimal · up to dc786

The extension now falls back to ESM loading when package-name resolution fails while preserving existing loading and export compatibility. Tests pass, with no merge-blocking risk evidenced.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ 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%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 1 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the primary change: using an ESM import fallback when createRequire is intercepted.
Linked Issues check ✅ Passed The changes address issue #238 by retaining real-path createRequire as the primary loader and adding an ESM import fallback when package-name resolution fails. The module interop validation also suppo…
Out of Scope Changes check ✅ Passed The changes are limited to the pi-pretty extension loader and module interop. They match issue #238 and introduce no unrelated dependency, manifest, or packaging changes.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

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.

pi-pretty never loads in compiled Pi binary (createRequire intercepted by jiti)

1 participant