chore(deps): migrate unrs-resolver to new oxc-resolver#479
chore(deps): migrate unrs-resolver to new oxc-resolver#479oceandrama wants to merge 3 commits intoimport-js:masterfrom
Conversation
🦋 Changeset detectedLatest commit: e097b2a 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 |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughMigrates resolver usage from Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
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. |
commit: |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
| const resolver = createTypeScriptImportResolver() | ||
|
|
||
| it( | ||
| it.skipIf(!process.versions.pnp)( |
There was a problem hiding this comment.
pnp mode works only if process.versions.pnp is set, so tests may be skipped otherwise. I have the same behavior with the latest version of unrs-resolver since it has the same implementation, so I don't think this is an issue.
There was a problem hiding this comment.
Nope, the test starts another process, which doesn't depend on pnp on main thread.
There was a problem hiding this comment.
So, what's the root cause of the problem? It doesn't work with either the latest versions of unrs-resolver or oxc-resolver and I'm not sure if pnp is broken in both packages or tests are outdated?
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@package.json`:
- Line 76: The TypeScriptResolverOptions interface claims to pass through
resolver options from NapiResolveOptions but oxc-resolver no longer supports the
descriptionFiles option (previously in unrs-resolver); add a compatibility shim
that reads descriptionFiles from the resolver config and translates it into
equivalent behavior (e.g., implement logic in the resolver initialization code
or a new adapter function that honors descriptionFiles when resolving packages)
or mark this change as breaking by updating the package metadata and release
notes; reference TypeScriptResolverOptions, NapiResolveOptions,
descriptionFiles, and the resolver initialization/adapter function to locate
where to implement the shim or where to change the release type.
In `@src/normalize-options.ts`:
- Around line 35-36: The destructuring assumes tsconfig is an object but crashes
when tsconfig is null because typeof null === "object"; update the guard around
the destructuring (the usage of tsconfig and the line assigning
configFile/references) to ensure tsconfig is not null/undefined before treating
it as an object (e.g., check tsconfig != null && typeof tsconfig === 'object' or
tsconfig && typeof tsconfig === 'object'), then perform the destructure from
that safe value (or default to {}); adjust the let { configFile, references }:
Partial<TsconfigOptions> = ... assignment accordingly so { configFile,
references } never attempts to destructure null.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8a7ce38d-55b1-4313-b199-c4f60fff9dcc
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (6)
.changeset/silly-beans-pretend.mdREADME.mdpackage.jsonsrc/index.tssrc/normalize-options.tssrc/types.ts
| "eslint-import-context": "^0.2.0", | ||
| "get-tsconfig": "^4.10.1", | ||
| "is-bun-module": "^2.0.0", | ||
| "oxc-resolver": "^11.19.1", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify that the public surface still exposes passthrough resolver options
# and that there is no repo-local compatibility shim/coverage for descriptionFiles.
sed -n '1,10p' src/types.ts
sed -n '331,335p' README.md
rg -n -C2 '\bdescriptionFiles\b'Repository: import-js/eslint-import-resolver-typescript
Length of output: 565
This dependency swap removes user-accessible resolver options.
The TypeScriptResolverOptions interface extends oxc-resolver's NapiResolveOptions and explicitly documents pass-through of additional options. However, oxc-resolver does not implement descriptionFiles, which was available in the previous unrs-resolver backend. No compatibility shim for this option exists in the codebase. This is a breaking change for users passing descriptionFiles through the resolver config. Either add a compatibility layer for dropped options or release as breaking instead of patch.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` at line 76, The TypeScriptResolverOptions interface claims to
pass through resolver options from NapiResolveOptions but oxc-resolver no longer
supports the descriptionFiles option (previously in unrs-resolver); add a
compatibility shim that reads descriptionFiles from the resolver config and
translates it into equivalent behavior (e.g., implement logic in the resolver
initialization code or a new adapter function that honors descriptionFiles when
resolving packages) or mark this change as breaking by updating the package
metadata and release notes; reference TypeScriptResolverOptions,
NapiResolveOptions, descriptionFiles, and the resolver initialization/adapter
function to locate where to implement the shim or where to change the release
type.
| let { configFile, references }: Partial<TsconfigOptions> = | ||
| typeof tsconfig === 'object' ? tsconfig : {} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect the current guard for non-object `tsconfig` values.
sed -n '33,36p' src/normalize-options.tsRepository: import-js/eslint-import-resolver-typescript
Length of output: 270
🏁 Script executed:
cat -n src/normalize-options.ts | head -60Repository: import-js/eslint-import-resolver-typescript
Length of output: 2318
🏁 Script executed:
rg "tsconfig" src/normalize-options.ts -B 3 -A 3Repository: import-js/eslint-import-resolver-typescript
Length of output: 868
🏁 Script executed:
# Check type definitions for the options parameter
rg "TsconfigOptions|NormalizeOptions" src/normalize-options.ts -B 2 -A 5Repository: import-js/eslint-import-resolver-typescript
Length of output: 510
🏁 Script executed:
# Look for tests related to tsconfig null/undefined handling
fd "test|spec" -type f | head -10Repository: import-js/eslint-import-resolver-typescript
Length of output: 257
🏁 Script executed:
fd -e ts -e tsx | xargs grep -l "TypeScriptResolverOptions" | head -5Repository: import-js/eslint-import-resolver-typescript
Length of output: 137
🏁 Script executed:
rg "TypeScriptResolverOptions" src/ -A 15 --type tsRepository: import-js/eslint-import-resolver-typescript
Length of output: 4451
🏁 Script executed:
# Look for test files
find . -name "*.test.*" -o -name "*.spec.*" -o -name "__tests__" -type d 2>/dev/nullRepository: import-js/eslint-import-resolver-typescript
Length of output: 179
🏁 Script executed:
cat -n src/types.tsRepository: import-js/eslint-import-resolver-typescript
Length of output: 528
🏁 Script executed:
# Check if NapiResolveOptions is defined locally or imported
rg "NapiResolveOptions" src/types.ts -B 2 -A 2Repository: import-js/eslint-import-resolver-typescript
Length of output: 320
🏁 Script executed:
# Look at test files to see how tsconfig is used
rg "tsconfig" tests/ -B 2 -A 2Repository: import-js/eslint-import-resolver-typescript
Length of output: 6564
tsconfig: null still crashes this branch.
typeof null evaluates to "object" in JavaScript, so this guard still selects null and the destructuring throws before the {} fallback is used. If options contain { tsconfig: null }, the code crashes on line 36 when attempting to destructure the null value.
Proposed fix
- let { configFile, references }: Partial<TsconfigOptions> =
- typeof tsconfig === 'object' ? tsconfig : {}
+ let { configFile, references }: Partial<TsconfigOptions> =
+ tsconfig != null && typeof tsconfig === 'object' ? tsconfig : {}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let { configFile, references }: Partial<TsconfigOptions> = | |
| typeof tsconfig === 'object' ? tsconfig : {} | |
| let { configFile, references }: Partial<TsconfigOptions> = | |
| tsconfig != null && typeof tsconfig === 'object' ? tsconfig : {} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/normalize-options.ts` around lines 35 - 36, The destructuring assumes
tsconfig is an object but crashes when tsconfig is null because typeof null ===
"object"; update the guard around the destructuring (the usage of tsconfig and
the line assigning configFile/references) to ensure tsconfig is not
null/undefined before treating it as an object (e.g., check tsconfig != null &&
typeof tsconfig === 'object' or tsconfig && typeof tsconfig === 'object'), then
perform the destructure from that safe value (or default to {}); adjust the let
{ configFile, references }: Partial<TsconfigOptions> = ... assignment
accordingly so { configFile, references } never attempts to destructure null.
| let { project, tsconfig, noWarnOnMultipleProjects } = (options ||= {}) | ||
|
|
||
| let { configFile, references }: Partial<TsconfigOptions> = tsconfig ?? {} | ||
| let { configFile, references }: Partial<TsconfigOptions> = |
There was a problem hiding this comment.
I'm not sure what to do with the auto option, so I decided to match the behavior with the undefined value
2 participants
Closes #470
Summary by CodeRabbit
Chores
Documentation
Tests