Skip to content

Fix _getEntryPointAbsolutePath conflating disk paths with URL pathnames - #2030

Merged
robhogan merged 1 commit into
mainfrom
pr2030
Oct 6, 2026
Merged

robhogan merged 1 commit into
mainfrom
pr2030

Conversation

@robhogan

@robhogan robhogan commented Oct 6, 2026 •

Copy link
Copy Markdown
Collaborator

Recreating #1708 / D104415558 (@motiz88) for GH first without a diff attachment

Closes: #1708

D102004228 added [metro-project]/[metro-watchFolders] virtual URL pathname resolution to _getEntryPointAbsolutePath, but this method doesn't take URL pathnames - it takes actual filesystem paths (ultimately from Metro's API / CLI).

The fix removes virtual prefix resolution from _getEntryPointAbsolutePath, making it a plain path.resolve(serverRootDir, entryFile) call, consistent with buildGraph which already treats the entry file as a plain disk path.

Changelog:

  • [Fix]: Remove erroneous [metro-project] and [metro-watchFolders] resolution from API and CLI

Test plan:
See D104415558

*Recreating #1708 / [D104415558](https://www.internalfb.com/diff/D104415558) (@motiz88) for GH first without a diff attachment*

D102004228 added `[metro-project]`/`[metro-watchFolders]` virtual URL pathname resolution to `_getEntryPointAbsolutePath`, but this method doesn't take URL pathnames - it takes actual filesystem paths (ultimately from Metro's API / CLI).

The fix removes virtual prefix resolution from `_getEntryPointAbsolutePath`, making it a plain `path.resolve(serverRootDir, entryFile)` call, consistent with `buildGraph` which already treats the entry file as a plain disk path.

Changelog:

* **[Fix]:** Remove erroneous `[metro-project]` and `[metro-watchFolders]` resolution from API and CLI

Test plan:
See [D104415558](https://www.internalfb.com/diff/D104415558)
@robhogan
robhogan requested review from huntie, motiz88 and vzaidman and a balanced review from Copilot October 6, 2026 14:36
@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 6, 2026
@robhogan robhogan added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 6, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

馃煝 Approval recommended

The focused implementation matches existing graph resolution behavior and is covered by an appropriate integration test.

Review effort: Balanced
Findings: None

What changed in this PR

Removes URL-prefix interpretation from API/CLI entry paths so literal filesystem directories resolve correctly.

Changes:

  • Resolves entry files directly against Metro鈥檚 server root.
  • Enables the regression test for a literal [metro-project] directory.
File Description
packages/鈥媘etro/鈥媠rc/鈥婼erver.js Treats entry points strictly as filesystem paths.
packages/鈥媘etro/鈥媠rc/鈥媔ntegration_tests/鈥媉_tests__/鈥媌uild-test.js Activates regression coverage for literal prefix-like directories.

馃挕 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@robhogan
robhogan merged commit ea2f8b7 into main Oct 6, 2026
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants