Skip to content

fix: handle symlinked uniwind origins in metro resolver - #609

Merged
Brentlok merged 1 commit into
uni-stack:mainfrom
joedeleeuw:fix/pnpm-internal-origin-realpath
Aug 19, 2026
Merged

Brentlok merged 1 commit into
uni-stack:mainfrom
joedeleeuw:fix/pnpm-internal-origin-realpath

Conversation

@joedeleeuw

@joedeleeuw joedeleeuw commented Jul 19, 2026 •

Copy link
Copy Markdown
Contributor

#598

keeps uniwind's own metro imports internal when pnpm reports the symlink path. one real-symlink regression; native/web suites, build, types, lint, and format pass.

Summary by CodeRabbit

  • Bug Fixes
    • Improved Metro module resolution for symlinked installs by more reliably detecting UniWIND’s internal origins.
    • Ensured native and web bundling preserves the intended project node_modules boundaries instead of resolving to dereferenced internal paths.
  • Tests
    • Added Jest coverage to verify native bundler resolution behavior when the package is symlinked into node_modules.

@coderabbitai

coderabbitai Bot commented Jul 19, 2026 •

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Metro internal-origin checks now use shared realpath-aware logic for symlinked and canonical paths. Native and web resolvers use the helper, while a native resolver test validates symlinked Uniwind origins and temporary-path behavior.

Changes

Metro origin detection

Layer / File(s) Summary
Canonical origin detection and resolver integration
packages/uniwind/src/bundler/adapters/metro/resolvers.ts
Adds cached, separator-aware realpath validation and applies the shared internal-origin helper in native and web resolver branches.
Symlinked origin validation
packages/uniwind/tests/native/bundler/resolvers.test.ts
Adds coverage for symlinked Uniwind origins, resolver calls, returned temporary-rooted paths, and cleanup.

Estimated code review effort: 3 (Moderate) | ~15–30 minutes

Possibly related PRs

Suggested reviewers: jpudysz, brentlok

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The resolver now canonicalizes internal-origin checks, preserves internal react-native imports, and adds a symlink regression test, matching #598.
Out of Scope Changes check ✅ Passed The changes stay focused on Metro resolver symlink handling and a corresponding regression test, with no unrelated scope added.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: handling symlinked Uniwind origins in the Metro resolver.
✨ 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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 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.

Inline comments:
In `@packages/uniwind/src/bundler/adapters/metro/resolvers.ts`:
- Around line 14-37: Update isInternalOrigin to memoize each originModulePath
result in a Map, returning the cached value before performing filesystem work
and storing all computed outcomes. Use sep-aware directory-boundary matching for
both the direct path and realpath checks, so only the internal base directory or
its descendants match; preserve false on unresolved paths.

In `@packages/uniwind/tests/native/bundler/resolvers.test.ts`:
- Line 11: Update the internalRoot initialization in the resolver test to wrap
require.resolve('uniwind/package.json') with realpathSync, matching the path
normalization used by resolvers.ts and ensuring the strict equality assertion
compares fully resolved paths.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c60927b2-3301-45e1-84a2-161d9cee3870

📥 Commits

Reviewing files that changed from the base of the PR and between 3f971f1 and c031f1b.

📒 Files selected for processing (2)
  • packages/uniwind/src/bundler/adapters/metro/resolvers.ts
  • packages/uniwind/tests/native/bundler/resolvers.test.ts

Comment thread packages/uniwind/src/bundler/adapters/metro/resolvers.ts
Comment thread packages/uniwind/tests/native/bundler/resolvers.test.ts Outdated
@greptile-apps

greptile-apps Bot commented Jul 19, 2026 •

Copy link
Copy Markdown

Greptile Summary

This PR fixes a Metro resolution bug where pnpm-managed symlinks caused isInternalOrigin to miss internal imports — previously originModulePath could be the symlink path while the cached base was the real path, so the startsWith check failed and internal imports were incorrectly treated as external.

  • resolvers.ts: Extracts a new isInternalOrigin helper that caches realpathSync of the package root, applies a fast startsWith check, then—only for paths containing node_modules/uniwind—calls realpathSync(originModulePath) to resolve any symlink before comparing.
  • resolvers.test.ts: Adds a real-filesystem test (temp dir + symlinkSync) that verifies the symlinked-origin path is recognised as internal, preventing the unintended react-native → uniwind/components redirection.

Confidence Score: 5/5

Safe to merge — the change is narrowly scoped to symlink detection in the Metro resolver, the core logic is correct, error paths are wrapped in try/catch, and a real-filesystem test validates the regression case.

The fix correctly resolves the symlink before comparing against the cached real path, the guard (includes node_modules/uniwind) keeps the realpathSync fallback on the narrow path where it is needed, and the new test directly exercises the symlinked-origin scenario that previously broke internal-boundary detection.

No files require special attention.

Important Files Changed

Filename Overview
packages/uniwind/src/bundler/adapters/metro/resolvers.ts Extracts symlink-aware isInternalOrigin helper: caches the real path of the package root via realpathSync, fast-paths the common case, and only calls realpathSync(originModulePath) for paths containing the node_modules/uniwind segment. Logic is sound and error paths are handled.
packages/uniwind/tests/native/bundler/resolvers.test.ts New test creates a real temp-dir symlink and verifies that nativeResolver returns early (does not redirect react-native → uniwind/components) when Metro supplies a symlinked origin path. Covers the regression case correctly.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["isInternalOrigin(originModulePath)"] --> B{cachedInternalBasePath\nis null?}
    B -- Yes --> C["realpathSync(require.resolve('uniwind/package.json'))\n→ cachedInternalBasePath"]
    C --> D{cachedInternalBasePath\nis empty?}
    B -- No --> D
    D -- Yes --> E[return false]
    D -- No --> F["internalPathPrefix = cachedInternalBasePath + sep"]
    F --> G{originModulePath\nstartsWith internalPathPrefix?}
    G -- Yes --> H[return true ✓ fast path]
    G -- No --> I{originModulePath contains\nnode_modules/uniwind?}
    I -- No --> J[return false]
    I -- Yes --> K["realpathSync(originModulePath)"]
    K --> L{realpath\nstartsWith internalPathPrefix?}
    L -- Yes --> M[return true ✓ symlink path]
    L -- No --> N[return false]
    K -- throws --> O[catch → return false]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A["isInternalOrigin(originModulePath)"] --> B{cachedInternalBasePath\nis null?}
    B -- Yes --> C["realpathSync(require.resolve('uniwind/package.json'))\n→ cachedInternalBasePath"]
    C --> D{cachedInternalBasePath\nis empty?}
    B -- No --> D
    D -- Yes --> E[return false]
    D -- No --> F["internalPathPrefix = cachedInternalBasePath + sep"]
    F --> G{originModulePath\nstartsWith internalPathPrefix?}
    G -- Yes --> H[return true ✓ fast path]
    G -- No --> I{originModulePath contains\nnode_modules/uniwind?}
    I -- No --> J[return false]
    I -- Yes --> K["realpathSync(originModulePath)"]
    K --> L{realpath\nstartsWith internalPathPrefix?}
    L -- Yes --> M[return true ✓ symlink path]
    L -- No --> N[return false]
    K -- throws --> O[catch → return false]
Loading

Reviews (4): Last reviewed commit: "fix: handle symlinked uniwind origins in..." | Re-trigger Greptile

Comment thread packages/uniwind/src/bundler/adapters/metro/resolvers.ts
Comment thread packages/uniwind/tests/native/bundler/resolvers.test.ts
@joedeleeuw
joedeleeuw force-pushed the fix/pnpm-internal-origin-realpath branch 3 times, most recently from 0f734c9 to 6e38f85 Compare July 19, 2026 17:41
@joedeleeuw
joedeleeuw force-pushed the fix/pnpm-internal-origin-realpath branch from 6e38f85 to 6804b60 Compare July 19, 2026 17:54

@Brentlok Brentlok 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.

Thanks for contribution! 🚀

@Brentlok
Brentlok merged commit 20b1d37 into uni-stack:main Aug 19, 2026
2 checks passed
@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

🚀 This pull request is included in v1.12.0. See Release v1.12.0 for release notes.

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.

2 participants