morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[PR #4221] [CLOSED] fix(reference): select a package main for trailing-slash requires #6131

New issue

Closed

opened 2026-10-31 01:12:06 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:12:06 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4221
Author: @kharedevansh11
Created: 10/15/2026
Status: ❌ Closed

Base: main ← Head: fix/trailing-slash-require-main

📝 Commits (2)

f60eb33 fix(reference): select a package main for trailing-slash requires
c01965e Merge origin/main into fix/trailing-slash-require-main

📊 Changes

2 files changed (+3 additions, -1 deletions)

View changed files

📝 src/application/ReferenceSourceImportEntries.ts (+2 -1)
📝 tests/boundary/cli/referenceSourceDirectoryImports.test.ts (+1 -0)

📄 Description

Summary

Fixes #4189. import-reference-source resolved require("./sub/") to sub/index.js even when sub/package.json declares "main": "actual.js"; Node resolves it to sub/actual.js. Without the trailing slash REA already selected the declared main.

Problem and expected behavior

resolveInternalSpecifier decides whether a match came through a directory index by comparing dirname(match) with the normalized specifier. posix.normalize("sub/") keeps the trailing slash, so "sub" !== "sub/", the index match is treated as a direct file, and selectPackageMain is skipped.

Change and scope

Strip trailing separators from the normalized specifier only for that comparison (same /(?<=.)\/+$/u pattern selectPackageMain already uses). Unresolved to values (e.g. missing/) keep their original spelling. Contribution scope: not applicable (ordinary fix).

Contract and boundary impact

Semantic owner: ReferenceSourceImportEntries.resolveInternalSpecifier.
CLI and MCP contract: unchanged schema; relationship target now matches Node.
Generated metadata / package impact: none.
Read existing files or captured data.

Evidence and regression coverage

Tests updated: added ["package-main-dir.cjs", "./pkg/", "pkg/actual.js"] to tests/boundary/cli/referenceSourceDirectoryImports.test.ts; the test already checks every fixture against Node's createRequire().resolve first. Fails on main (package-main-dir.cjs receives pkg/index.js), passes with the fix.
Base reproduction: the issue's script now prints

case              REA target       Node target
slash-main        sub/actual.js    sub/actual.js
plain-main        sub/actual.js    sub/actual.js
slash-index-only  sub/index.js     sub/index.js

Remaining proof gaps: Windows skipped by the existing test (no-follow opens).

Validation performed

npx vitest run tests/boundary/cli/referenceSourceDirectoryImports.test.ts — passes (fails on base)
npx vitest run over all 26 reference-source test files — 135 passed, 2 skipped
npx tsc --noEmit, npx oxfmt --check — clean

Compatibility, safety, and release

Breaking changes: none.
Security/containment: none.

Author readiness

Human owner: @kharedevansh11

I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
I declare this PR ready for initial maintainer review.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/4221 **Author:** [@kharedevansh11](https://github.com/kharedevansh11) **Created:** 10/15/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/trailing-slash-require-main` --- ### 📝 Commits (2) - [`f60eb33`](https://github.com/morluto/rea/commit/f60eb338e5c95ec6c669c0c049b9b669c3649878) fix(reference): select a package main for trailing-slash requires - [`c01965e`](https://github.com/morluto/rea/commit/c01965ea3b52fe049e92fe29c714267cc1c09ccb) Merge origin/main into fix/trailing-slash-require-main ### 📊 Changes **2 files changed** (+3 additions, -1 deletions) <details> <summary>View changed files</summary> 📝 `src/application/ReferenceSourceImportEntries.ts` (+2 -1) 📝 `tests/boundary/cli/referenceSourceDirectoryImports.test.ts` (+1 -0) </details> ### 📄 Description ## Summary Fixes #4189. `import-reference-source` resolved `require("./sub/")` to `sub/index.js` even when `sub/package.json` declares `"main": "actual.js"`; Node resolves it to `sub/actual.js`. Without the trailing slash REA already selected the declared main. ## Problem and expected behavior `resolveInternalSpecifier` decides whether a match came through a directory index by comparing `dirname(match)` with the normalized specifier. `posix.normalize("sub/")` keeps the trailing slash, so `"sub" !== "sub/"`, the index match is treated as a direct file, and `selectPackageMain` is skipped. ## Change and scope Strip trailing separators from the normalized specifier only for that comparison (same `/(?<=.)\/+$/u` pattern `selectPackageMain` already uses). Unresolved `to` values (e.g. `missing/`) keep their original spelling. Contribution scope: not applicable (ordinary fix). ## Contract and boundary impact - Semantic owner: `ReferenceSourceImportEntries.resolveInternalSpecifier`. - CLI and MCP contract: unchanged schema; relationship target now matches Node. - Generated metadata / package impact: none. - [x] Read existing files or captured data. ## Evidence and regression coverage - Tests updated: added `["package-main-dir.cjs", "./pkg/", "pkg/actual.js"]` to `tests/boundary/cli/referenceSourceDirectoryImports.test.ts`; the test already checks every fixture against Node's `createRequire().resolve` first. Fails on `main` (`package-main-dir.cjs` receives `pkg/index.js`), passes with the fix. - Base reproduction: the issue's script now prints ```text case REA target Node target slash-main sub/actual.js sub/actual.js plain-main sub/actual.js sub/actual.js slash-index-only sub/index.js sub/index.js ``` - Remaining proof gaps: Windows skipped by the existing test (no-follow opens). ## Validation performed - `npx vitest run tests/boundary/cli/referenceSourceDirectoryImports.test.ts` — passes (fails on base) - `npx vitest run` over all 26 reference-source test files — 135 passed, 2 skipped - `npx tsc --noEmit`, `npx oxfmt --check` — clean ## Compatibility, safety, and release - Breaking changes: none. - Security/containment: none. ## Author readiness Human owner: @kharedevansh11 - [ ] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [ ] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [ ] I declare this PR ready for initial maintainer review. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:12:06 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-01 21:45:48 +00:00

[PR #6144] [MERGED] fix(cancellation): preserve caller abort reason in cancellation errors #7159

bsakaguchi changed title from ~~[PR #4221] [CLOSED] fix(reference): select a package main for trailing-slash requires~~ to [PR #4221] fix(reference): select a package main for trailing-slash requires

2026-11-03 13:31:56 +00:00

bsakaguchi changed title from ~~[PR #4221] fix(reference): select a package main for trailing-slash requires~~ to [PR #4221] [CLOSED] fix(reference): select a package main for trailing-slash requires

2026-11-03 14:33:40 +00:00

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

morluto/rea#6131

No description provided.

Rows
Columns