feat: add ESM file:// protocol support with comprehensive tests #746

Boshen · 2025-10-03T14:01:47Z

closes #744

Summary

Add full ESM file:// protocol URL resolution support across all platforms (macOS, Linux, Windows) with comprehensive test coverage.

Changes

Implementation

Enable cross-platform support: Move url crate dependency from Windows-only to all platforms
Preserve URL metadata: Query strings and fragments are preserved when converting file:// URLs to paths
WebAssembly compatibility: Use cfg_if to exclude file:// protocol handling on wasm32 targets (no filesystem access)

Testing

Add comprehensive test suite in napi/tests/file-protocol.test.mjs:

✅ Absolute file:// URLs using pathToFileURL()
✅ Query strings (?query=value)
✅ Fragments (#fragment)
✅ Combined query and fragment
✅ Unicode paths (测试.js)
✅ Percent-encoded special characters
✅ Directory paths resolving to index.js
✅ Invalid/malformed URLs (error cases)
✅ Both sync and async resolution

Technical Details

Previously: file:// protocol was only supported on Windows (#[cfg(target_os = "windows")])

Now: Supported on all platforms with filesystems using conditional compilation:

cfg_if::cfg_if! {
    if #[cfg(not(target_arch = "wasm32"))] {
        let specifier = resolve_file_protocol(specifier)?;
        let specifier = specifier.as_ref();
    }
};

The url crate's to_file_path() method works on all non-wasm platforms, so we removed the Windows-only restriction.

Test Plan

All tests pass on macOS/Linux/Windows:

just test

Verify wasm32 build:

cargo check --all-features --target wasm32-unknown-unknown

Verify no clippy warnings:

cargo clippy --all-features

Add comprehensive tests for ESM file:// protocol URL resolution in the NAPI bindings. Tests cover: - Absolute file:// URLs - Query strings and fragments - Unicode paths - Percent-encoded characters - Directory paths - Invalid/malformed URLs - Both sync and async resolution All tests assert consistent behavior across platforms. Platform-specific failures will be addressed in follow-up work. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>

graphite-app · 2025-10-03T14:01:56Z

How to use the Graphite Merge Queue

Add the label merge to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

_{An organization admin has enabled the Graphite Merge Queue in this repository.} _{Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.}

codspeed-hq · 2025-10-03T14:04:19Z

CodSpeed Performance Report

Merging #746 will not alter performance

_{Comparing esm-file-protocol-tests (48d28e1) with main (a1122a7)}

Summary

✅ 7 untouched
⏩ 2 skipped¹

2 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

napi/tests/file-protocol.test.mjs

Enable ESM file:// protocol URL resolution on macOS and Linux, not just Windows. Changes: - Move `url` crate dependency from Windows-only to all platforms in Cargo.toml - Remove `#[cfg(target_os = "windows")]` from `resolve_file_protocol` function - Remove platform-specific `cfg_if` wrapper at call site - Preserve URL query strings and fragments when converting file:// URLs to paths All file-protocol tests now pass on macOS/Linux. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>

codecov · 2025-10-04T01:28:45Z

Codecov Report

❌ Patch coverage is 0% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 95.09%. Comparing base (870f67e) to head (48d28e1).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
src/lib.rs	0.00%	16 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #746      +/-   ##
==========================================
- Coverage   95.62%   95.09%   -0.54%     
==========================================
  Files          16       16              
  Lines        2951     2974      +23     
==========================================
+ Hits         2822     2828       +6     
- Misses        129      146      +17

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Add conditional compilation to support wasm32 targets where `to_file_path()` is not available. On wasm32, the file:// protocol resolver is a no-op that returns the specifier unchanged, since WebAssembly has no filesystem access. This fixes the build error: ``` error[E0599]: no method named `to_file_path` found for struct `Url` ``` 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>

Refactor to use cfg_if::cfg_if! macro at the call site instead of having two separate function definitions. This matches the original pattern and is more consistent with the rest of the codebase. Changes: - Wrap resolve_file_protocol call in cfg_if block - Only compile/call on non-wasm32 targets - Remove wasm32 stub function (no longer needed) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>

Fix clippy warnings: - Remove unnecessary reference in url::Url::parse call - Use () instead of _ for explicit unit type in map_err 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>

Skip all file:// protocol tests when running with WASI_TEST=1 since the file:// protocol code is excluded on wasm32 targets (no filesystem access). Uses vitest's `test.skipIf(process.env.WASI_TEST)` to conditionally skip all 9 tests in the file when running on WASI. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>

Refactor file:// protocol tests to use a single `describe.skipIf` block instead of repeating `test.skipIf` on each individual test. This is cleaner and groups all file:// protocol tests together. Changes: - Import `describe` from vitest - Wrap all tests in `describe.skipIf(process.env.WASI_TEST)` block - Simplify test names (remove "file:// protocol" prefix) - Proper indentation for nested tests 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>

JounQin · 2025-10-04T07:12:47Z

napi/tests/file-protocol.test.mjs

+// ESM allows file:// protocol URLs for module specifiers
+// See: https://nodejs.org/api/esm.html#urls
+
+describe.skipIf(process.env.WASI_TEST)('file:// protocol', () => {


Why the tests are skipped for WASI?

[autofix.ci] apply automated fixes

87dfa13

graphite-app bot reviewed Oct 3, 2025

View reviewed changes

napi/tests/file-protocol.test.mjs Outdated Show resolved Hide resolved

Boshen and others added 3 commits October 4, 2025 09:32

Boshen changed the title ~~test: add ESM file:// protocol tests for NAPI~~ feat: add ESM file:// protocol support with comprehensive tests Oct 4, 2025

Boshen and others added 2 commits October 4, 2025 12:02

Boshen merged commit 28b8920 into main Oct 4, 2025
16 checks passed

Boshen deleted the esm-file-protocol-tests branch October 4, 2025 04:16

Boshen mentioned this pull request Oct 3, 2025

chore: release v11.10.0 #743

Merged

JounQin reviewed Oct 4, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

feat: add ESM file:// protocol support with comprehensive tests #746

feat: add ESM file:// protocol support with comprehensive tests #746

Uh oh!

Boshen commented Oct 3, 2025 •

edited

Loading

Uh oh!

graphite-app bot commented Oct 3, 2025

Uh oh!

codspeed-hq bot commented Oct 3, 2025 •

edited

Loading

Uh oh!

Uh oh!

codecov bot commented Oct 4, 2025 •

edited

Loading

Uh oh!

Uh oh!

JounQin Oct 4, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

feat: add ESM file:// protocol support with comprehensive tests #746

feat: add ESM file:// protocol support with comprehensive tests #746

Uh oh!

Conversation

Boshen commented Oct 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Implementation

Testing

Technical Details

Test Plan

Related

Uh oh!

graphite-app bot commented Oct 3, 2025

How to use the Graphite Merge Queue

Uh oh!

codspeed-hq bot commented Oct 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CodSpeed Performance Report

Merging #746 will not alter performance

Summary

Footnotes

Uh oh!

Uh oh!

codecov bot commented Oct 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

JounQin Oct 4, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Boshen commented Oct 3, 2025 •

edited

Loading

codspeed-hq bot commented Oct 3, 2025 •

edited

Loading

codecov bot commented Oct 4, 2025 •

edited

Loading