Skip to content

fix(#473): Strip querystrings when checking for file existence#479

Open
mrginglymus wants to merge 2 commits intoun-ts:masterfrom
mrginglymus:case-sensitive-qs
Open

fix(#473): Strip querystrings when checking for file existence#479
mrginglymus wants to merge 2 commits intoun-ts:masterfrom
mrginglymus:case-sensitive-qs

Conversation

@mrginglymus
Copy link
Copy Markdown
Contributor

@mrginglymus mrginglymus commented Mar 27, 2026

This prevents false positives on case mismatches on case sensitive file systems when imports have a querystring.

Summary by CodeRabbit

  • Bug Fixes
    • Improved file resolution to correctly handle import statements with query string parameters, ensuring they properly resolve to the target file regardless of query suffix differences.

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Mar 27, 2026

⚠️ No Changeset found

Latest commit: 40adcc5

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 27, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2ce2c327-33d7-4c9b-b73e-f29a301d2ca9

📥 Commits

Reviewing files that changed from the base of the PR and between 4b2c0c5 and 40adcc5.

📒 Files selected for processing (2)
  • src/utils/resolve.ts
  • test/rules/no-unresolved.spec.ts

📝 Walkthrough

Walkthrough

The PR modifies file path resolution to strip query string suffixes from filepaths before performing filesystem checks and cache lookups. This ensures paths differing only by query parameters are treated as identical for resolution purposes. A corresponding test case validates this behavior.

Changes

Cohort / File(s) Summary
Path Resolution Logic
src/utils/resolve.ts
Added querystring stripping (filepath.split('?')[0]) in fileExistsWithCaseSync immediately after null-handling to normalize paths before cache lookup and filesystem comparisons.
Test Coverage
test/rules/no-unresolved.spec.ts
Added test case import foo from "./bar?qs"; to validate querystring handling in both node and webpack resolver configurations.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

  • JounQin

Poem

🐰 A query string hops through the path so neat,
Split and sorted, the resolution's complete!
No more confusion when ?qs appears,
The resolver hops on, no filesystem fears! 🌿

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly and clearly describes the main change: stripping querystrings when checking file existence, which matches the primary modification in src/utils/resolve.ts.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codesandbox-ci
Copy link
Copy Markdown

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.

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new bot commented Mar 27, 2026

Open in StackBlitz

npm i https://pkg.pr.new/eslint-plugin-import-x@479

commit: 40adcc5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

import-x/no-unresolved: false positive casing mismatch for query string imports on macOS

1 participant