fix(resolve): strip query string from id before filesystem resolution

[LeSingh1]

Problem

When a module specifier contains a query string — a common pattern for cache-busting (./foo.js?v=123) or Vite/Rollup HMR invalidation (./foo.js?t=1718000000) — resolveSync / resolvePath fail with ERR_MODULE_NOT_FOUND.

The root cause: the query string is never stripped before the id is handed to moduleResolve (from import-meta-resolve) or to statSync. The OS/resolver looks for a file literally named foo.js?v=123 which does not exist on disk.

Reproduction (before fix)

import { resolveSync } from 'mlly'

// Throws: Cannot find module ./foo.js?v=123
resolveSync('./foo.js?v=123', { url: import.meta.url })

This surfaces in real projects wherever a bundler/dev-server passes query-annotated specifiers through mlly's resolution utilities (Nuxt, Nitro, Vite SSR, etc.).

Fix

Strip the query portion of id (everything from the first ? onwards) early in _resolve(), after the file:// fast-path but before any filesystem or moduleResolve calls. This mirrors what Node's own fileURLToPath already does for file:// URLs, and what sanitizeFilePath (in utils.ts) already does for the path-sanitisation helper.

The query is meaningless for local file resolution — it carries no information about which file to open.

+  // Strip query string (e.g. cache-busting `?v=123`) before filesystem resolution.
+  const queryIndex = id.indexOf('?')
+  if (queryIndex !== -1) {
+    id = id.slice(0, queryIndex)
+  }

Tests added (test/resolve.test.ts)

Case	Description
relative path with query	./fixture/cjs.mjs?v=123 resolves identically to ./fixture/cjs.mjs
absolute file URL with query	file:///path/to/cjs.mjs?t=456 resolves to the real file
bare specifier with query	ufo?hash=abc resolves identically to ufo
resolvePathSync end-to-end	returns a real filesystem path with no ? in it

All 202 existing tests continue to pass.

Checklist

• Bug reproduced against main before fix
• Fix is minimal and targeted (8 lines in _resolve)
• No behaviour change for http:, https:, node:, data: specifiers (those return early before the new code runs)
• Full test suite green (202 passed)

Summary by CodeRabbit

• Bug Fixes

  • Fixed module resolution to properly handle cache-busting URLs by stripping query strings from identifiers.

• Tests

  • Added test coverage for query string stripping in module resolution across relative paths, absolute URLs, and package specifiers.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7ff69bce-ced1-4cf9-a92f-0741b4f76487

📥 Commits

Reviewing files that changed from the base of the PR and between beece9b and d9e8301.

📒 Files selected for processing (2)

• src/resolve.ts
• test/resolve.test.ts

---

📝 Walkthrough

Walkthrough

_resolve() in src/resolve.ts now strips any query string from the module identifier by truncating at the first ? before performing filesystem resolution. A new test suite in test/resolve.test.ts verifies this behavior for relative paths, absolute file URLs, and bare package specifiers via both resolveSync and resolvePathSync.

Changes

Query String Stripping in _resolve()

Layer / File(s)	Summary
Query string stripping implementation and tests src/resolve.ts, test/resolve.test.ts	_resolve() truncates id at the first ? before resolution. New query string stripping test suite covers resolveSync and resolvePathSync for relative paths (./fixture/cjs.mjs?v=123), absolute file URLs, bare package specifiers (ufo?hash=abc), and confirms resolvePathSync returns a real filesystem path without a ?.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

Poem

🐇 Hop, hop, what's this I see?
A query string trailing after ?v=3!
Snip-snip goes the resolver's ear,
The path comes out crisp and clear.
No ?hash=abc shall confuse my burrow —
Just clean module paths, straight as an arrow! 🌿

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix(resolve): strip query string from id before filesystem resolution' clearly and specifically summarizes the main change—stripping query strings during module resolution to fix cache-busting URL failures.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ 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

• X
• Mastodon
• Reddit
• LinkedIn

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