feat: map legacy node resolver to the new one with fallback support #272
Conversation
🦋 Changeset detectedLatest commit: e0eb9a2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
""" WalkthroughThis update introduces a new utility for array conversion, refactors legacy resolver handling to prefer a new node resolver with fallback, and modifies error handling for missing legacy node resolvers. Dependency declarations are reorganized, and CI is updated to test with and without the legacy node resolver. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ESLint
participant Plugin
participant NewNodeResolver
participant LegacyNodeResolver
User->>ESLint: Run lint with import resolver
ESLint->>Plugin: Request module resolution
Plugin->>NewNodeResolver: Try resolve (legacy options)
alt NewNodeResolver succeeds
NewNodeResolver->>Plugin: Return resolved path
else Needs legacy package options
Plugin->>LegacyNodeResolver: Dynamically load and resolve
LegacyNodeResolver->>Plugin: Return resolved path or error
end
Plugin->>ESLint: Return resolution result
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
src/utils/resolve.tsOops! Something went wrong! :( ESLint: 9.27.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/lib/index.js' imported from /eslint.config.js test/rules/no-unresolved.spec.tsOops! Something went wrong! :( ESLint: 9.27.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/lib/index.js' imported from /eslint.config.js 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧠 Learnings (1)src/utils/resolve.ts (1)⏰ Context from checks skipped due to timeout of 90000ms (21)
🔇 Additional comments (5)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
There was a problem hiding this comment.
Pull Request Overview
This PR migrates the legacy "import-x/resolve" option to use the new built-in Node resolver, improving consistency and removing unsupported legacy options.
- Updates the resolution logic in src/utils/resolve.ts to utilize the new node resolver for backward compatibility.
- Renames type definitions from NodeResolverOptions to LegacyNodeResolverOptions in both src/utils/legacy-resolver-settings.ts and src/types.ts.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/utils/resolve.ts | Implements new resolution logic using createNodeResolver and updates branch logic for legacy settings. |
| src/utils/legacy-resolver-settings.ts | Updates type imports to use LegacyNodeResolverOptions for legacy resolver configurations. |
| src/types.ts | Renames NodeResolverOptions to LegacyNodeResolverOptions and updates associated documentation. |
Comments suppressed due to low confidence (2)
src/utils/resolve.ts:107
- [nitpick] The variable name 'nodeResolverInstanceForLegacyNodeResolverSettings' is overly verbose. Consider renaming it to 'legacyNodeResolver' or a similar concise identifier.
let nodeResolverInstanceForLegacyNodeResolverSettings: NewResolver | null = null
src/utils/resolve.ts:168
- The new branch for handling legacy 'import-x/resolve' options should be accompanied by test cases to validate its behavior. Consider adding tests that cover both successful and unsuccessful resolution scenarios.
else if ( Object.prototype.hasOwnProperty.call(settings, 'import-x/resolve') && settings['import-x/resolve'] ) {
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
commit: |
SukkaW
force-pushed
the
replace-eslint-import-resolver-node
branch
from
7dab322 to
2a66b0a
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (2)
src/utils/resolve.ts (1)
114-114: Variable initialization concernThis variable is declared but only conditionally initialized. Consider using
let nodeResolverInstanceForLegacyNodeResolverSettings: NewResolver | undefinedto make nullability explicit.-let nodeResolverInstanceForLegacyNodeResolverSettings: NewResolver +let nodeResolverInstanceForLegacyNodeResolverSettings: NewResolver | undefined🧰 Tools
🪛 GitHub Actions: Publish Any Commit
[error] Circular dependency: src/utils/resolve.ts -> src/utils/legacy-resolver-settings.ts -> src/utils/resolve.ts
src/types.ts (1)
45-48: Document the reason for noop propertiesProperties marked as "Noop now" should have a clearer explanation of why they've been deprecated or marked as noop, especially since they were previously functional as indicated by the comments.
- /** Noop now, Previously a directory to begin resolving from */ + /** Noop now. Previously a directory to begin resolving from. These legacy options are maintained for backward compatibility but no longer affect resolution. */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between 7dab322c2fc41bbf101f249acf0549aab1c15e2b and 2a66b0a.
📒 Files selected for processing (3)
src/types.ts(2 hunks)src/utils/legacy-resolver-settings.ts(3 hunks)src/utils/resolve.ts(3 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
src/utils/resolve.ts (2)
src/types.ts (1)
NewResolver(86-91)src/utils/legacy-resolver-settings.ts (2)
normalizeConfigResolvers(103-155)resolveWithLegacyResolver(75-101)
🪛 GitHub Check: Lint and Test with Node.js 20 and ESLint 8.56 on windows-latest
src/types.ts
[failure] 124-124:
Cannot find name 'LegacyNodeResolverOptions'. Did you mean 'NodeResolverOptions'?
src/utils/legacy-resolver-settings.ts
[failure] 10-10:
'"../types.js"' has no exported member named 'LegacyNodeResolverOptions'. Did you mean 'NodeResolverOptions'?
src/utils/resolve.ts
[failure] 7-7:
Relative import paths need explicit file extensions in ECMAScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Did you mean '../node-resolver.js'?
🪛 GitHub Check: Lint and Test with Node.js 22 and ESLint 8 on windows-latest
src/types.ts
[failure] 124-124:
Cannot find name 'LegacyNodeResolverOptions'. Did you mean 'NodeResolverOptions'?
src/utils/legacy-resolver-settings.ts
[failure] 10-10:
'"../types.js"' has no exported member named 'LegacyNodeResolverOptions'. Did you mean 'NodeResolverOptions'?
src/utils/resolve.ts
[failure] 7-7:
Relative import paths need explicit file extensions in ECMAScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Did you mean '../node-resolver.js'?
🪛 GitHub Check: Lint and Test with Node.js 22 and ESLint 9 on windows-latest
src/types.ts
[failure] 124-124:
Cannot find name 'LegacyNodeResolverOptions'. Did you mean 'NodeResolverOptions'?
src/utils/legacy-resolver-settings.ts
[failure] 10-10:
'"../types.js"' has no exported member named 'LegacyNodeResolverOptions'. Did you mean 'NodeResolverOptions'?
src/utils/resolve.ts
[failure] 7-7:
Relative import paths need explicit file extensions in ECMAScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Did you mean '../node-resolver.js'?
🪛 GitHub Check: Lint and Test with Node.js 22 and ESLint 8.56 on windows-latest
src/types.ts
[failure] 124-124:
Cannot find name 'LegacyNodeResolverOptions'. Did you mean 'NodeResolverOptions'?
src/utils/legacy-resolver-settings.ts
[failure] 10-10:
'"../types.js"' has no exported member named 'LegacyNodeResolverOptions'. Did you mean 'NodeResolverOptions'?
src/utils/resolve.ts
[failure] 7-7:
Relative import paths need explicit file extensions in ECMAScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Did you mean '../node-resolver.js'?
🪛 GitHub Check: Lint and Test with Node.js 22 and ESLint 9 on macos-latest
src/types.ts
[failure] 124-124:
Cannot find name 'LegacyNodeResolverOptions'. Did you mean 'NodeResolverOptions'?
src/utils/legacy-resolver-settings.ts
[failure] 10-10:
'"../types.js"' has no exported member named 'LegacyNodeResolverOptions'. Did you mean 'NodeResolverOptions'?
src/utils/resolve.ts
[failure] 7-7:
Relative import paths need explicit file extensions in ECMAScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Did you mean '../node-resolver.js'?
🪛 GitHub Check: Lint and Test with Node.js 18 and ESLint 8 on windows-latest
src/types.ts
[failure] 124-124:
Cannot find name 'LegacyNodeResolverOptions'. Did you mean 'NodeResolverOptions'?
src/utils/legacy-resolver-settings.ts
[failure] 10-10:
'"../types.js"' has no exported member named 'LegacyNodeResolverOptions'. Did you mean 'NodeResolverOptions'?
src/utils/resolve.ts
[failure] 7-7:
Relative import paths need explicit file extensions in ECMAScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Did you mean '../node-resolver.js'?
🪛 GitHub Check: Lint and Test with Node.js 18 and ESLint 9 on windows-latest
src/types.ts
[failure] 124-124:
Cannot find name 'LegacyNodeResolverOptions'. Did you mean 'NodeResolverOptions'?
src/utils/legacy-resolver-settings.ts
[failure] 10-10:
'"../types.js"' has no exported member named 'LegacyNodeResolverOptions'. Did you mean 'NodeResolverOptions'?
src/utils/resolve.ts
[failure] 7-7:
Relative import paths need explicit file extensions in ECMAScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Did you mean '../node-resolver.js'?
🪛 GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on windows-latest
src/types.ts
[failure] 124-124:
Cannot find name 'LegacyNodeResolverOptions'. Did you mean 'NodeResolverOptions'?
src/utils/legacy-resolver-settings.ts
[failure] 10-10:
'"../types.js"' has no exported member named 'LegacyNodeResolverOptions'. Did you mean 'NodeResolverOptions'?
src/utils/resolve.ts
[failure] 7-7:
Relative import paths need explicit file extensions in ECMAScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Did you mean '../node-resolver.js'?
🪛 GitHub Check: Lint and Test with Node.js 20 and ESLint 8.56 on ubuntu-latest
src/types.ts
[failure] 124-124:
Cannot find name 'LegacyNodeResolverOptions'. Did you mean 'NodeResolverOptions'?
src/utils/legacy-resolver-settings.ts
[failure] 10-10:
'"../types.js"' has no exported member named 'LegacyNodeResolverOptions'. Did you mean 'NodeResolverOptions'?
src/utils/resolve.ts
[failure] 7-7:
Relative import paths need explicit file extensions in ECMAScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Did you mean '../node-resolver.js'?
🪛 GitHub Check: Lint and Test with Node.js 18 and ESLint 8.56 on macos-latest
src/types.ts
[failure] 124-124:
Cannot find name 'LegacyNodeResolverOptions'. Did you mean 'NodeResolverOptions'?
src/utils/legacy-resolver-settings.ts
[failure] 10-10:
'"../types.js"' has no exported member named 'LegacyNodeResolverOptions'. Did you mean 'NodeResolverOptions'?
src/utils/resolve.ts
[failure] 7-7:
Relative import paths need explicit file extensions in ECMAScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Did you mean '../node-resolver.js'?
🪛 GitHub Check: Lint and Test with Node.js 18 and ESLint 8.56 on windows-latest
src/types.ts
[failure] 124-124:
Cannot find name 'LegacyNodeResolverOptions'. Did you mean 'NodeResolverOptions'?
src/utils/legacy-resolver-settings.ts
[failure] 10-10:
'"../types.js"' has no exported member named 'LegacyNodeResolverOptions'. Did you mean 'NodeResolverOptions'?
src/utils/resolve.ts
[failure] 7-7:
Relative import paths need explicit file extensions in ECMAScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Did you mean '../node-resolver.js'?
🪛 GitHub Check: Lint and Test with Node.js 22 and ESLint 8 on ubuntu-latest
src/types.ts
[failure] 124-124:
Cannot find name 'LegacyNodeResolverOptions'. Did you mean 'NodeResolverOptions'?
src/utils/legacy-resolver-settings.ts
[failure] 10-10:
'"../types.js"' has no exported member named 'LegacyNodeResolverOptions'. Did you mean 'NodeResolverOptions'?
src/utils/resolve.ts
[failure] 7-7:
Relative import paths need explicit file extensions in ECMAScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Did you mean '../node-resolver.js'?
🪛 GitHub Check: Lint and Test with Node.js 20 and ESLint 8 on windows-latest
src/types.ts
[failure] 124-124:
Cannot find name 'LegacyNodeResolverOptions'. Did you mean 'NodeResolverOptions'?
src/utils/legacy-resolver-settings.ts
[failure] 10-10:
'"../types.js"' has no exported member named 'LegacyNodeResolverOptions'. Did you mean 'NodeResolverOptions'?
src/utils/resolve.ts
[failure] 7-7:
Relative import paths need explicit file extensions in ECMAScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Did you mean '../node-resolver.js'?
🪛 GitHub Check: Lint and Test with Node.js 22 and ESLint 9 on ubuntu-latest
src/types.ts
[failure] 124-124:
Cannot find name 'LegacyNodeResolverOptions'. Did you mean 'NodeResolverOptions'?
src/utils/legacy-resolver-settings.ts
[failure] 10-10:
'"../types.js"' has no exported member named 'LegacyNodeResolverOptions'. Did you mean 'NodeResolverOptions'?
src/utils/resolve.ts
[failure] 7-7:
Relative import paths need explicit file extensions in ECMAScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Did you mean '../node-resolver.js'?
🪛 GitHub Check: Lint and Test with Node.js 18 and ESLint 8 on ubuntu-latest
src/types.ts
[failure] 124-124:
Cannot find name 'LegacyNodeResolverOptions'. Did you mean 'NodeResolverOptions'?
src/utils/legacy-resolver-settings.ts
[failure] 10-10:
'"../types.js"' has no exported member named 'LegacyNodeResolverOptions'. Did you mean 'NodeResolverOptions'?
src/utils/resolve.ts
[failure] 7-7:
Relative import paths need explicit file extensions in ECMAScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Did you mean '../node-resolver.js'?
🪛 GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on ubuntu-latest
src/types.ts
[failure] 124-124:
Cannot find name 'LegacyNodeResolverOptions'. Did you mean 'NodeResolverOptions'?
src/utils/legacy-resolver-settings.ts
[failure] 10-10:
'"../types.js"' has no exported member named 'LegacyNodeResolverOptions'. Did you mean 'NodeResolverOptions'?
src/utils/resolve.ts
[failure] 7-7:
Relative import paths need explicit file extensions in ECMAScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Did you mean '../node-resolver.js'?
🪛 GitHub Check: Lint and Test with Node.js 20 and ESLint 8 on ubuntu-latest
src/types.ts
[failure] 124-124:
Cannot find name 'LegacyNodeResolverOptions'. Did you mean 'NodeResolverOptions'?
src/utils/legacy-resolver-settings.ts
[failure] 10-10:
'"../types.js"' has no exported member named 'LegacyNodeResolverOptions'. Did you mean 'NodeResolverOptions'?
src/utils/resolve.ts
[failure] 7-7:
Relative import paths need explicit file extensions in ECMAScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Did you mean '../node-resolver.js'?
🪛 GitHub Check: Lint and Test with Node.js 18 and ESLint 9 on ubuntu-latest
src/types.ts
[failure] 124-124:
Cannot find name 'LegacyNodeResolverOptions'. Did you mean 'NodeResolverOptions'?
src/utils/legacy-resolver-settings.ts
[failure] 10-10:
'"../types.js"' has no exported member named 'LegacyNodeResolverOptions'. Did you mean 'NodeResolverOptions'?
src/utils/resolve.ts
[failure] 7-7:
Relative import paths need explicit file extensions in ECMAScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Did you mean '../node-resolver.js'?
🪛 GitHub Check: Lint and Test with Node.js 18 and ESLint 8.56 on ubuntu-latest
src/types.ts
[failure] 124-124:
Cannot find name 'LegacyNodeResolverOptions'. Did you mean 'NodeResolverOptions'?
src/utils/legacy-resolver-settings.ts
[failure] 10-10:
'"../types.js"' has no exported member named 'LegacyNodeResolverOptions'. Did you mean 'NodeResolverOptions'?
src/utils/resolve.ts
[failure] 7-7:
Relative import paths need explicit file extensions in ECMAScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Did you mean '../node-resolver.js'?
🪛 GitHub Check: Lint and Test with Node.js 22 and ESLint 8.56 on ubuntu-latest
src/types.ts
[failure] 124-124:
Cannot find name 'LegacyNodeResolverOptions'. Did you mean 'NodeResolverOptions'?
src/utils/legacy-resolver-settings.ts
[failure] 10-10:
'"../types.js"' has no exported member named 'LegacyNodeResolverOptions'. Did you mean 'NodeResolverOptions'?
src/utils/resolve.ts
[failure] 7-7:
Relative import paths need explicit file extensions in ECMAScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Did you mean '../node-resolver.js'?
🪛 GitHub Check: Lint and Test with Node.js 20 and ESLint 8.56 on macos-latest
src/types.ts
[failure] 124-124:
Cannot find name 'LegacyNodeResolverOptions'. Did you mean 'NodeResolverOptions'?
src/utils/legacy-resolver-settings.ts
[failure] 10-10:
'"../types.js"' has no exported member named 'LegacyNodeResolverOptions'. Did you mean 'NodeResolverOptions'?
src/utils/resolve.ts
[failure] 7-7:
Relative import paths need explicit file extensions in ECMAScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Did you mean '../node-resolver.js'?
🪛 GitHub Check: Lint and Test with Node.js 18 and ESLint 9 on macos-latest
src/types.ts
[failure] 124-124:
Cannot find name 'LegacyNodeResolverOptions'. Did you mean 'NodeResolverOptions'?
src/utils/legacy-resolver-settings.ts
[failure] 10-10:
'"../types.js"' has no exported member named 'LegacyNodeResolverOptions'. Did you mean 'NodeResolverOptions'?
src/utils/resolve.ts
[failure] 7-7:
Relative import paths need explicit file extensions in ECMAScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Did you mean '../node-resolver.js'?
🪛 GitHub Check: Lint and Test with Node.js 20 and ESLint 8 on macos-latest
src/types.ts
[failure] 124-124:
Cannot find name 'LegacyNodeResolverOptions'. Did you mean 'NodeResolverOptions'?
src/utils/legacy-resolver-settings.ts
[failure] 10-10:
'"../types.js"' has no exported member named 'LegacyNodeResolverOptions'. Did you mean 'NodeResolverOptions'?
src/utils/resolve.ts
[failure] 7-7:
Relative import paths need explicit file extensions in ECMAScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Did you mean '../node-resolver.js'?
🪛 GitHub Check: Lint and Test with Node.js 22 and ESLint 8 on macos-latest
src/types.ts
[failure] 124-124:
Cannot find name 'LegacyNodeResolverOptions'. Did you mean 'NodeResolverOptions'?
src/utils/legacy-resolver-settings.ts
[failure] 10-10:
'"../types.js"' has no exported member named 'LegacyNodeResolverOptions'. Did you mean 'NodeResolverOptions'?
src/utils/resolve.ts
[failure] 7-7:
Relative import paths need explicit file extensions in ECMAScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Did you mean '../node-resolver.js'?
🪛 GitHub Check: Lint and Test with Node.js 22 and ESLint 8.56 on macos-latest
src/types.ts
[failure] 124-124:
Cannot find name 'LegacyNodeResolverOptions'. Did you mean 'NodeResolverOptions'?
src/utils/legacy-resolver-settings.ts
[failure] 10-10:
'"../types.js"' has no exported member named 'LegacyNodeResolverOptions'. Did you mean 'NodeResolverOptions'?
src/utils/resolve.ts
[failure] 7-7:
Relative import paths need explicit file extensions in ECMAScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Did you mean '../node-resolver.js'?
🪛 GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on macos-latest
src/types.ts
[failure] 124-124:
Cannot find name 'LegacyNodeResolverOptions'. Did you mean 'NodeResolverOptions'?
src/utils/legacy-resolver-settings.ts
[failure] 10-10:
'"../types.js"' has no exported member named 'LegacyNodeResolverOptions'. Did you mean 'NodeResolverOptions'?
src/utils/resolve.ts
[failure] 7-7:
Relative import paths need explicit file extensions in ECMAScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Did you mean '../node-resolver.js'?
🪛 GitHub Check: Lint and Test with Node.js 18 and ESLint 8 on macos-latest
src/types.ts
[failure] 124-124:
Cannot find name 'LegacyNodeResolverOptions'. Did you mean 'NodeResolverOptions'?
src/utils/legacy-resolver-settings.ts
[failure] 10-10:
'"../types.js"' has no exported member named 'LegacyNodeResolverOptions'. Did you mean 'NodeResolverOptions'?
src/utils/resolve.ts
[failure] 7-7:
Relative import paths need explicit file extensions in ECMAScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Did you mean '../node-resolver.js'?
🪛 GitHub Check: publish
src/types.ts
[failure] 124-124:
Cannot find name 'LegacyNodeResolverOptions'. Did you mean 'NodeResolverOptions'?
src/utils/legacy-resolver-settings.ts
[failure] 10-10:
'"../types.js"' has no exported member named 'LegacyNodeResolverOptions'. Did you mean 'NodeResolverOptions'?
src/utils/resolve.ts
[failure] 7-7:
Relative import paths need explicit file extensions in ECMAScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Did you mean '../node-resolver.js'?
🪛 GitHub Actions: Publish Any Commit
src/types.ts
[error] 124-124: error TS2552: Cannot find name 'LegacyNodeResolverOptions'. Did you mean 'NodeResolverOptions'?
src/utils/resolve.ts
[warning] 24-24: "import.meta" is not available in the configured target environment ("es2015") and will be empty [empty-import-meta]
[error] Circular dependency: src/utils/resolve.ts -> src/utils/legacy-resolver-settings.ts -> src/utils/resolve.ts
🪛 GitHub Actions: CI
src/types.ts
[error] 124-124: error TS2552: Cannot find name 'LegacyNodeResolverOptions'. Did you mean 'NodeResolverOptions'?
src/utils/resolve.ts
[warning] 24-24: "import.meta" is not available in the configured target environment ("es2015") and will be empty
🔇 Additional comments (3)
src/utils/resolve.ts (2)
175-181: Consider extracting settings to a variableThe settings['import-x/resolve'] is accessed multiple times. Consider extracting it to a variable for better readability and maintainability.
- } else if ( - // backward compatibility for very old `import-x/resolve` options that is no longer supported - Object.prototype.hasOwnProperty.call(settings, 'import-x/resolve') && - settings['import-x/resolve'] - ) { - const resolveSettings = settings['import-x/resolve'] + } else if ( + // backward compatibility for very old `import-x/resolve` options that is no longer supported + Object.prototype.hasOwnProperty.call(settings, 'import-x/resolve') && + settings['import-x/resolve'] + ) { + const resolveSettings = settings['import-x/resolve']This was already suggested in a previous review and implemented in a later commit.
🧰 Tools
🪛 GitHub Actions: Publish Any Commit
[error] Circular dependency: src/utils/resolve.ts -> src/utils/legacy-resolver-settings.ts -> src/utils/resolve.ts
182-188: Verify semantic equivalence between optionsThere may be a semantic mismatch between
includeCoreModules(legacy) andbuiltinModules(new API).- builtinModules: resolveSettings.includeCoreModules !== false, + builtinModules: resolveSettings.includeCoreModules !== false,This was flagged in a previous review. If
includeCoreModules: falsemeans "exclude core modules" in the legacy API, verify thatbuiltinModules: falsehas the same meaning in the new API.What's the difference between builtinModules and includeCoreModules in Node.js module resolution?🧰 Tools
🪛 GitHub Actions: Publish Any Commit
[error] Circular dependency: src/utils/resolve.ts -> src/utils/legacy-resolver-settings.ts -> src/utils/resolve.ts
src/types.ts (1)
42-42:✅ Verification successful
Verify moduleDirectory type compatibility
The type of
moduleDirectoryhas been changed from what was likely an array to a single string. Make sure this is compatible with how it's used in the resolution logic.
🏁 Script executed:
#!/bin/bash # Check for usages of moduleDirectory in the codebase echo "Checking how moduleDirectory is used:" rg "moduleDirectory" --include "*.ts" .Length of output: 566
Action Required: Verify the
moduleDirectoryType UsageThe
moduleDirectoryfield insrc/types.tshas been changed from an array to a string. The initial search using--includeresulted in an error, so please re-run the query with the corrected flag to check howmoduleDirectoryis used throughout the code. For example, you can use the following script:Once you verify that all usages of
moduleDirectoryexpect a string (and not an array), you can confirm that the resolution logic remains correct. If any usage still treats it as an array, additional adjustments may be required.
🏁 Script executed:
#!/bin/bash echo "Searching for usages of moduleDirectory in .ts files:" rg "moduleDirectory" -g "*.ts" .Length of output: 384
Confirmed: moduleDirectory Type Compatibility is Consistent
Based on the verification:
- In
src/types.ts,moduleDirectoryis defined as an optional string.- In
src/utils/resolve.ts,moduleDirectoryis used by simply passing it to themodulesfield without any array-specific operations.- No evidence was found that any part of the resolution logic still expects
moduleDirectoryto be an array.The change from an array to a string appears compatible with the current usage in the resolution logic.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/types.ts (1)
45-48: Consider adding modules parameter supportBased on the previous review comments, there's a discussion about
basedirandpaths. It appears thatmodulesis the replacement forpathsin the new resolver.Consider adding a comment about how users can migrate from
pathsto the modern equivalent in the new resolver, if applicable. This would help with the migration and backwards compatibility.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/types.ts(2 hunks)src/utils/resolve.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/utils/resolve.ts
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 8 on windows-latest
🔇 Additional comments (4)
src/types.ts (4)
37-64: Interface name change aligns with migration to built-in resolverThe renaming from
NodeResolverOptionstoLegacyNodeResolverOptionsclearly indicates this is for backward compatibility while the codebase migrates to the new built-in node resolver. The addition of properties marked as "Noop now" maintains the interface signature while properly documenting which options are no longer functional.
50-63: Effective use of the never type for no-op propertiesUsing the
nevertype for deprecated properties is a good practice. It ensures TypeScript will raise an error if someone tries to use these properties, while still maintaining the interface shape for backward compatibility.
124-124: Property correctly updated to use new interface nameThe property type has been correctly updated to reference the renamed interface, fixing the build failures noted in the previous review comments.
41-42:❓ Verification inconclusive
Comment and type are inconsistent for moduleDirectory
The comment mentions "directory (or directories)" but the type has been changed from
string[]tostring, which only allows a single directory.- /** directory (or directories) in which to recursively look for modules. Default "node_modules" */ - moduleDirectory?: string + /** directory in which to recursively look for modules. Default "node_modules" */ + moduleDirectory?: stringPlease verify if the built-in resolver supports multiple directories. If it does, consider using
string | string[]to maintain backward compatibility.
🏁 Script executed:
#!/bin/bash # Check if unrs-resolver supports multiple directories for moduleDirectory rg -F "moduleDirectory" --context 10 --type ts node_modules/unrs-resolverLength of output: 73
Action Required: Confirm Multi-Directory Support
The automated search did not reveal any indication that the built-in resolver supports multiple directories for
moduleDirectory. At this point, the change updating the comment to suggest only a single directory (with the type remaining asstring) appears appropriate. However, if the resolver actually supports an array (i.e., multiple directories), the type should be updated tostring | string[]to maintain backward compatibility.
- Location:
src/types.tsat lines 41–42- Next Step: Please manually verify the built-in resolver’s source or documentation to confirm whether it accepts multiple directories for
moduleDirectory. If it does, update the type accordingly; otherwise, the current single-string approach is correct.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/types.ts (3)
47-48: Fix incomplete comment for paths propertyThe comment for the
pathsproperty is incomplete and doesn't fully explain its previous purpose.- /** Noop now. Previously for require.paths array to */ + /** Noop now. Previously for require.paths array to use if nothing is found on the normal node_modules recursive walk */
39-44: Consider adding default values in commentsFor properties like
includeCoreModules,moduleDirectory, andpreserveSymlinks, it would be helpful to include their default values in the comments to provide better guidance for users.- /** set to `false` to exclude node core modules (e.g.` fs`) from the search */ + /** set to `false` to exclude node core modules (e.g.` fs`) from the search. Default: true */ - /** directory (or directories) in which to recursively look for modules. Default "node_modules" */ + /** directory in which to recursively look for modules. Default: "node_modules" */ - /** if true, doesn't resolve basedir to real path before resolving. This is the way Node resolves dependencies when executed with the --preserve-symlinks flag. Default to true */ + /** if true, doesn't resolve basedir to real path before resolving. This is the way Node resolves dependencies when executed with the --preserve-symlinks flag. Default: true */
45-63: Ensure consistency in comment styleThere's a slight inconsistency in the comment style for the noop properties. Some use "Noop now, Previously..." while others use "Noop now. Previously..." (with a period).
Consider standardizing to one style for consistency, preferably "Noop now. Previously..." (with a period) as it's used more frequently in the code.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/types.ts(2 hunks)src/utils/resolve.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/utils/resolve.ts
🔇 Additional comments (2)
src/types.ts (2)
37-64: LGTM: Interface structure is clear with appropriate documentationThe
LegacyNodeResolverOptionsinterface is well-organized with clear documentation of which options are now no-ops versus those that remain functional. This approach properly maintains backward compatibility while moving to the new built-in node resolver.
124-124: Properly updated type reference in ImportSettingsThis properly updates the type reference to use the new
LegacyNodeResolverOptionsinterface.
|
JounQin
force-pushed
the
replace-eslint-import-resolver-node
branch
from
f47e353 to
dd566e1
Compare
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed everything up to dd566e1 in 2 minutes and 18 seconds. Click for details.
- Reviewed
365lines of code in7files - Skipped
1files when reviewing. - Skipped posting
12draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. .github/workflows/ci.yml:35
- Draft comment:
Matrix entry 'noLegacyNodeResolver' is added to test removal of the legacy resolver; configuration looks good. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative, as it only states that a matrix entry was added and that the configuration looks good. It doesn't provide any actionable feedback or suggestions for improvement.
2. .github/workflows/ci.yml:57
- Draft comment:
Conditional step to uninstall the legacy resolver is clear and properly gated by the matrix flag. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative, praising the clarity and gating of a conditional step. It doesn't provide any actionable feedback or suggestions for improvement.
3. package.json:67
- Draft comment:
Marking 'eslint-import-resolver-node' as an optional peer dependency and updating related versions is appropriate; ensure consistency between peer and direct dependency versions. - Reason this comment was not posted:
Comment looked like it was already resolved.
4. src/utils/arraify.ts:1
- Draft comment:
The 'arraify' utility is concise and properly converts its input to an array (or undefined); the implementation is clear. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative and does not provide any actionable feedback or suggestions for improvement. It simply praises the existing code without offering any constructive criticism or questions.
5. src/utils/index.ts:13
- Draft comment:
Exporting 'arraify' via the index file improves convenience in importing utilities. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative, as it simply states that exportingarraifyvia the index file improves convenience. It does not provide any actionable feedback or suggestions for improvement.
6. src/utils/legacy-resolver-settings.ts:70
- Draft comment:
Caching the result of requireResolver into a local 'resolver' variable avoids duplicate calls and is a good performance improvement. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative, as it only states that caching the result ofrequireResolveris a performance improvement. It does not provide any actionable feedback or suggestions for improvement.
7. src/utils/legacy-resolver-settings.ts:119
- Draft comment:
The check for LEGACY_NODE_RESOLVERS to ignore missing resolvers (returning undefined) is intended for fallback; consider leaving a brief comment explaining why undefined is returned. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking for an explanation, which violates the rule against asking the PR author to explain their intention. It doesn't provide a specific suggestion or point out a potential issue with the code.
8. src/utils/resolve.ts:122
- Draft comment:
The new 'legacyNodeResolve' function cleanly implements fallback logic using various resolution strategies; verify that the filtering of normalizedPaths meets the intended behavior. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is asking the PR author to verify the behavior of a function, which is against the rules. It does not provide a specific suggestion or point out a clear issue.
9. src/utils/resolve.ts:238
- Draft comment:
Using '\0' as a separator for the cache key is a clever technique to minimize collisions; ensure that file paths will not contain this character. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking to ensure that file paths do not contain a specific character when used as a separator for a cache key. This is a request for verification, which violates the rule against asking the PR author to ensure behavior is intended or tested.
10. src/utils/resolve.ts:302
- Draft comment:
The fallback branch for legacy node resolvers (checking if name is in LEGACY_NODE_RESOLVERS and resolver is missing) correctly triggers the new node resolver logic; continue the loop if resolution isn’t found. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative and does not provide any actionable feedback or suggestions for improvement. It simply states that something is working correctly, which violates the rule against making purely informative comments.
11. src/utils/index.ts:13
- Draft comment:
Typo: The file name 'arraify.js' on this line might be a misspelling. Consider verifying if it should be 'arrayify.js'. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% While 'arrayify' might be a more common spelling, 'arraify' could be intentional. Without seeing the actual file or understanding the codebase's conventions, I can't be certain this is a typo. The comment is speculative and asks for verification, which violates our rules about not asking authors to verify things. If this really is a typo, it would cause a build error since the file wouldn't exist. Also, the spelling could be intentional - 'arraify' might be the intended name. Since this would cause a build error if incorrect, and we're told to not comment on things that would be caught by the build, this comment is unnecessary. Delete this comment. If the filename is wrong, it would be caught by the build system, and we can't be certain it's actually a typo without more context.
12. src/utils/resolve.ts:301
- Draft comment:
Typo in comment: consider adding an article so it reads 'as a fallback instead' instead of 'as fallback instead'. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50% This is a purely stylistic change to a comment that doesn't affect code functionality or clarity. The meaning is perfectly clear either way. The rules state not to make purely informative comments or unimportant changes. Grammar nitpicks in comments definitely fall into this category. The grammar suggestion is technically correct - "as a fallback" is more proper English than "as fallback". While technically correct, this kind of minor grammar fix in comments provides no real value and just creates noise in the PR review. This comment should be deleted as it's an unimportant stylistic suggestion about comment grammar that doesn't affect code quality or clarity.
Workflow ID: wflow_P9JWNtEu4O4EZqpM
You can customize 
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/ci.yml (2)
32-38: Verify matrix includes for fallback testing
You've extended the matrix to explicitly test both the default and thenoLegacyNodeResolverscenarios onnode: lts/*,eslint: 9, andubuntu-latest. This ensures we cover the new built-in resolver with and without the legacy package.Optional: if you’d like linting to run under the no-legacy scenario as well, consider adding
executeLint: trueto that include entry.
60-63: Optimize placement of the removal step
The “Uninstall Legacy Node Resolver” step currently runs afteryarn --immutable, causing a second dependency resolution when the package is removed. To leverage the cached install and avoid extra work, move this step just before installing dependencies:- - name: Install Dependencies - run: yarn --immutable + - name: Uninstall Legacy Node Resolver + if: ${{ matrix.noLegacyNodeResolver }} + run: yarn remove eslint-import-resolver-node + - name: Install Dependencies + run: yarn --immutableThis will build the environment without the legacy resolver in a single pass.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (7)
.changeset/khaki-sites-teach.md(1 hunks).github/workflows/ci.yml(2 hunks)package.json(2 hunks)src/utils/arraify.ts(1 hunks)src/utils/index.ts(1 hunks)src/utils/legacy-resolver-settings.ts(3 hunks)src/utils/resolve.ts(5 hunks)
✅ Files skipped from review due to trivial changes (3)
- .changeset/khaki-sites-teach.md
- src/utils/index.ts
- src/utils/arraify.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- package.json
- src/utils/legacy-resolver-settings.ts
- src/utils/resolve.ts
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Codacy Static Code Analysis
|
It seems a bug of eslint-plugin-import-x/src/node-resolver.ts Lines 10 to 12 in 3bc48fc Resolved in unrs/unrs-resolver#115 |
|
Last failing test case due to eslint-plugin-import-x/test/rules/no-duplicates.spec.ts Lines 88 to 99 in 3bc48fc |
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferences |
|
OK... Windows... |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
test/rules/no-unresolved.spec.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (21)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Lint and Test with Node.js 24 and ESLint 8 on windows-latest
- GitHub Check: Lint and Test with Node.js lts/* and ESLint 9 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 24 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js lts/* and ESLint 9 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 24 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 8 on windows-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 8 on windows-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 24 and ESLint 8.56 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 8.56 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 8.56 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8.56 on macos-latest
- GitHub Check: Lint and Test with Node.js 24 and ESLint 8.56 on macos-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 8 on macos-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 8 on macos-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 8.56 on macos-latest
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/utils/resolve.ts (1)
8-9: Address the circular dependency issue.The pipeline failures indicate a circular dependency between
resolve.tsandlegacy-resolver-settings.ts. This needs to be resolved to prevent build and runtime issues.Also applies to: 20-20, 23-23
🧹 Nitpick comments (1)
src/utils/resolve.ts (1)
201-204: Improve error message readability.The error message is quite long and could be formatted better for readability. Consider breaking it into multiple lines or providing more structured guidance.
- throw new Error( - "You're using legacy resolver options which are not supported by the new resolver and `eslint-import-resolver-node` is not installed as fallback,\nplease install `eslint-import-resolver-node` manually or remove those legacy resolver options including `package`, `packageFilter`, `pathFilter` and `packageIterator`.", - ) + throw new Error([ + "You're using legacy resolver options which are not supported by the new resolver.", + "Please either:", + "1. Install `eslint-import-resolver-node` as a fallback, or", + "2. Remove legacy options: `package`, `packageFilter`, `pathFilter`, `packageIterator`" + ].join('\n'))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/utils/resolve.ts(5 hunks)
🧰 Additional context used
🧠 Learnings (1)
src/utils/resolve.ts (1)
Learnt from: SukkaW
PR: un-ts/eslint-plugin-import-x#272
File: src/utils/resolve.ts:0-0
Timestamp: 2025-03-30T14:44:11.779Z
Learning: In eslint-plugin-import-x's node resolver, the `modules` parameter of `createNodeResolver` function accepts both string and string array types, making it flexible when passing values like `moduleDirectory`.
⏰ Context from checks skipped due to timeout of 90000ms (20)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Lint and Test with Node.js lts/* and ESLint 9 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8 on windows-latest
- GitHub Check: Lint and Test with Node.js 24 and ESLint 8 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 8 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 8 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 8 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 and ESLint 9 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 24 and ESLint 8.56 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 24 and ESLint 8 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 8.56 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 8 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 8.56 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 24 and ESLint 8.56 on macos-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 8 on macos-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 8.56 on macos-latest
- GitHub Check: Lint and Test with Node.js 20 and ESLint 8 on macos-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 9 on macos-latest
🔇 Additional comments (3)
src/utils/resolve.ts (3)
145-147: Clarify the symlinks logic.The comment says "TODO: change the default behavior to align node itself" but the logic appears inverted. If
preserveSymlinksis false,symlinksis set to true, which seems counterintuitive. Please verify this logic is correct or add a comment explaining the reasoning.
243-248: Good improvement to cache key uniqueness.Using null characters (
'\0') as separators instead of commas eliminates potential collisions in cache keys since null characters cannot appear in file paths or other cache key components.
304-328: Verify the fallback logic handles all cases correctly.The logic appears sound but please verify:
- When
legacyNodeResolvefails butresolverexists, it falls through to use the legacy resolver (lines 325-328)- When
legacyNodeResolvefails and noresolverexists, it continues to the next resolver- This ensures proper fallback behavior while avoiding unnecessary legacy resolver calls
The implementation looks correct for the migration strategy.
JounQin
force-pushed
the
replace-eslint-import-resolver-node
branch
from
e5fff03 to
e0eb9a2
Compare
|
just wanted to say - fantastic work on this SukkaW + all involved! |
Migrate the legacy
import-x/resolveoptions andimport-x/resolver+ node resolver to use our new built-in node resolver.The
import-x/resolveis a legacy option that is not even documented anymore (neither in theeslint-plugin-importnoreslint-plugin-import-x).Summary by CodeRabbit
New Features
Bug Fixes
Chores
Documentation
Important
Migrate legacy node resolver configurations to a new built-in resolver with fallback support, updating utilities, CI, and dependencies.
import-x/resolveandimport-x/resolverto new built-in node resolver with fallback inresolve.ts.legacyNodeResolve()inresolve.tsfor handling legacy resolver options.normalizeConfigResolvers()inlegacy-resolver-settings.tsto support new resolver mapping.arraify()inarraify.tsfor array normalization.arraify()inindex.ts.ci.ymlto includenoLegacyNodeResolvermatrix option.noLegacyNodeResolveris true.eslint-import-resolver-nodeas optional peer dependency inpackage.json.This description was created by
for dd566e1. You can customize this summary. It will automatically update as commits are pushed.