morluto/rea

Fork 0

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

[PR #3789] [MERGED] fix(reference): never resolve a directory to an extensionless index #5901

New issue

Closed

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

bsakaguchi commented

2026-10-31 01:10:18 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/3789
Author: @kaoru0822-kitauji
Created: 10/15/2026
Status: ✅ Merged
Merged: 10/15/2026
Merged by: @kaoru0822-kitauji

Base: main ← Head: fix/3381-reference-file-before-index

📝 Commits (1)

2a06943 fix(reference): never resolve a directory to an extensionless index

📊 Changes

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

View changed files

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

📄 Description

Fixes #3381

Root cause

resolveInternalSpecifier (src/application/ReferenceSourceImportEntries.ts) builds its candidates by interleaving each suffix with its directory form: ["dep", "dep/index", "dep.ts", "dep/index.ts", "dep.js", …]. Because the empty suffix also produced a directory candidate, an extensionless dep/index came second, ahead of dep.js. Neither Node's LOAD_INDEX (index.js, index.json, index.node) nor TypeScript ever selects an extensionless index, so the historical graph recorded an internal dependency on a file that the source never loads.

Change

Directory candidates are generated only for suffixes that carry an extension. Everything else in the candidate order is unchanged.

I first tried moving every file form ahead of every directory form. An existing Node-validated case rules that out: require("./dep") with dep.cjs and dep/index.js resolves to dep/index.js, because CommonJS never tries .cjs. The narrower fix keeps that case correct.

Validation

Repro from the issue (main.cjs requiring ./dep, with dep.js and an extensionless dep/index): Node resolves dep.js. main recorded to: "dep/index"; this PR records to: "dep.js".
Real sources, main vs this PR: relationships are identical and the rest of the graph is byte-identical. REA's own src/ (11,637 relationships, 2,268,023 B output) takes 3.01 s on main and 2.83 s here; node_modules/@babel (207 relationships, 100,658 B) takes 1.16 s and 1.09 s. Output size is unchanged.
Tests: tests/boundary/cli/referenceSourceDirectoryImports.test.ts gains two cases, each checked against Node's createRequire first. ./bare with bare.js and an extensionless bare/index must resolve to bare.js, and ./only/ with only an extensionless only/index must stay unresolved. The test fails on main; I checked by reverting the source line and rebuilding.
All 22 reference-source boundary test files pass (153 tests), as does ReferenceSourceImportEntries.test.ts.

Remaining

#3383 (a directory's package.json main is ignored) shares this function but needs manifest contents. The artifact graph already owns a full CommonJS resolver with main/exports handling in JavaScriptArtifactPathResolution.ts. Moving reference-source import onto that owner, rather than adding a second main parser here, is the structural fix. It also changes the extension and TypeScript-counterpart policy, so I'm leaving it to its own PR.
The interleaved order can still pick, for example, dep/index.ts over dep.js for a CommonJS require in a JavaScript file. Making the order mode-aware belongs to that same resolver unification.

🤖 Generated with Claude Code

_{🔄 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/3789 **Author:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Created:** 10/15/2026 **Status:** ✅ Merged **Merged:** 10/15/2026 **Merged by:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Base:** `main` ← **Head:** `fix/3381-reference-file-before-index` --- ### 📝 Commits (1) - [`2a06943`](https://github.com/morluto/rea/commit/2a069430af482e45aa417949b09206a4303d8ac4) fix(reference): never resolve a directory to an extensionless index ### 📊 Changes **2 files changed** (+11 additions, -1 deletions) <details> <summary>View changed files</summary> 📝 `src/application/ReferenceSourceImportEntries.ts` (+3 -1) 📝 `tests/boundary/cli/referenceSourceDirectoryImports.test.ts` (+8 -0) </details> ### 📄 Description Fixes #3381 ## Root cause `resolveInternalSpecifier` (`src/application/ReferenceSourceImportEntries.ts`) builds its candidates by interleaving each suffix with its directory form: `["dep", "dep/index", "dep.ts", "dep/index.ts", "dep.js", …]`. Because the empty suffix also produced a directory candidate, an extensionless `dep/index` came second, ahead of `dep.js`. Neither Node's `LOAD_INDEX` (`index.js`, `index.json`, `index.node`) nor TypeScript ever selects an extensionless `index`, so the historical graph recorded an internal dependency on a file that the source never loads. ## Change Directory candidates are generated only for suffixes that carry an extension. Everything else in the candidate order is unchanged. I first tried moving every file form ahead of every directory form. An existing Node-validated case rules that out: `require("./dep")` with `dep.cjs` and `dep/index.js` resolves to `dep/index.js`, because CommonJS never tries `.cjs`. The narrower fix keeps that case correct. ## Validation - **Repro from the issue** (`main.cjs` requiring `./dep`, with `dep.js` and an extensionless `dep/index`): Node resolves `dep.js`. `main` recorded `to: "dep/index"`; this PR records `to: "dep.js"`. - **Real sources, `main` vs this PR:** relationships are identical and the rest of the graph is byte-identical. REA's own `src/` (11,637 relationships, 2,268,023 B output) takes 3.01 s on `main` and 2.83 s here; `node_modules/@babel` (207 relationships, 100,658 B) takes 1.16 s and 1.09 s. Output size is unchanged. - **Tests:** `tests/boundary/cli/referenceSourceDirectoryImports.test.ts` gains two cases, each checked against Node's `createRequire` first. `./bare` with `bare.js` and an extensionless `bare/index` must resolve to `bare.js`, and `./only/` with only an extensionless `only/index` must stay unresolved. The test **fails on `main`**; I checked by reverting the source line and rebuilding. - All 22 reference-source boundary test files pass (153 tests), as does `ReferenceSourceImportEntries.test.ts`. ## Remaining - #3383 (a directory's `package.json` `main` is ignored) shares this function but needs manifest contents. The artifact graph already owns a full CommonJS resolver with `main`/`exports` handling in `JavaScriptArtifactPathResolution.ts`. Moving reference-source import onto that owner, rather than adding a second `main` parser here, is the structural fix. It also changes the extension and TypeScript-counterpart policy, so I'm leaving it to its own PR. - The interleaved order can still pick, for example, `dep/index.ts` over `dep.js` for a CommonJS `require` in a JavaScript file. Making the order mode-aware belongs to that same resolver unification. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:10:18 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-01 21:47:13 +00:00

[PR #6420] [CLOSED] fix(analysis): reject malformed procedure resolutions in navigation context #7345

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#5901

No description provided.

Rows
Columns