fix: use fileURLToPath for Windows path resolution#101
Open
voidborne-d wants to merge 1 commit intopbakaus:mainfrom
Open
fix: use fileURLToPath for Windows path resolution#101voidborne-d wants to merge 1 commit intopbakaus:mainfrom
voidborne-d wants to merge 1 commit intopbakaus:mainfrom
Conversation
On Windows, `new URL(import.meta.url).pathname` returns `/C:/...` (with a leading slash). Passing that to `path.resolve()` or `path.join()` causes Node to prepend the drive letter again, producing doubled paths like `C:\C:\Users\...\detect-antipatterns-browser.js`. Replace both occurrences (puppeteer scan at ~L2690 and live detect at ~L3506) with `fileURLToPath(import.meta.url)` from `node:url`, which correctly strips the leading slash on Windows while remaining a no-op on POSIX. Add regression tests verifying the source no longer uses the raw `.pathname` accessor for local path construction and that `fileURLToPath` handles both Windows and POSIX file URLs correctly. Closes pbakaus#95
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
npx impeccable detect <url>fails on Windows with:Note the doubled
C:\C:\.Fixes #95.
Root Cause
Two locations in
src/detect-antipatterns.mjsconstruct the path todetect-antipatterns-browser.jsusing:On Windows,
new URL("file:///C:/foo/bar").pathnamereturns/C:/foo/bar(with a leading slash). When passed topath.resolve()orpath.join(), Node prepends the drive letter again, producingC:\C:\....Fix
Replace both occurrences (~L2690 puppeteer scan and ~L3506 live detect) with
fileURLToPath(import.meta.url)fromnode:url, which correctly strips the leading slash on Windows while remaining a no-op on POSIX.The import is added inside the existing
!IS_BROWSERguard alongsidefsandpath, so the browser bundle is unaffected.Testing
Added
tests/windows-path-fix.test.jswith 4 regression tests:fileURLToPath+path.resolvenever producesC:\C:\patternsfileURLToPathreturns correct paths on Linux/macOSfile://URLpath.(resolve|join|dirname)(new URL(import.meta.url).pathnamepatterns in the sourceAll existing tests pass (157 pass, 1 pre-existing unrelated failure in jsdom stylesheet test).