fix: CLI tests fail on Windows due to path separator differences #1333
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes CLI tests failing on Windows by normalizing path separators from backslashes to forward slashes for cross-platform compatibility.
- Normalizes path separators in
buckets.tsto return forward slashes in placeholdered paths - Normalizes path separators in
find-locale-paths.tspatterns - Adds changeset documenting the fix
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/cli/src/cli/utils/buckets.ts | Converts backslashes to forward slashes in returned placeholdered paths |
| packages/cli/src/cli/utils/find-locale-paths.ts | Normalizes path separators in pattern strings |
| .changeset/fix-windows-path-separators.md | Documents the fix as a patch release |
Comments suppressed due to low confidence (1)
packages/cli/src/cli/utils/buckets.ts:154
- Converting forward slashes to
path.sepat line 153, then later converting back to forward slashes at line 179 creates unnecessary back-and-forth conversions. This approach conflicts with the stated goal of normalizing to forward slashes throughout. Consider keeping forward slashes consistently or refactoring the path handling logic.
const normalizedSourcePath = normalizePath(
sourcePath.replace(/\//g, path.sep),
);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const placeholderedPath = sourcePathChunks.join(path.sep); | ||
| return placeholderedPath; | ||
| }); | ||
| // return the placeholdered paths | ||
| return placeholderedPaths; | ||
| // return the placeholdered paths (normalized to forward slashes for cross-platform compatibility) | ||
| return placeholderedPaths.map(p => p.replace(/\\/g, "/")); |
There was a problem hiding this comment.
The normalization is performed after constructing paths with path.sep, which creates platform-specific separators only to immediately replace them. Consider normalizing earlier at line 175 by using sourcePathChunks.join('/') directly instead of path.sep, which would be more efficient and avoid the redundant mapping step.
| // return the placeholdered paths | ||
| return placeholderedPaths; | ||
| // return the placeholdered paths (normalized to forward slashes for cross-platform compatibility) | ||
| return placeholderedPaths.map(p => p.replace(/\\/g, "/")); |
There was a problem hiding this comment.
[nitpick] Arrow function parameter lacks proper spacing around the arrow. Should be p => p.replace (with space before arrow) for consistency with project style. This is already done correctly in the codebase elsewhere.
2 participants
Fixes #1280
Summary
Normalizes path separators to forward slashes in test output for cross-platform compatibility.
Changes
buckets.tsto normalize backslashes to forward slashes in returned pathsfind-locale-paths.tsto normalize path separators in patternsTesting
All 554 tests pass on Windows 11 (Node v22.19.0, pnpm 9.12.3):