fix: strip trailing slash from resolved node builtins

[ashtonjurgens]

resolves #555

This PR

• Strips the trailing slash
• Includes a test

Summary by CodeRabbit

• Bug Fixes
  • Improved module path resolution to properly normalize trailing slashes in module identifiers, ensuring consistent canonicalization across different reference formats.

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR fixes module ID resolution to strip trailing slashes from node: builtin specifiers. The _resolve() function now detects node: prefixes, extracts the builtin name, normalizes trailing slashes, and returns a canonical node:<builtin> form only when the builtin exists. A test case validates the normalization behavior.

Changes

Builtin Module Trailing Slash Normalization

Layer / File(s)	Summary
Builtin normalization and trailing slash handling src/env.ts, test/env.test.ts	The _resolve() function adds explicit handling for node:-prefixed IDs by extracting the builtin name, trimming trailing slashes, and returning a canonical node:<builtin> form when the builtin exists in builtinModules. A test case verifies that override mappings like foo -> punycode/ resolve to node:punycode.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

A trailing slash caused Node's dismay,
But our rabbit sorted the fray,
Stripping the / with care and grace,
Now builtins work in their rightful place,
node:punycode shines bright today! 🐰✨

🚥 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%.	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 accurately and concisely describes the main fix: stripping trailing slashes from node: prefixed builtin module specifiers, which directly addresses the core problem.
Linked Issues check	✅ Passed	The changes directly implement the requirements from issue #555: normalizing node: builtins by stripping trailing slashes while preserving non-builtin imports unchanged.
Out of Scope Changes check	✅ Passed	All changes in src/env.ts and test/env.test.ts are directly related to addressing the trailing slash normalization issue in node: builtins with no unrelated modifications.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

🧹 Nitpick comments (1)

test/env.test.ts (1)

57-70: ⚡ Quick win

Consider adding a test case for node:-prefixed IDs with trailing slashes.

The test validates the core scenario (punycode/ → node:punycode), but it would be valuable to also test the case where an override maps to a node:-prefixed ID with a trailing slash (e.g., bar: "node:punycode/"). This would explicitly exercise lines 122-128 in src/env.ts and provide more comprehensive coverage of the fix.

🧪 Suggested additional test case

     it("strips trailing slash when resolving builtin aliases", () => {
       const { env } = defineEnv({
         nodeCompat: false,
         resolve: true,
         overrides: {
           alias: {
             punycode: "node:punycode",
             foo: "punycode/",
+            bar: "node:punycode/",
           },
         },
       });
 
       expect(env.alias.foo).toBe("node:punycode");
+      expect(env.alias.bar).toBe("node:punycode");
     });

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/env.test.ts` around lines 57 - 70, Add a test to cover node:-prefixed
IDs that include a trailing slash: extend the existing test using defineEnv to
include an overrides.alias mapping such as bar: "node:punycode/" and assert that
env.alias.bar resolves to "node:punycode"; this will exercise the normalization
logic around node: IDs (the code paths exercised by the current test for foo and
the logic in the env normalization functions) and ensure trailing slashes are
stripped for node: prefixed targets as well.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@test/env.test.ts`:
- Around line 57-70: Add a test to cover node:-prefixed IDs that include a
trailing slash: extend the existing test using defineEnv to include an
overrides.alias mapping such as bar: "node:punycode/" and assert that
env.alias.bar resolves to "node:punycode"; this will exercise the normalization
logic around node: IDs (the code paths exercised by the current test for foo and
the logic in the env normalization functions) and ensure trailing slashes are
stripped for node: prefixed targets as well.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ea52618c-90e9-4064-bd33-79302c2b3760

📥 Commits

Reviewing files that changed from the base of the PR and between f89b7cc and f8a46bb.

📒 Files selected for processing (2)

• src/env.ts
• test/env.test.ts