Skip to content

fix: normalize CLI paths to POSIX so sourcemap sources[] is portable on Windows#20

Open
YevheniiKotyrlo wants to merge 1 commit into
jbroma:mainfrom
YevheniiKotyrlo:fix/cli-windows-path-resolution
Open

fix: normalize CLI paths to POSIX so sourcemap sources[] is portable on Windows#20
YevheniiKotyrlo wants to merge 1 commit into
jbroma:mainfrom
YevheniiKotyrlo:fix/cli-windows-path-resolution

Conversation

@YevheniiKotyrlo

@YevheniiKotyrlo YevheniiKotyrlo commented May 9, 2026

Copy link
Copy Markdown

Summary

Fixes #19. Normalizes node:path.resolve results to POSIX separators so:

  1. The CLI's inputFile (which lands as sources[0] in emitted source maps) is portable per the sourcemap spec.
  2. The 6 vitest CLI runner tests pass on Windows (were 1/7 before; now 7/7).
  3. The 8 example vitest configs discover their test files on Windows (were "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 inputFile flows into:

// packages/core/src/cli/run.ts
return {
  request: {
    filename: command.inputFile,  // ← backslash on Windows pre-fix
    source,
    sourcemap: options.sourcemap,
  },
};

That filename becomes sources[0] in the emitted source map JSON. The sourcemap spec requires forward-slash URI-style paths; Chrome DevTools, Node's source-map consumer, 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

File Change
packages/core/src/cli/args.ts Add a toPosix helper; wrap the 4 resolve(cwd, …) sites that mint inputFile/outputFile/inputSourceMapFile/sourceMapFile.
packages/core/src/cli/__tests__/run.test.ts Replace 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.
packages/core/__tests__/correctness.test.ts Wrap outputsDir() / inputsDir() in toPosix.
examples/{esbuild,parcel,rolldown,rollup,rsbuild,rspack,vite,webpack}/vitest.config.ts Inline .replaceAll('\\', '/') after resolve(import.meta.dirname, …) so tinyglobby (vitest's include matcher) actually discovers the test file on Windows.

toPosix is a one-line sep === '\\' ? 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.ts7/7 pass (was 1/7 before)
  • bunx vitest run __tests__/correctness.test.ts9/9 pass (still passing)
  • bunx vitest run (full packages/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 requires bun run build on 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 formatted
  • cargo 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.

Check Pre-existing failure on Windows host Why
pnpm lint 2 errors in bench/fixtures/source.flow{,.preserve}.js Those files are git symlinks; on Windows checkout they resolve as plain text containing the target path; oxlint parses the path string as JS syntax
pnpm codegen:rust:check spawnSync c++ ENOENT scripts/rust-bindings/lib.mts:83 calls c++ (Unix compiler shorthand). MSVC uses cl.exe; Clang on Windows uses clang++
pnpm build / pnpm test / pnpm e2e turbo + napi-rs cmake-generator mismatch — Visual Studio 16 2019 cache vs Visual Studio 17 2022 env var turbo doesn't propagate CMAKE_GENERATOR to child processes; cmake-rs falls back to oldest-installed VS
cargo test --release --workspace hparser::tests::test1 panics: "All source locations from Hermes parser must be valid" The MSVC EBO bug being fixed in #18. Unrelated to this PR's CLI path scope

Pre-existing lint errors on bench/fixtures/source.flow{,.preserve}.js are unrelated — they're git symlinks resolved to plain text on Windows checkout. Out of scope for this PR.

Notes

  • The toPosix helper is duplicated in three files (args.ts, correctness.test.ts, and inlined in 8 vitest configs). Could be hoisted to a shared utils/ module, but each instance is one line and there's no existing utils/ shape — leaving inline keeps the diff minimal.
  • The examples/*/vitest.config.ts files 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.
  • This is a Windows-only behavior change. On POSIX, sep === '/' so toPosix is the identity function; the resolved path was already forward-slash. Zero impact on Linux or macOS.

…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.
@github-actions github-actions Bot added the release: patch Patch release impact label May 9, 2026
@YevheniiKotyrlo YevheniiKotyrlo changed the title fix(cli): normalize CLI paths to POSIX so sourcemap sources[] is portable on Windows fix: normalize CLI paths to POSIX so sourcemap sources[] is portable on Windows May 9, 2026
@github-actions github-actions Bot added the type: fix Bug fix pull request label May 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release: patch Patch release impact type: fix Bug fix pull request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(cli): CLI emits backslash paths in source maps on Windows; 6 vitest CLI tests fail

1 participant