Skip to content

Commit c2d877e

Browse files
chore: tighten path-fix comments — one-line WHYs, no line-number rot
Following an audit pass: comments narrating WHAT (`posix so forward-slash on Windows`) deleted; multi-line blocks compressed to one-liners; rot phrases removed (`this PR`, hardcoded `build.ts:1031`, "during initial review"); JSDoc shrunk; backslash-acceptance rationale + Windows-specific specifier surprise preserved as one-liners. No behavior change. 421/477 vitest, 33/33 vxrn, 9/9 resolve, lint 0/0.
1 parent e0cad28 commit c2d877e

10 files changed

Lines changed: 14 additions & 46 deletions

File tree

packages/compiler/src/transformSWC.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,7 @@ import { configuration } from './configure'
1212
import { asyncGeneratorRegex, debug, parsers, runtimePublicPath } from './constants'
1313
import type { Options } from './types'
1414

15-
// POSIX-only sentinel — Vite plugin context hands us forward-slash ids; the
16-
// other caller (patches.ts) hands native paths, so we normalize id below.
15+
// POSIX-only — id is normalized below
1716
const ignoreId = /node_modules\/(\.vite|vite)\//
1817

1918
export async function transformSWC(

packages/one/src/cli/build.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -547,7 +547,6 @@ export async function build(args: {
547547
chunkInfo.name,
548548
Path.extname(chunkInfo.name)
549549
)
550-
// posix so nested-chunk fileNames stay forward-slash on Windows
551550
return posix.join(dir, `${name}-[hash].cjs`)
552551
},
553552
assetFileNames: (assetInfo) => {
@@ -602,7 +601,6 @@ export async function build(args: {
602601
const outChunks = middlewareBuildInfo.output.filter((x) => x.type === 'chunk')
603602
const chunk = outChunks.find((x) => x.facadeModuleId === fullPath)
604603
if (!chunk) throw new Error(`internal err finding middleware`)
605-
// posix so manifest entry stays forward-slash on Windows
606604
builtMiddlewares[middleware.file] = posix.join(
607605
outDir,
608606
'middlewares',

packages/one/src/metro-config/getViteMetroPluginOptions.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -233,8 +233,7 @@ export function getViteMetroPluginOptions({
233233
[
234234
'one/babel-plugin-one-router-metro',
235235
{
236-
// both values are inlined by babel into metro-entry.js as JS module
237-
// specifiers — Metro's isRelativeImport regex rejects `..\foo` on Windows
236+
// inlined as JS module specifiers; Metro's isRelativeImport rejects `..\foo` on Windows
238237
ONE_ROUTER_APP_ROOT_RELATIVE_TO_ENTRY: normalizePath(
239238
path.relative(
240239
path.dirname(metroEntryPath),

packages/one/src/utils/posixPathContract.test.ts

Lines changed: 5 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,4 @@
1-
// POSIX path contract tests — locks in the cross-platform behavior of the
2-
// Node primitives this PR's fixes depend on. If a future Node version
3-
// changes any of these, the failing test surfaces the regression before
4-
// the framework breaks on Windows again.
5-
1+
// Lock in cross-platform output of Node primitives — a regression here is a future Windows break.
62
import { posix } from 'node:path'
73
import { pathToFileURL } from 'node:url'
84
import { describe, expect, it } from 'vitest'
@@ -41,9 +37,7 @@ describe('pathToFileURL — canonical file:// URL specifier', () => {
4137
})
4238

4339
it('round-trips through JSON.stringify without backslashes leaking in', () => {
44-
// The motivating defect: emitting `import "${absolutePath}"` puts
45-
// backslashes into the generated JS source on Windows. A file:// URL
46-
// does not.
40+
// motivating defect: bare absolute path puts backslashes in emitted JS on Windows
4741
const href = pathToFileURL('/proj/src/setup.ts').href
4842
const stringified = JSON.stringify(href)
4943
expect(stringified.includes('\\\\')).toBe(false)
@@ -56,9 +50,7 @@ describe('Vite normalizePath — converts on Windows, no-op on POSIX', () => {
5650
})
5751

5852
it('converts backslashes (Windows-shaped input)', () => {
59-
// normalizePath only converts on Windows hosts. We assert the contract
60-
// shape — on Windows: backslashes converted; on POSIX: passthrough
61-
// because backslash is a valid filename character.
53+
// Windows converts; POSIX passthrough (backslash is a valid filename char there)
6254
const input = String.raw`C:\proj\src\foo.tsx`
6355
const out = normalizePath(input)
6456
if (process.platform === 'win32') {
@@ -70,10 +62,7 @@ describe('Vite normalizePath — converts on Windows, no-op on POSIX', () => {
7062
})
7163

7264
describe('toServerOutputPath — cross-platform parity', () => {
73-
// toServerOutputPath uses path.posix.join + replace(/\\/g, '/') so the
74-
// output must be byte-identical across platforms. The fact that we
75-
// converted from Vite's normalizePath to this implementation was driven
76-
// by a Linux Docker test failure during initial review.
65+
// posix.join + backslash-replace (not normalizePath) — output must be byte-identical across platforms
7766
const cases: Array<[string, string, string]> = [
7867
['foo.js', 'dist', 'dist/server/foo.js'],
7968
['subdir/bar.js', 'dist', 'dist/server/subdir/bar.js'],
@@ -93,11 +82,7 @@ describe('toServerOutputPath — cross-platform parity', () => {
9382
})
9483

9584
describe('seed bug regression — SSR loader path doubling on Windows', () => {
96-
// Direct regression test for the specific shape that hit production:
97-
// build.ts:1031 minted `dist\server\assets\time_ssr-XXX.js` (native sep);
98-
// oneServe.ts:168 then ran `.includes('dist/server')` on it (forward slash);
99-
// the check missed; `join('./','dist/server','dist\\server\\...')` was
100-
// called; result was `dist\server\dist\server\...` — passed to await import().
85+
// serverJsPath minted native sep; oneServe `.includes('/server')` missed; prefix doubled, await import() crashed
10186
it('does not double-prefix backslash-shaped server-output input', () => {
10287
const windowsShapedInput = String.raw`dist\server\assets\time_ssr-COqAsxju.js`
10388
expect(toServerOutputPath(windowsShapedInput, 'dist')).toBe(

packages/one/src/utils/toServerOutputPath.test.ts

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,7 @@ describe('toServerOutputPath', () => {
2222
})
2323

2424
it('treats backslashes as separators on every platform (the Windows path-doubling case)', () => {
25-
// Simulates what `path.join` produces on Windows after the first prefix
26-
// has been added by an earlier caller. The function must convert
27-
// backslashes to forward slashes here regardless of host OS so the
28-
// sentinel check matches and the prefix isn't doubled.
25+
// already-prefixed backslash input — must normalize before sentinel check
2926
expect(toServerOutputPath('dist\\server\\assets\\loader-abc.js', 'dist')).toBe(
3027
'dist/server/assets/loader-abc.js'
3128
)
@@ -44,8 +41,7 @@ describe('toServerOutputPath', () => {
4441
})
4542

4643
it('does not false-positive on inputs that contain ${outDir}/server as a substring but are not rooted under it', () => {
47-
// Without startsWith, `.includes('dist/server')` would match this input
48-
// and skip the prefix — incorrectly treating it as already-prefixed.
44+
// startsWith required: .includes would match foo/dist/server/x and wrongly skip
4945
expect(toServerOutputPath('foo/dist/server/bar.js', 'dist')).toBe(
5046
'dist/server/foo/dist/server/bar.js'
5147
)

packages/one/src/utils/toServerOutputPath.ts

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,6 @@
11
import { posix } from 'node:path'
22

3-
/**
4-
* Idempotent: prepends `${outDir}/server/` to `input` if not already rooted there.
5-
* Output is forward-slash on every platform. Backslash input is accepted (legacy
6-
* callers may hand us `path.join`-shaped strings).
7-
*/
3+
// Idempotent. Returns forward-slash output. Accepts backslash input (legacy path.join-shaped callers).
84
export function toServerOutputPath(input: string, outDir: string): string {
95
const normalized = input.replace(/\\/g, '/')
106
const prefix = `${outDir}/server/`

packages/one/src/vite/one.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -251,9 +251,7 @@ export function one(options: One.PluginOptions = {}): PluginOption {
251251
} catch {}
252252
}
253253

254-
// for bare imports, map real path → node_modules path. resolved.id
255-
// is POSIX (Vite normalizes); realpathSync/join return native on
256-
// Windows, so normalize both sides before the startsWith check.
254+
// resolved.id is POSIX (Vite's getRealPath normalizes); realpathSync returns native — normalize both
257255
const realPkgDir = normalizePath(realpathSync(nmPkgDir))
258256
if (resolved.id.startsWith(realPkgDir)) {
259257
const relativePart = resolved.id.slice(realPkgDir.length)

packages/one/src/vite/plugins/virtualEntryPlugin.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,8 +87,7 @@ function getSetupFileImport(
8787
if (!setupFile)
8888
return { importStatement: '', promiseDeclaration: '', promiseVarName: '' }
8989

90-
// file:// URL is the canonical specifier shape; bare Windows path with
91-
// backslashes is not (mirrors the native sibling in createNativeDevEngine.ts)
90+
// file:// URL is canonical; bare Windows absolute path has backslashes (mirrors createNativeDevEngine.ts)
9291
const resolvedSetupFile = root
9392
? pathToFileURL(resolve(root, setupFile)).href
9493
: setupFile

packages/vxrn/src/utils/createNativeDevEngine.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -559,8 +559,7 @@ const VIRTUAL_NATIVE_ENTRY = 'virtual:native-entry'
559559

560560
function nativeVirtualEntryPlugin(root: string, opts?: { dev?: boolean }): Plugin {
561561
const isDev = opts?.dev !== false
562-
// absolute path so `import.meta.glob('./app/...')` resolves; forward-slash
563-
// for Vite/rolldown module-graph convention
562+
// absolute for import.meta.glob resolution; forward-slash for module-graph convention
564563
const resolvedId = normalizePath(resolve(root, '__virtual-native-entry.tsx'))
565564

566565
// read config passed from One's vite plugin via globalThis

packages/vxrn/src/utils/patches.test.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,7 @@ import { mkdtemp, rm, symlink } from 'node:fs/promises'
44
import { tmpdir } from 'node:os'
55
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
66

7-
// 'junction' avoids the admin requirement for 'dir' symlinks on Windows;
8-
// ignored on POSIX (per Node fs docs)
7+
// 'junction' avoids Windows admin requirement; type ignored on POSIX
98

109
// mock heavy deps that aren't needed for patch resolution tests
1110
vi.mock('@vxrn/compiler', () => ({ transformSWC: vi.fn() }))

0 commit comments

Comments
 (0)