fix(path): improve Node.js compatibility, run tests in node and bun#7104
fix(path): improve Node.js compatibility, run tests in node and bun#7104kt3k wants to merge 1 commit into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7104 +/- ##
=======================================
Coverage 94.60% 94.61%
=======================================
Files 633 633
Lines 51777 51777
Branches 9324 9324
=======================================
+ Hits 48986 48987 +1
Misses 2216 2216
+ Partials 575 574 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| const url = new URL("file:///"); | ||
| url.pathname = encodeWhitespace( | ||
| path.replace(/%/g, "%25").replace(/\\/g, "%5C"), | ||
| path.replace(/^\/+/, "/").replace(/%/g, "%25").replace(/\\/g, "%5C"), |
There was a problem hiding this comment.
The below works differently in Deno and Node:
const u = new URL("file:///");
u.pathname = "//foo";
console.log(u.href);
// prints file:///foo in Deno
// prints file:////foo in Node, Bun, and browsersThis change normalizes the differences above (and therefore no breaking changes to deno users)
There was a problem hiding this comment.
Created an issue in rust-url about the above behavior servo/rust-url#1118
lunadogbot
left a comment
There was a problem hiding this comment.
path/posix/to_file_url.ts: collapsing leading /+ before assigning url.pathname is the right fix — Deno already produced file:///foo from pathname = "//foo", this aligns Node/Bun to that. Existing posix.toFileUrl("//localhost/...") cases in to_file_url_test.ts cover it. The is_glob worker rewrite is also an improvement — it now assertEquals the three results, where the data-URL version only asserted that postMessage ran.
- nit: title is
test(path)but theto_file_url.tschange is a runtime behaviour tweak, not test-only.fix(path)would be more accurate.
|
@bartlomieju this is ready to merge |
updated the title following the above message |
This PR adds tests of
pathin Node and Bunref #7103