fix: import resolve find in paths's node_modules #35
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe pull request introduces an enhancement to the Changes
Sequence DiagramsequenceDiagram
participant Resolver as Module Resolver
participant NodeModules as node_modules Directory
participant DebugLogger as Debug Logger
Resolver->>NodeModules: Attempt to resolve module path
alt Module Found
NodeModules-->>Resolver: Return module file path
Resolver->>DebugLogger: Log resolution details
else Module Not Found
Resolver->>Resolver: Continue with existing resolution logic
end
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/import.ts (1)
205-215: Well-structured extension for node_modules resolution
The loop correctly attempts to resolve the requested module fromnode_modulespaths. It short-circuits once a valid file is found, meeting the PR objective of enhanced import resolution.You might also consider gracefully handling cases where
node_modulesmay not exist or be inaccessible, e.g., checking its presence before attempting resolution, to optimize performance and error handling further.test/import.test.ts (1)
30-34: Test coverage for CommonJS module resolution
This test case effectively verifies loading a CommonJS package from{paths}/node_modules.A negative test scenario (e.g., module not found in
node_modules) could further strengthen test coverage.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
test/fixtures/cjs/node_modules/inject/index.jsis excluded by!**/node_modules/**test/fixtures/cjs/node_modules/inject/package.jsonis excluded by!**/node_modules/**test/fixtures/esm/node_modules/inject/index.jsis excluded by!**/node_modules/**test/fixtures/esm/node_modules/inject/package.jsonis excluded by!**/node_modules/**
📒 Files selected for processing (2)
src/import.ts(1 hunks)test/import.test.ts(2 hunks)
🔇 Additional comments (1)
test/import.test.ts (1)
58-62: Test coverage for ESM module resolution
This test ensures that an ESM package can also be successfully resolved from {paths}/node_modules, paralleling the CommonJS flow.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #35 +/- ##
==========================================
- Coverage 89.78% 87.50% -2.29%
==========================================
Files 6 6
Lines 685 696 +11
Branches 120 120
==========================================
- Hits 615 609 -6
- Misses 70 87 +17 ☔ View full report in Codecov by Sentry. |
[skip ci] ## [4.2.2](v4.2.1...v4.2.2) (2024-12-30) ### Bug Fixes * import resolve find in paths's node_modules ([#35](#35)) ([bd772af](bd772af))
Summary by CodeRabbit
New Features
node_modulesdirectoryTests