mirror of
https://github.com/pbakaus/impeccable.git
synced 2026-09-14 23:26:39 +03:00
fix: use fileURLToPath for Windows path resolution (#95)
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 #95
This commit is contained in:
@@ -26,10 +26,11 @@ const IS_BROWSER = typeof window !== 'undefined';
|
||||
const IS_NODE = !IS_BROWSER;
|
||||
|
||||
// @browser-strip-start
|
||||
let fs, path;
|
||||
let fs, path, fileURLToPath;
|
||||
if (!IS_BROWSER) {
|
||||
fs = (await import('node:fs')).default;
|
||||
path = (await import('node:path')).default;
|
||||
fileURLToPath = (await import('node:url')).fileURLToPath;
|
||||
}
|
||||
// @browser-strip-end
|
||||
|
||||
@@ -2697,7 +2698,7 @@ async function detectUrl(url) {
|
||||
|
||||
// Read the browser detection script — reuse it instead of reimplementing
|
||||
const browserScriptPath = path.resolve(
|
||||
path.dirname(new URL(import.meta.url).pathname),
|
||||
path.dirname(fileURLToPath(import.meta.url)),
|
||||
'detect-antipatterns-browser.js'
|
||||
);
|
||||
let browserScript;
|
||||
|
||||
@@ -0,0 +1,73 @@
|
||||
import { describe, test, expect } from 'bun:test';
|
||||
import path from 'path';
|
||||
import { fileURLToPath } from 'node:url';
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Regression: Windows drive-letter doubling (#95)
|
||||
//
|
||||
// On Windows, `new URL(import.meta.url).pathname` returns `/C:/foo/bar`
|
||||
// (leading slash). Passing that to `path.resolve()` or `path.join()` on
|
||||
// Windows produces a doubled drive letter like `C:\C:\...`. The canonical
|
||||
// fix is to use `fileURLToPath()` from `node:url`, which strips the leading
|
||||
// slash on Windows.
|
||||
//
|
||||
// These tests verify the contract that `fileURLToPath` behaves correctly on
|
||||
// both POSIX and Windows-style file URLs, and that the source no longer uses
|
||||
// the raw `.pathname` accessor for local path construction.
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe('Windows path doubling fix (#95)', () => {
|
||||
test('fileURLToPath strips leading slash from Windows file URLs', () => {
|
||||
// Simulates the exact scenario: file:///C:/Users/foo/detect-antipatterns.mjs
|
||||
const winUrl = new URL('file:///C:/Users/foo/src/detect-antipatterns.mjs');
|
||||
|
||||
// Raw .pathname returns '/C:/Users/foo/src/detect-antipatterns.mjs'
|
||||
expect(winUrl.pathname).toBe('/C:/Users/foo/src/detect-antipatterns.mjs');
|
||||
|
||||
// fileURLToPath returns 'C:\\Users\\...' on Windows or '/C:/Users/...' on POSIX,
|
||||
// but crucially never returns '/C:/...' on Windows (which causes the double-drive bug)
|
||||
const resolved = fileURLToPath(winUrl);
|
||||
|
||||
// The resolved path should NOT start with /C: on either platform when joined
|
||||
// On POSIX, fileURLToPath('file:///C:/...') returns '/C:/...' which is fine
|
||||
// because POSIX doesn't have drive letters.
|
||||
// The key assertion: path.resolve won't produce a doubled drive letter
|
||||
const dirPart = path.dirname(resolved);
|
||||
const joined = path.resolve(dirPart, 'detect-antipatterns-browser.js');
|
||||
// Should never contain doubled drive pattern like C:\C:\ or /C:/C:/
|
||||
expect(joined).not.toMatch(/[A-Z]:[/\\][A-Z]:/i);
|
||||
});
|
||||
|
||||
test('fileURLToPath handles POSIX file URLs correctly', () => {
|
||||
const posixUrl = new URL('file:///home/user/src/detect-antipatterns.mjs');
|
||||
const resolved = fileURLToPath(posixUrl);
|
||||
expect(resolved).toBe('/home/user/src/detect-antipatterns.mjs');
|
||||
});
|
||||
|
||||
test('import.meta.url produces a valid file URL', () => {
|
||||
// Ensure import.meta.url is a file:// URL that fileURLToPath can handle
|
||||
expect(import.meta.url).toMatch(/^file:\/\//);
|
||||
const thisFile = fileURLToPath(import.meta.url);
|
||||
expect(thisFile).toContain('windows-path-fix.test');
|
||||
});
|
||||
|
||||
test('source file no longer uses raw .pathname for path construction', () => {
|
||||
const fs = require('fs');
|
||||
const src = fs.readFileSync(
|
||||
path.join(__dirname, '..', 'src', 'detect-antipatterns.mjs'),
|
||||
'utf-8'
|
||||
);
|
||||
|
||||
// The bug pattern: using new URL(import.meta.url).pathname in path.resolve/join
|
||||
// After the fix, all occurrences should use fileURLToPath instead
|
||||
const pathnameBugPattern = /path\.(resolve|join|dirname)\(\s*new URL\(import\.meta\.url\)\.pathname/g;
|
||||
const matches = src.match(pathnameBugPattern);
|
||||
expect(matches).toBeNull();
|
||||
|
||||
// Verify fileURLToPath is imported
|
||||
expect(src).toContain('fileURLToPath');
|
||||
|
||||
// Verify fileURLToPath is used with import.meta.url
|
||||
expect(src).toMatch(/fileURLToPath\(import\.meta\.url\)/);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user