chore(deps): migrate unrs-resolver to new oxc-resolver #422
+1,497
−1,460
Conversation
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
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughSwapped resolver imports from Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor L as Linter/Rule
participant R as createNodeResolver
participant O as oxc-resolver
rect rgb(211,228,205)
note right of O: Resolver backend swapped to oxc-resolver
end
L->>R: resolve(request, importer)
alt builtin or data URI
R-->>L: short-circuit (not resolved)
else non-builtin
R->>O: sync(request, importer, options)
O-->>R: resolved path or error
R-->>L: result (found/path) or found:false
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
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 |
|
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. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #422 +/- ##
=======================================
Coverage 96.10% 96.10%
=======================================
Files 96 96
Lines 4952 4952
Branches 1837 1860 +23
=======================================
Hits 4759 4759
Misses 192 192
Partials 1 1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![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 f75c0e4 in 44 seconds. Click for details.
- Reviewed
162lines of code in3files - Skipped
2files when reviewing. - Skipped posting
3draft 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. package.json:77
- Draft comment:
Migrated dependency: removal of 'unrs-resolver' and addition of 'oxc-resolver'. Ensure the new package’s API is fully compatible. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is about a dependency change, specifically the removal of one package and the addition of another. The comment asks to ensure API compatibility, which is not allowed according to the rules. The rules state not to comment on dependency changes or ask the author to ensure compatibility.
2. src/node-resolver.ts:4
- Draft comment:
Verify that 'oxc-resolver' exposes a compatible ResolverFactory and that 'resolver.sync' behaves as expected. - 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 'oxc-resolver' and 'resolver.sync', which is against the rules as it asks for confirmation of intended behavior.
3. src/utils/declared-scope.ts:10
- Draft comment:
Explicit return type 'ScopeType | undefined' improves clarity. Confirm this matches all expected outcomes. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =50%<= threshold50%The comment suggests an improvement by making the return type explicit, which is a valid suggestion. However, it also asks the author to confirm that this matches all expected outcomes, which violates the rule against asking for confirmation. The suggestion itself is useful, but the request for confirmation is not allowed.
Workflow ID: wflow_UYR14YjSC75jyLRb
You can customize 
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/node-resolver.ts (2)
28-31: Optionally handle file:// specifiersNode supports file URLs; short-circuiting them avoids handing off to the resolver and improves correctness for such imports.
Apply:
import { isBuiltin } from 'node:module' import path from 'node:path' +import { fileURLToPath } from 'node:url' ... if (isBuiltin(modulePath) || modulePath.startsWith('data:')) { return { found: true, path: null } } + + if (modulePath.startsWith('file://')) { + try { + return { found: true, path: fileURLToPath(modulePath) } + } catch { + // fall through to resolver + } + }
32-40: Swallowed resolve errors — consider debug loggingSilent catch is fine for normal operation, but adding a guarded debug log can help diagnose resolution edge cases without impacting users.
- } catch { - // - } + } catch (e) { + // eslint-disable-next-line no-empty + if (process.env.DEBUG?.includes('eslint-plugin-import-x')) { + // Lazy require to avoid adding hard dep on debug + // eslint-disable-next-line @typescript-eslint/no-var-requires + const dbg = require('node:console'); // or integrate 'debug' if already in deps + dbg.error?.('[import-x:node] resolve error:', e, { modulePath, sourceFile }) + } + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (4)
package.json(4 hunks)patches/@swc-node+jest+1.9.1.patch(1 hunks)src/node-resolver.ts(1 hunks)src/utils/declared-scope.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-03-30T14:44:11.779Z
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`.
Applied to files:
src/node-resolver.ts
📚 Learning: 2025-03-30T09:06:59.006Z
Learnt from: JounQin
PR: un-ts/eslint-plugin-import-x#271
File: test/rules/no-unused-modules.spec.ts:1528-1532
Timestamp: 2025-03-30T09:06:59.006Z
Learning: The import from 'eslint8.56/use-at-your-own-risk' has incorrect TypeScript types but works correctly at runtime, which is properly handled with a `ts-expect-error` comment.
Applied to files:
package.json
📚 Learning: 2025-06-18T12:49:41.795Z
Learnt from: JounQin
PR: un-ts/eslint-plugin-import-x#385
File: src/rules/prefer-namespace-import.ts:0-0
Timestamp: 2025-06-18T12:49:41.795Z
Learning: The eslint-plugin-import-x project requires Node versions `^18.18.0 || ^20.9.0 || >=21.1.0` as specified in package.json, so modern JavaScript features like String.prototype.at() are safe to use.
Applied to files:
package.json
🧬 Code Graph Analysis (2)
src/utils/declared-scope.ts (1)
src/types.ts (1)
RuleContext(69-78)
package.json (1)
test/core/export-map.spec.ts (2)
fakeContext(13-486)espreeContext(292-334)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (190)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 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 22 and ESLint 8 on ubuntu-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.56 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 24.5 and ESLint 8 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 9 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 22 and ESLint 9 on macos-latest
- GitHub Check: Lint and Test with Node.js 24.5 and ESLint 9 on macos-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 8 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 24.5 and ESLint 8 on windows-latest
- GitHub Check: Lint and Test with Node.js 24.5 and ESLint 9 on ubuntu-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 22 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 24.5 and ESLint 9 on macos-latest
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 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 22 and ESLint 8 on ubuntu-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.56 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 24.5 and ESLint 8 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 9 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 22 and ESLint 9 on macos-latest
- GitHub Check: Lint and Test with Node.js 24.5 and ESLint 9 on macos-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 8 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 24.5 and ESLint 8 on windows-latest
- GitHub Check: Lint and Test with Node.js 24.5 and ESLint 9 on ubuntu-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 22 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 24.5 and ESLint 9 on macos-latest
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 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 22 and ESLint 8 on ubuntu-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.56 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 24.5 and ESLint 8 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 9 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 22 and ESLint 9 on macos-latest
- GitHub Check: Lint and Test with Node.js 24.5 and ESLint 9 on macos-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 8 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 24.5 and ESLint 8 on windows-latest
- GitHub Check: Lint and Test with Node.js 24.5 and ESLint 9 on ubuntu-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 22 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 24.5 and ESLint 9 on macos-latest
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 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 22 and ESLint 8 on ubuntu-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.56 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 24.5 and ESLint 8 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 9 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 22 and ESLint 9 on macos-latest
- GitHub Check: Lint and Test with Node.js 24.5 and ESLint 9 on macos-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 8 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 24.5 and ESLint 8 on windows-latest
- GitHub Check: Lint and Test with Node.js 24.5 and ESLint 9 on ubuntu-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 22 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 24.5 and ESLint 9 on macos-latest
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 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 22 and ESLint 8 on ubuntu-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.56 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 24.5 and ESLint 8 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 9 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 22 and ESLint 9 on macos-latest
- GitHub Check: Lint and Test with Node.js 24.5 and ESLint 9 on macos-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 8 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 24.5 and ESLint 8 on windows-latest
- GitHub Check: Lint and Test with Node.js 24.5 and ESLint 9 on ubuntu-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 22 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 24.5 and ESLint 9 on macos-latest
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 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 22 and ESLint 8 on ubuntu-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.56 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 24.5 and ESLint 8 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 9 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 22 and ESLint 9 on macos-latest
- GitHub Check: Lint and Test with Node.js 24.5 and ESLint 9 on macos-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 8 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 24.5 and ESLint 8 on windows-latest
- GitHub Check: Lint and Test with Node.js 24.5 and ESLint 9 on ubuntu-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 22 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 24.5 and ESLint 9 on macos-latest
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 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 22 and ESLint 8 on ubuntu-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.56 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 24.5 and ESLint 8 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 9 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 22 and ESLint 9 on macos-latest
- GitHub Check: Lint and Test with Node.js 24.5 and ESLint 9 on macos-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 8 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 24.5 and ESLint 8 on windows-latest
- GitHub Check: Lint and Test with Node.js 24.5 and ESLint 9 on ubuntu-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 22 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 24.5 and ESLint 9 on macos-latest
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 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 22 and ESLint 8 on ubuntu-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.56 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 24.5 and ESLint 8 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 9 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 22 and ESLint 9 on macos-latest
- GitHub Check: Lint and Test with Node.js 24.5 and ESLint 9 on macos-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 8 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 24.5 and ESLint 8 on windows-latest
- GitHub Check: Lint and Test with Node.js 24.5 and ESLint 9 on ubuntu-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 22 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 24.5 and ESLint 9 on macos-latest
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 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 22 and ESLint 8 on ubuntu-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.56 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 24.5 and ESLint 8 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 9 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 22 and ESLint 9 on macos-latest
- GitHub Check: Lint and Test with Node.js 24.5 and ESLint 9 on macos-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 8 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 24.5 and ESLint 8 on windows-latest
- GitHub Check: Lint and Test with Node.js 24.5 and ESLint 9 on ubuntu-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 22 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 24.5 and ESLint 9 on macos-latest
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Lint and Test with Node.js 20 and ESLint 9 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 22 and ESLint 8 on ubuntu-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.56 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 24.5 and ESLint 8 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 9 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 22 and ESLint 9 on macos-latest
- GitHub Check: Lint and Test with Node.js 24.5 and ESLint 9 on macos-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 8 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 24.5 and ESLint 8 on windows-latest
- GitHub Check: Lint and Test with Node.js 24.5 and ESLint 9 on ubuntu-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 22 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 24.5 and ESLint 9 on macos-latest
🔇 Additional comments (7)
patches/@swc-node+jest+1.9.1.patch (1)
2-18: Patch metadata updated — please verify it still applies cleanly with @swc-node/jest 1.9.1We’ve rebased the patch to use the new upstream index hash (4060446); the functional changes (expanded file/glob regex and returning
{code: src}) remain the same. Please confirm:• Location:
patches/@swc-node+jest+1.9.1.patch(targetsnode_modules/@swc-node/jest/lib/index.js)
• Steps to verify:
- Run
yarn install && yarn prepare- Ensure you see “Applying patches…” with no errors
(Optional) Diff for quick reference:
index 9a2fc9d..4060446 100644 --- a/node_modules/@swc-node/jest/lib/index.js +++ b/node_modules/@swc-node/jest/lib/index.js @@ -17,13 +17,13 @@ - if (/\.(tsx?|jsx?|mjs)$/.test(path)) { + if (/\.([cm]ts|tsx?|[cm]js|jsx?)$/.test(path)) { … - return src; + return { code: src };src/node-resolver.ts (2)
4-6: API migration to oxc-resolver looks correctImport switch and types align with the expected API surface; the rest of the resolver flow remains intact. Good job keeping the public resolver contract unchanged.
10-14: Confirmmodulesoption parity
NapiResolveOptionsin[email protected]declaresmodules?: string | string[]and our
createNodeResolver({ …restOptions })spreads it directly intoResolverFactory, preserving the original union and default behaviour. No changes needed.src/utils/declared-scope.ts (1)
1-14: Stronger typing for declaredScope return typeExplicit
ScopeType | undefinedimproves downstream safety with no runtime changes. LGTM.package.json (3)
78-87: Dependency swap to oxc-resolver aligns with code changesReplacing
unrs-resolverwithoxc-resolver@^11.6.1matches the migrated resolver code. No issues spotted.
64-76: Peer deps remain compatible with updated toolchainESLint ranges and @typescript-eslint utils alignment look fine given the devDep bumps. No action needed.
96-112: Verify @swc-node/jest patch for proper regex coverageI confirmed the patch returns
{ code: src }at line 17, but the expanded file-extension regex (/\.(?:[cm]ts|tsx?|[cm]js|jsx?)$/) wasn’t found in the patch. Please ensure that:
- The
transformregex inpatches/@swc-node+jest+1.9.1.patchcovers all relevant extensions:.cts,.mts,.tsx,.jsx, etc.- The patch still returns an object with
{ code }rather than a raw string.Once updated, rerun:
yarn test -w 1to validate that Jest transforms behave as expected.
SukkaW
approved these changes
Aug 22, 2025
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 5500131 in 40 seconds. Click for details.
- Reviewed
56lines of code in3files - Skipped
3files when reviewing. - Skipped posting
5draft 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. package.json:83
- Draft comment:
Bump oxc-resolver version to ^11.6.2. Ensure update compatibility with modules relying on this resolver. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. package.json:170
- Draft comment:
Add resolutions alias for unrs-resolver mapping to oxc-resolver@^11.6.2. Confirm consistent usage. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
3. test/fixtures/yarn-pnp/package.json:21
- Draft comment:
Resolutions added for unrs-resolver ensure test fixture uses oxc-resolver@^11.6.2. Verify alignment with root config. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
4. test/fixtures/yarn-pnp/test-package/package.json:8
- Draft comment:
Lint script now prepends DEBUG flag for eslint-import-resolver-typescript. Confirm this debug output is acceptable in all environments. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
5. test/fixtures/yarn-pnp/test-package/package.json:17
- Draft comment:
Using a patch dependency for eslint-import-resolver-typescript. Verify the referenced patch file exists and remains maintained. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
Workflow ID: wflow_hrhlT0IYFIQtms8e
You can customize
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed f25dd4a in 1 minute and 29 seconds. Click for details.
- Reviewed
58lines of code in3files - Skipped
3files when reviewing. - Skipped posting
5draft 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. package.json:84
- Draft comment:
Updated 'oxc-resolver' to ^11.6.2. Confirm that migration documentation clearly explains switching from unrs-resolver to oxc-resolver. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to confirm that the migration documentation is clear. This falls under the rule of not asking the author to confirm or ensure things. It doesn't provide a specific code suggestion or point out a specific issue with the code itself.
2. package.json:172
- Draft comment:
Added a resolutions mapping for 'unrs-resolver'. Verify that this alias is consistently applied across the project and documented if needed. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to verify consistency and documentation, which is against the rules. It doesn't provide a specific suggestion or point out a specific issue with the code.
3. test/fixtures/yarn-pnp/package.json:20
- Draft comment:
The resolutions for 'unrs-resolver' are now added in this PnP fixture. Ensure this matches the root configuration. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is asking the PR author to ensure that the resolutions match the root configuration, which is a form of asking them to double-check something. This violates the rule against asking the author to ensure or verify things.
4. test/fixtures/yarn-pnp/test-package/package.json:8
- Draft comment:
Lint script now prepends DEBUG for eslint-import-resolver-typescript. Consider potential cross-platform issues (e.g. using cross-env) on non-Unix systems. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% This is a test fixture package.json, not production code. The DEBUG variable is likely intentionally added for debugging purposes in tests. Cross-platform concerns are less relevant for test fixtures. Additionally, this seems more like a suggestion than a critical issue. The comment does raise a valid point about cross-platform compatibility which could be important if this were production code. Maybe test fixtures should follow best practices too? Since this is clearly a test fixture (given the path and package name), and the DEBUG flag appears intentional for testing/debugging purposes, following strict production best practices is unnecessary here. Delete the comment as it's raising a non-critical issue about cross-platform compatibility in test fixture code where it's not particularly relevant.
5. test/fixtures/yarn-pnp/test-package/package.json:18
- Draft comment:
Dependency 'eslint-import-resolver-typescript' is now specified using Yarn patch syntax. Ensure the referenced patch file exists and is maintained. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is about a dependency change and is asking the author to ensure that a patch file exists and is maintained. This falls under the rule of not commenting on dependency changes or asking the author to ensure something.
Workflow ID: wflow_bsh1jV8X3KWaTi50
You can customize
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
test/fixtures/yarn-pnp/test-package/package.json (1)
8-8: Make the DEBUG env var cross-platform.Setting environment variables inline (
DEBUG=... eslint) doesn't work on Windows shells. If these fixtures are expected to run on Windows (contributors or CI), prefer using cross-env.Apply within this hunk:
- "lint": "DEBUG=eslint-import-resolver-typescript eslint" + "lint": "cross-env DEBUG=eslint-import-resolver-typescript eslint"Outside this hunk, add cross-env to devDependencies in this fixture:
{ "devDependencies": { "cross-env": "^7.0.3" } }If your CI only runs on Linux, feel free to skip; otherwise this avoids flaky contributor workflows on Windows.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (3)
test/fixtures/yarn-pnp/.yarn/patches/eslint-import-resolver-typescript-npm-4.4.3-f982fbc045.patchis excluded by!**/.yarn/**test/fixtures/yarn-pnp/yarn.lockis excluded by!**/yarn.lock,!**/*.lockyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (3)
package.json(4 hunks)test/fixtures/yarn-pnp/package.json(1 hunks)test/fixtures/yarn-pnp/test-package/package.json(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-03-30T09:06:59.006Z
Learnt from: JounQin
PR: un-ts/eslint-plugin-import-x#271
File: test/rules/no-unused-modules.spec.ts:1528-1532
Timestamp: 2025-03-30T09:06:59.006Z
Learning: The import from 'eslint8.56/use-at-your-own-risk' has incorrect TypeScript types but works correctly at runtime, which is properly handled with a `ts-expect-error` comment.
Applied to files:
test/fixtures/yarn-pnp/test-package/package.jsonpackage.json
📚 Learning: 2025-06-18T12:49:41.795Z
Learnt from: JounQin
PR: un-ts/eslint-plugin-import-x#385
File: src/rules/prefer-namespace-import.ts:0-0
Timestamp: 2025-06-18T12:49:41.795Z
Learning: The eslint-plugin-import-x project requires Node versions `^18.18.0 || ^20.9.0 || >=21.1.0` as specified in package.json, so modern JavaScript features like String.prototype.at() are safe to use.
Applied to files:
test/fixtures/yarn-pnp/test-package/package.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
- GitHub Check: Lint and Test with Node.js 20 and ESLint 8 on windows-latest
- GitHub Check: Lint and Test with Node.js 24.5 and ESLint 8 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 8.56 on windows-latest
- GitHub Check: Lint and Test with Node.js 20 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 24.5 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 20 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 24.5 and ESLint 8 on windows-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 8.56 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 18 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 18 and ESLint 9 on ubuntu-latest
- GitHub Check: Lint and Test with Node.js 24.5 and ESLint 9 on macos-latest
- GitHub Check: Lint and Test with Node.js 24.5 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
- GitHub Check: Lint and Test with Node.js 18 and ESLint 8.56 on macos-latest
🔇 Additional comments (2)
test/fixtures/yarn-pnp/package.json (1)
20-22: Good use of Yarn alias to map unrs-resolver → oxc-resolver.The resolution ensures anything requesting
unrs-resolvergetsoxc-resolver@^11.6.2under PnP. This aligns with the project-wide migration and keeps tests representative.package.json (1)
84-84: Dependency switch to oxc-resolver looks correct.Adding
oxc-resolveras a runtime dependency matches the resolver import changes and keeps consumers self-contained.
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed c752788 in 39 seconds. Click for details.
- Reviewed
88lines of code in4files - Skipped
3files when reviewing. - Skipped posting
4draft 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:17
- Draft comment:
Ensure that commenting out older OS/node versions is intentional. A brief inline note on why only windows and the LTS include blocks are maintained may help future readers. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
2. package.json:168
- Draft comment:
The dependency update for oxc-resolver (from ^11.6.1 to ^11.6.2) and the added unrs-resolver alias look correct. Confirm that all internal references and peer expectations are aligned with this change. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
3. test/fixtures/yarn-pnp/package.json:20
- Draft comment:
The added unrs-resolver resolution here aligns with the main package. Verify consistency across all Yarn workspaces. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
4. test/fixtures/yarn-pnp/test-package/package.json:8
- Draft comment:
The lint script now prefixes with 'DEBUG=eslint-import-resolver-typescript'. Confirm this debug flag is intended only for testing and not accidentally left in production scripts. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
Workflow ID: wflow_1JH8cati8ioGWNbi
You can customize
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed fe3c8e0 in 33 seconds. Click for details.
- Reviewed
88lines of code in4files - Skipped
3files when reviewing. - Skipped posting
4draft 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:17
- Draft comment:
Matrix OS and Node version entries are now commented out; only windows-latest is active by default. Confirm that this isn’t an unintended reduction in test coverage. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to confirm their intention regarding the reduction in test coverage. This violates the rule against asking the author to confirm their intention or ensure behavior is intended.
2. package.json:170
- Draft comment:
The migration from unrs-resolver to oxc-resolver is implemented via dependency update and resolutions alias. Verify that this alias covers all usage scenarios. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to verify that the alias covers all usage scenarios, which is against the rules. It does not provide a specific suggestion or point out a specific issue with the code.
3. test/fixtures/yarn-pnp/package.json:20
- Draft comment:
Resolution for 'unrs-resolver' is updated consistently in the test fixture package.json. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment seems to be purely informative, as it is just stating that a resolution is updated consistently. It doesn't provide any actionable feedback or suggestions for improvement.
4. test/fixtures/yarn-pnp/test-package/package.json:8
- Draft comment:
The lint script now includes a DEBUG flag (DEBUG=eslint-import-resolver-typescript). Ensure this extra logging is intentional for test debugging. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the author to ensure that the extra logging is intentional, which violates the rule against asking the author to confirm their intention. It doesn't provide a specific suggestion or point out a clear issue with the code.
Workflow ID: wflow_5TbEiDf12yaKLWlt
You can customize
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed c785248 in 31 seconds. Click for details.
- Reviewed
88lines of code in4files - Skipped
3files when reviewing. - Skipped posting
4draft 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:17
- Draft comment:
Matrix config now only tests Windows (node 24.5, ESLint 9); confirm that disabling macOS/Ubuntu and other Node/ESLint versions is intentional. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
2. package.json:168
- Draft comment:
The resolution mapping for 'unrs-resolver' aliasing to 'oxc-resolver@^11.6.2' is added—verify that this change aligns with all dependency requirements. - Reason this comment was not posted:
Confidence changes required:20%<= threshold50%None
3. test/fixtures/yarn-pnp/package.json:21
- Draft comment:
The resolution for 'unrs-resolver' has been added here as well, matching the root package config. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
4. test/fixtures/yarn-pnp/test-package/package.json:8
- Draft comment:
The lint script now prepends 'DEBUG=eslint-import-resolver-typescript'; ensure that verbose debugging is desired in all environments. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
Workflow ID: wflow_ZkEBGGDls9dXH1sh
You can customize
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed fb7d04a in 1 minute and 25 seconds. Click for details.
- Reviewed
88lines of code in4files - Skipped
3files when reviewing. - Skipped posting
5draft 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:17
- Draft comment:
Ensure that the commented-out OS and Node/Eslint matrix entries (e.g. macos-latest, ubuntu-latest, Node 18/20/22) are intentionally disabled to avoid reducing CI coverage. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to ensure that certain matrix entries are intentionally disabled. This falls under the rule of not asking the author to confirm their intention or to ensure behavior is intended. Therefore, this comment should be removed.
2. package.json:81
- Draft comment:
Bump of 'oxc-resolver' to ^11.6.2 properly reflects the migration from unrs-resolver. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative and discusses a dependency change. It doesn't provide a specific suggestion or ask for confirmation about a specific aspect of the code. It violates the rule against making purely informative comments and commenting on dependency changes.
3. package.json:168
- Draft comment:
The added resolution mapping for 'unrs-resolver' to oxc-resolver ensures backward compatibility; verify that this mapping meets the migration intent. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to verify the migration intent, which is against the rules. It does not provide a specific suggestion or point out a clear issue with the code.
4. test/fixtures/yarn-pnp/package.json:20
- Draft comment:
Test fixture resolutions now map 'unrs-resolver' to oxc-resolver; this change is consistent with the main migration. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative, explaining a change in test fixture resolutions. It doesn't provide a suggestion, ask for confirmation, or point out a potential issue. It seems to violate the rule against making purely informative comments.
5. test/fixtures/yarn-pnp/test-package/package.json:8
- Draft comment:
The lint script now prepends DEBUG and RUST_BACKTRACE; consider using a cross‐platform mechanism (e.g. cross-env) for setting environment variables to ensure compatibility on Windows. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% This is a test fixture file, likely used for testing the eslint-plugin-import-x package. The environment variables added (DEBUG and RUST_BACKTRACE) are debug-related, suggesting they're for development/debugging purposes. Since this is a test fixture, cross-platform compatibility is less critical than in production code. The comment does point out a real potential issue with Windows compatibility. Even test fixtures should ideally work cross-platform. However, these are debug-related environment variables in a test fixture, likely used temporarily for development. The comment violates the rule about not making purely informative comments without clear required changes. Delete the comment as it's more informative than actionable, and it's in a test fixture where cross-platform compatibility is less critical.
Workflow ID: wflow_GPxmhkdkpqVlxqXJ
You can customize
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.github/workflows/ci.yml (2)
17-18: OS matrix trimmed; consider restoring at least Ubuntu for baseline Linux coverageoxc-resolver ships native bindings; platform differences (path case-sensitivity, symlink behavior, binary loading) can bite. You’ve reintroduced Ubuntu only via “include” for two scenarios, but the main matrix now runs mostly on Windows. Recommend making Linux a first-class matrix entry so every node/eslint combo gets Linux coverage; keep macOS optional if CI time is a concern.
Proposed tweak:
matrix: os: - # - macos-latest - # - ubuntu-latest + # - macos-latest + - ubuntu-latest - windows-latest
27-28: Re-enable ESLint v8.57.0 workflow lane to match peerDependenciesYour package.json declares ESLint support as
^8.57.0 || ^9.0.0, so running CI only against v9 risks regressions for v8 users. Let’s add back a single ESLint 8 job pinned to the minimum supported version (8.57.0):
- File:
.github/workflows/ci.yml- Peer range from
package.json:^8.57.0 || ^9.0.0Suggested diff:
matrix: eslint: - # - 8.56 - # - 8 + - 8.57.0 - 9This ensures the “Install ESLint ${{ matrix.eslint }}” step runs for both 8.57.0 and 9.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (3)
test/fixtures/yarn-pnp/.yarn/patches/eslint-import-resolver-typescript-npm-4.4.3-f982fbc045.patchis excluded by!**/.yarn/**test/fixtures/yarn-pnp/yarn.lockis excluded by!**/yarn.lock,!**/*.lockyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (4)
.github/workflows/ci.yml(1 hunks)package.json(4 hunks)test/fixtures/yarn-pnp/package.json(1 hunks)test/fixtures/yarn-pnp/test-package/package.json(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- test/fixtures/yarn-pnp/package.json
- test/fixtures/yarn-pnp/test-package/package.json
- package.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Lint and Test with Node.js 24.5 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 24.5 and ESLint 9 on windows-latest
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 28a608a in 58 seconds. Click for details.
- Reviewed
90lines of code in5files - Skipped
3files when reviewing. - Skipped posting
5draft 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:17
- Draft comment:
Clarify the CI matrix reduction: commented-out OS and Node versions might confuse contributors. Add a comment explaining the intentional reduction. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
2. package.json:169
- Draft comment:
The resolutions now map 'unrs-resolver' to oxc-resolver v11.6.2. Confirm all code references are updated accordingly. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
3. test/fixtures/yarn-pnp/.env.yarn:3
- Draft comment:
RUST_BACKTRACE has been added. Consider adding a comment to document its purpose for debugging. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
4. test/fixtures/yarn-pnp/package.json:16
- Draft comment:
The patch directive for eslint-import-resolver-typescript and the resolution for 'unrs-resolver' ensure consistency. Confirm these changes align with fixture expectations. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
5. test/fixtures/yarn-pnp/test-package/package.json:16
- Draft comment:
Pinned eslint-import-resolver-typescript to a patch version for consistency. Verify the patch remains compatible with upstream updates. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
Workflow ID: wflow_vE6QCzBpMYm0NbhB
You can customize
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed e8b61a0 in 48 seconds. Click for details.
- Reviewed
90lines of code in5files - Skipped
3files when reviewing. - Skipped posting
6draft 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:17
- Draft comment:
Consider removing the commented-out OS and Node.js versions if they aren’t needed; if kept for future reference, add a clarifying note. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
2. package.json:84
- Draft comment:
Bump of 'oxc-resolver' to ^11.6.2 and addition of the 'unrs-resolver' alias look correct; ensure all consumers reference the updated resolver. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
3. package.json:172
- Draft comment:
The resolutions now include an alias for 'unrs-resolver'; verify this change consistently propagates to all workspaces. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
4. test/fixtures/yarn-pnp/.env.yarn:1
- Draft comment:
DEBUG and RUST_BACKTRACE env variables are added for debugging; ensure they’re appropriate only for test usage. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
5. test/fixtures/yarn-pnp/package.json:16
- Draft comment:
The workspace package now uses a patched 'eslint-import-resolver-typescript' and adds the 'unrs-resolver' alias; confirm that these changes are synced with the main configuration. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
6. test/fixtures/yarn-pnp/test-package/package.json:18
- Draft comment:
Switching 'eslint-import-resolver-typescript' to use the patch reference ensures consistency; please verify the patch file remains valid with future updates. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
Workflow ID: wflow_DiYYTIkyoVkXUbxW
You can customize
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.github/workflows/ci.yml (1)
21-25: Align Node.js matrix with declared engines and N-API (oxc-resolver) coverageYou’ve commented out Node 18/20/22 and only run 24.5. If
package.json.engines.nodestill lists^18.18 || ^20.9 || >=21.1, CI should exercise at least 22 (Active LTS) and ideally 20 as well. This also validates oxc-resolver N-API prebuilds across multiple ABIs, reducing “works on my Node” surprises. If you intend to drop 18/20, update engines accordingly to avoid false advertising.Minimal adjustment (keep fast lanes, add back coverage where it matters):
matrix: node: - # - 18 - # - 20 - # - 22 + # - 18 + - 20 + - 22 # https://github.com/nodejs/node/issues/59480 - 24.5Run this to confirm your engines and decide which lanes to keep vs. drop:
#!/bin/bash set -euo pipefail python - <<'PY' import json pkg=json.load(open('package.json')) print("engines.node:", pkg.get('engines',{}).get('node')) PY
🧹 Nitpick comments (2)
.github/workflows/ci.yml (2)
17-19: Re-enable at least Linux coverage to catch platform-specific native (N-API) issuesWith macOS/Linux commented out, most combinations only run on Windows. oxc-resolver ships native binaries; having at least one Linux lane is valuable to catch prebuild or toolchain regressions. Since you already include two Ubuntu combos via the “include” matrix, make Linux a first-class OS in the main matrix to exercise the same node versions consistently.
matrix: os: - # - macos-latest - # - ubuntu-latest + # - macos-latest + - ubuntu-latest - windows-latestOptional: keep macOS disabled here, but consider a scheduled job to run macOS weekly to validate prebuilds without slowing PRs.
27-29: ESLint matrix collapsed to 9; remove the dead “Install ESLint” step or keep it guarded for future useWith only
eslint: 9in the matrix, the “Install ESLint” step is never executed. Either remove it (simpler) or leave a comment explaining it’s intentionally dormant for back-compat lanes.Lean removal:
- - name: Install ESLint ${{ matrix.eslint }} - if: ${{ matrix.eslint != 9 }} - run: | - yarn add -D eslint@${{ matrix.eslint }} eslint-plugin-unicorn@56 - yarn --no-immutableIf you remove the step, also simplify the job name to avoid a dangling
${{ matrix.eslint }}:- name: Lint and Test with Node.js ${{ matrix.node }} and ESLint ${{ matrix.eslint }} on ${{ matrix.os }} + name: Lint and Test with Node.js ${{ matrix.node }} on ${{ matrix.os }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (3)
test/fixtures/yarn-pnp/.yarn/patches/eslint-import-resolver-typescript-npm-4.4.3-f982fbc045.patchis excluded by!**/.yarn/**test/fixtures/yarn-pnp/yarn.lockis excluded by!**/yarn.lock,!**/*.lockyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (5)
.github/workflows/ci.yml(1 hunks)package.json(4 hunks)test/fixtures/yarn-pnp/.env.yarn(1 hunks)test/fixtures/yarn-pnp/package.json(1 hunks)test/fixtures/yarn-pnp/test-package/package.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- test/fixtures/yarn-pnp/.env.yarn
🚧 Files skipped from review as they are similar to previous changes (3)
- test/fixtures/yarn-pnp/test-package/package.json
- test/fixtures/yarn-pnp/package.json
- package.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Lint and Test with Node.js 24.5 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 24.5 and ESLint 9 on windows-latest
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 488ba10 in 54 seconds. Click for details.
- Reviewed
107lines of code in6files - Skipped
3files when reviewing. - Skipped posting
8draft 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:17
- Draft comment:
Matrix now only includes 'windows-latest' and Node 24.5; confirm this reduced test matrix is intentional. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
2. package.json:81
- Draft comment:
Bumped 'oxc-resolver' version from ^11.6.1 to ^11.6.2; verify compatibility with all consumers. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
3. package.json:168
- Draft comment:
Added resolution for 'unrs-resolver' alias to 'oxc-resolver@^11.6.2'; ensure dependent modules resolve as expected. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
4. src/node-resolver.ts:37
- Draft comment:
Using raw console logging in the error handler; consider conditional logging or a debug library to avoid noisy production output. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
5. test/fixtures/yarn-pnp/.env.yarn:1
- Draft comment:
Added DEBUG and RUST_BACKTRACE env vars; ensure these debug settings remain limited to testing environments. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
6. test/fixtures/yarn-pnp/package.json:18
- Draft comment:
Patched 'eslint-import-resolver-typescript' is now specified; verify the patch file and version consistency with updates. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
7. test/fixtures/yarn-pnp/package.json:23
- Draft comment:
Added resolution for 'unrs-resolver' pointing to 'oxc-resolver@^11.6.2'; ensure this aligns with the root configuration. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
8. test/fixtures/yarn-pnp/test-package/package.json:16
- Draft comment:
Updated 'eslint-import-resolver-typescript' to use the patched version; confirm that the patch applies correctly within the test-package. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
Workflow ID: wflow_2wTcMjJnH6nYCzhb
You can customize
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed 59fd3a3 in 1 minute and 18 seconds. Click for details.
- Reviewed
113lines of code in6files - Skipped
3files when reviewing. - Skipped posting
5draft 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:17
- Draft comment:
The matrix now has several entries commented out (e.g. macos-latest, ubuntu-latest, Node 18/20/22). If these are no longer used, consider removing them or add a clarifying comment for future maintainers. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% The comment suggests either removing commented code or adding documentation. Commented code can be confusing, but in CI configs it's common to keep commented options as reference. There's already a comment explaining the Node 24.5 choice. The history is preserved in git if needed. This feels like a style preference rather than a clear issue requiring action. The commented entries might be temporarily disabled for debugging or might be re-enabled later. Having them visible could help maintainers quickly adjust test coverage. While the commented code could be useful reference, it doesn't clearly indicate a problem that needs fixing. The comment is more of a style suggestion than a required change. This comment should be removed as it's making a style suggestion rather than identifying a clear issue that needs to be fixed. The commented entries serve as useful reference and their presence isn't harmful.
2. package.json:170
- Draft comment:
The resolution now aliases 'unrs-resolver' to 'oxc-resolver' at version ^11.6.2. Ensure documentation and dependent packages are updated to reflect this migration. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is asking the PR author to ensure documentation and dependent packages are updated, which violates the rule against asking the author to ensure things are updated or tested. It also mentions dependency changes, which we should not comment on.
3. test/fixtures/yarn-pnp/.env.yarn:3
- Draft comment:
RUST_BACKTRACE=full has been added. Confirm that this verbose debug setting is intended for the test environment. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
4. test/fixtures/yarn-pnp/package.json:13
- Draft comment:
The root lint script was changed to only run the workspace lint command (removing the standalone 'eslint' run). Verify that this change is intentional and that no lint checks are skipped at the root level. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to verify their intention, which is against the rules. It does not provide a specific suggestion or point out a clear issue with the code. Therefore, it should be removed.
5. test/fixtures/yarn-pnp/test-package/package.json:15
- Draft comment:
The dependency for 'eslint-import-resolver-typescript' was updated to use a patched version. Ensure that the patch file exists at the specified location and that the change is documented for maintainers. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is related to a dependency change, which is against the rules for making comments. It asks the author to ensure the patch file exists and to document the change, which is not allowed.
Workflow ID: wflow_UwULISMdqhP1n1Uw
You can customize
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed b5c22ab in 1 minute and 16 seconds. Click for details.
- Reviewed
111lines of code in6files - Skipped
3files when reviewing. - Skipped posting
4draft 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. package.json:170
- Draft comment:
Document why 'unrs-resolver' is aliased to 'oxc-resolver' in resolutions for clarity. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking for documentation on why a specific alias is used in the code. This is a request for clarification, which is not allowed according to the rules. The comment does not provide a specific code suggestion or point out a potential issue with the code itself.
2. test/fixtures/yarn-pnp/.env.yarn:3
- Draft comment:
Ensure setting RUST_BACKTRACE=full is intended for this test fixture environment. - Reason this comment was not posted:
Confidence changes required:30%<= threshold50%None
3. test/fixtures/yarn-pnp/package.json:13
- Draft comment:
Validate that removing the top-level 'eslint' command (from 'eslint && yarn workspace test-package lint') is intentional. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to confirm their intention regarding the removal of a command. This is not allowed as per the rules, which prohibit asking the author to confirm their intention or ensure behavior is intended.
4. test/fixtures/yarn-pnp/test-package/package.json:18
- Draft comment:
Pinning 'eslint-import-resolver-typescript' to a patched version increases maintenance; confirm the patch is necessary. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to confirm if pinning a dependency to a patched version is necessary. This falls under asking the author to confirm their intention, which is against the rules. Therefore, this comment should be removed.
Workflow ID: wflow_gT4OnqRcZIS2wy2q
You can customize
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed 910f54a in 1 minute and 20 seconds. Click for details.
- Reviewed
120lines of code in6files - Skipped
3files when reviewing. - Skipped posting
5draft 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:17
- Draft comment:
Consider removing or adding a comment explaining the commented-out OS/Node/ESLint matrix entries so future maintainers know if they're intentionally disabled or obsolete. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%None
2. package.json:170
- Draft comment:
The resolutions block now aliases 'unrs-resolver' to 'oxc-resolver'. Ensure this migration is documented for developers who might expect 'unrs-resolver' as a dependency. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
3. test/fixtures/yarn-pnp/package.json:13
- Draft comment:
The root lint script changed from running 'eslint' and a workspace lint to just the workspace lint. Confirm that running ESLint at the root is no longer needed. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%None
4. test/fixtures/yarn-pnp/test-package/package.json:8
- Draft comment:
The lint command now explicitly targets 'test.ts'. Verify that this doesn't miss linting other necessary files for the test package. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
5. test/fixtures/yarn-pnp/.env.yarn:3
- Draft comment:
Enabling 'RUST_BACKTRACE=full' in the test fixture can be useful for debugging but could lead to verbose logs. Ensure this setting is intended for all test runs. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
Workflow ID: wflow_pdBHOpqvP39SpVYU
You can customize
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
test/fixtures/yarn-pnp/test-package/package.json (1)
18-18: Use a project-relative Yarn patch path (do not rely on ~/.yarn).This fixture should reference the committed patch under the workspace, not the user’s home directory, to be reproducible in CI and for contributors.
- "eslint-import-resolver-typescript": "patch:eslint-import-resolver-typescript@npm%3A4.4.3#~/.yarn/patches/eslint-import-resolver-typescript-npm-4.4.3-f982fbc045.patch", + "eslint-import-resolver-typescript": "patch:eslint-import-resolver-typescript@npm%3A4.4.3#../.yarn/patches/eslint-import-resolver-typescript-npm-4.4.3-f982fbc045.patch",
🧹 Nitpick comments (1)
test/fixtures/yarn-pnp/test-package/package.json (1)
8-8: Confirm intent: linting only test.ts reduces coverage.If this is deliberate for the PnP fixture, ignore. Otherwise consider linting the whole fixture to better simulate user setups.
- "lint": "eslint test.ts" + "lint": "eslint ."
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (3)
test/fixtures/yarn-pnp/.yarn/patches/eslint-import-resolver-typescript-npm-4.4.3-f982fbc045.patchis excluded by!**/.yarn/**test/fixtures/yarn-pnp/yarn.lockis excluded by!**/yarn.lock,!**/*.lockyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (6)
.github/workflows/ci.yml(1 hunks)package.json(4 hunks)src/node-resolver.ts(2 hunks)test/fixtures/yarn-pnp/.env.yarn(1 hunks)test/fixtures/yarn-pnp/package.json(1 hunks)test/fixtures/yarn-pnp/test-package/package.json(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- .github/workflows/ci.yml
- test/fixtures/yarn-pnp/.env.yarn
- src/node-resolver.ts
- test/fixtures/yarn-pnp/package.json
- package.json
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-03-30T09:06:59.006Z
Learnt from: JounQin
PR: un-ts/eslint-plugin-import-x#271
File: test/rules/no-unused-modules.spec.ts:1528-1532
Timestamp: 2025-03-30T09:06:59.006Z
Learning: The import from 'eslint8.56/use-at-your-own-risk' has incorrect TypeScript types but works correctly at runtime, which is properly handled with a `ts-expect-error` comment.
Applied to files:
test/fixtures/yarn-pnp/test-package/package.json
📚 Learning: 2025-06-18T12:49:41.795Z
Learnt from: JounQin
PR: un-ts/eslint-plugin-import-x#385
File: src/rules/prefer-namespace-import.ts:0-0
Timestamp: 2025-06-18T12:49:41.795Z
Learning: The eslint-plugin-import-x project requires Node versions `^18.18.0 || ^20.9.0 || >=21.1.0` as specified in package.json, so modern JavaScript features like String.prototype.at() are safe to use.
Applied to files:
test/fixtures/yarn-pnp/test-package/package.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Socket Security: Pull Request Alerts
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Lint and Test with Node.js 24.5 and ESLint 9 on windows-latest
- GitHub Check: Lint and Test with Node.js 24.5 and ESLint 9 on windows-latest
JounQin
force-pushed
the
chore/deps
branch
11 times, most recently
from
2696be7 to
3e54b26
Compare
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.
Important
Migrate from
unrs-resolvertooxc-resolver, update dependencies, and refine CI configuration without affecting public API or runtime behavior.unrs-resolverwithoxc-resolverinpackage.jsonandnode-resolver.ts.package.json..github/workflows/ci.ymlby commenting out certain OS and Node.js versions.createNodeResolver()innode-resolver.ts.declaredScope()indeclared-scope.tstoScopeType | undefined..env.yarn.package.jsoninyarn-pnpandtest-packageto useoxc-resolver.This description was created by
for 910f54a. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit