Revert "refactor!: remove modules options (#484)"#504
Merged
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR reverts the changes from commit f64911c by reintroducing the modules option support in Resolver and its related tests and N-API bindings. The changes update tests, adjust configuration options and resolvers, and update the documentation accordingly.
- Reintroduces the modules option to ResolveOptions and adds a helper method for it.
- Updates various test cases to use new module-based resolution paths.
- Updates the N-API bindings and README to reflect the modules option.
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/integration_test.rs | Adds a test using the with_module option |
| src/tests/resolve.rs | Reintroduces a new test (issue238_resolve) using modules |
| src/tests/fallback.rs | Updates fallback tests to use new module paths |
| src/tests/dependencies.rs | Adjusts paths to expect modules-based resolution |
| src/tests/alias.rs | Updates alias tests to work with new module paths |
| src/options.rs | Adds the modules field and with_module helper method |
| src/lib.rs | Modifies module resolution logic to use options.modules |
| src/fs_cache.rs | Adds caching for node_modules lookups |
| src/cache.rs | Updates the CachedPath trait to include module methods |
| napi/src/options.rs | Adds modules option to NAPI resolve options |
| napi/src/lib.rs | Uses the modules option in NAPI resolver initialization |
| README.md | Updates documentation to include the modules option |
Comments suppressed due to low confidence (1)
src/options.rs:337
- [nitpick] Consider renaming 'with_module' to 'with_modules' or 'add_module' for clarity, since the underlying field is a vector of modules.
pub fn with_module<M: Into<String>>(mut self, module: M) -> Self {
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #504 +/- ##
=======================================
Coverage 93.84% 93.84%
=======================================
Files 13 13
Lines 2795 2829 +34
=======================================
+ Hits 2623 2655 +32
- Misses 172 174 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
CodSpeed Performance ReportMerging #504 will not alter performanceComparing Summary
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This reverts commit f64911c.
closes #502
closes #493
I don't have have a clear understanding of how .d.ts resolution works so reverting the previous commit.