fix: normalize CLI paths to POSIX so sourcemap sources[] is portable on Windows#20
Open
YevheniiKotyrlo wants to merge 1 commit into
Open
Conversation
…able
`node:path.resolve` (and `path.join`) is `path.win32.*` on Windows and
emits backslash paths. Two consequences:
1. The CLI's `inputFile` flows into `fft({ filename, ... })` and lands
as `sources[0]` in the emitted source map. Sourcemap spec says
forward-slash URI-style paths; backslash sources don't resolve in
browsers' dev tools or in Node's source-map consumers on Windows.
2. The 6 vitest unit tests for the CLI runner mock `readFile`/`transform`
with POSIX-style keys (`/repo/src/input.js`); on Windows they receive
`\repo\src\input.js` and never match — all 6 fail.
3. The 8 example vitest configs use `resolve(import.meta.dirname, …)`
for the `include` array. tinyglobby (vitest's include matcher) only
resolves forward-slash patterns, so on Windows every adapter test
silently exits with "No test files found" — esbuild, parcel, rolldown,
rollup, rsbuild, rspack, vite, webpack all uncovered on Windows.
CI is `ubuntu-24.04`-only, so all of this ships invisible.
Single-line fix in each site: `toPosix(resolve(…))` where `toPosix` is
`p => p.replaceAll('\', '/')` on Windows and identity elsewhere. Same
defect class OneJS just standardized away from in onejs/one#702.
Test changes:
- run.test.ts: replaces hard-coded `/repo` with a platform-aware
`FAKE_CWD = process.platform === 'win32' ? 'C:/repo' : '/repo'`. On
Windows `node:path.resolve` interprets a bare `/repo` as drive-relative
and prepends the current drive, so the fixture has to be a fully
qualified absolute path on each platform.
- correctness.test.ts: outputsDir/inputsDir wrapped in toPosix.
Validation on Windows MSVC + Bun 1.3.13 + Node 25:
packages/core run.test.ts 7/7 pass (was 1/7)
packages/core correctness.test.ts 9/9 pass (still passing; toPosix is
a no-op for in-fixture paths
that already resolve to the
same drive on Windows)
packages/core full vitest 39/39 pass (was 33/39, +6 unblocked)
examples/* vitest discovery 8/8 now find their test files (were
"No test files found" on Windows)
test execution itself still requires
`bun run build` on each example first
— that's an existing test-runner
ordering concern, not in this PR's scope
pnpm typecheck 11/11 packages clean
pnpm format:check:js clean
Pre-existing lint failures on `bench/fixtures/source.flow*.js` are
unrelated — they're git symlinks resolved to text on Windows checkout.
Closes jbroma#19.
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.
Summary
Fixes #19. Normalizes
node:path.resolveresults to POSIX separators so:inputFile(which lands assources[0]in emitted source maps) is portable per the sourcemap spec."No test files found"— silently uncovered).Same defect class onejs/one#702 standardized away from. CI is
ubuntu-24.04-only, so all of this ships invisible to Windows users.Why this matters in production
The CLI's
inputFileflows into:That
filenamebecomessources[0]in the emitted source map JSON. The sourcemap spec requires forward-slash URI-style paths; Chrome DevTools, Node'ssource-mapconsumer, and most browser dev tools don't resolve backslash sources on Windows. So pre-fix, every CLI invocation on Windows emits a non-portable sourcemap.Diff shape
packages/core/src/cli/args.tstoPosixhelper; wrap the 4resolve(cwd, …)sites that mintinputFile/outputFile/inputSourceMapFile/sourceMapFile.packages/core/src/cli/__tests__/run.test.ts'/repo'with a platform-awareFAKE_CWD = process.platform === 'win32' ? 'C:/repo' : '/repo'. On Windows,node:path.resolveinterprets a bare/repoas drive-relative and prepends the current drive, so the fixture has to be a fully qualified absolute path on each platform.packages/core/__tests__/correctness.test.tsoutputsDir()/inputsDir()intoPosix.examples/{esbuild,parcel,rolldown,rollup,rsbuild,rspack,vite,webpack}/vitest.config.ts.replaceAll('\\', '/')afterresolve(import.meta.dirname, …)sotinyglobby(vitest's include matcher) actually discovers the test file on Windows.toPosixis a one-linesep === '\\' ? p.replaceAll('\\', '/') : p— no extra dependencies, identity-on-POSIX, so this is purely additive on Linux/macOS.Test plan
Validated on Windows MSVC + Bun 1.3.13 + Node 25.
Passes locally on this branch — relevant to the change:
bunx vitest run src/cli/__tests__/run.test.ts— 7/7 pass (was 1/7 before)bunx vitest run __tests__/correctness.test.ts— 9/9 pass (still passing)bunx vitest run(fullpackages/core) — 39/39 pass (was 33/39 — 6 unblocked, 0 regressed)bunx vitest --config examples/esbuild/vitest.config.ts run— finds the test file (was "No test files found"). Same for the other 7 adapter examples. Test execution itself still requiresbun run buildon each example first; that's an unrelated test-runner ordering concern, not in this PR's scope.pnpm typecheck— 11/11 packages clean (turbo full)pnpm format:check:js— 179 files all formattedcargo fmt --all --check— clean (no Rust changes from this PR)Pre-existing Windows-host limitations — out of scope, would not affect Linux CI:
These were exercised running fft's own CI matrix locally. None are introduced or affected by this PR. Linux CI (the one fft actually runs) wouldn't see these.
pnpm lintbench/fixtures/source.flow{,.preserve}.jspnpm codegen:rust:checkspawnSync c++ ENOENTscripts/rust-bindings/lib.mts:83callsc++(Unix compiler shorthand). MSVC usescl.exe; Clang on Windows usesclang++pnpm build/pnpm test/pnpm e2eVisual Studio 16 2019cache vsVisual Studio 17 2022env varCMAKE_GENERATORto child processes; cmake-rs falls back to oldest-installed VScargo test --release --workspacehparser::tests::test1panics: "All source locations from Hermes parser must be valid"#18. Unrelated to this PR's CLI path scopePre-existing lint errors on
bench/fixtures/source.flow{,.preserve}.jsare unrelated — they're git symlinks resolved to plain text on Windows checkout. Out of scope for this PR.Notes
toPosixhelper is duplicated in three files (args.ts,correctness.test.ts, and inlined in 8 vitest configs). Could be hoisted to a sharedutils/module, but each instance is one line and there's no existingutils/shape — leaving inline keeps the diff minimal.examples/*/vitest.config.tsfiles all share the same shape (only the test file name differs). They could be deduped via a shared helper, but each is a 1-line change to the existing config and the shape stays self-explanatory.sep === '/'sotoPosixis the identity function; the resolved path was already forward-slash. Zero impact on Linux or macOS.