Skip to content

DIAG (do not merge): bypass loadViteBuildOutDir to isolate #703 CI failure#710

Closed
YevheniiKotyrlo wants to merge 2 commits into
onejs:mainfrom
YevheniiKotyrlo:diag/703-skip-load-vite-config
Closed

DIAG (do not merge): bypass loadViteBuildOutDir to isolate #703 CI failure#710
YevheniiKotyrlo wants to merge 2 commits into
onejs:mainfrom
YevheniiKotyrlo:diag/703-skip-load-vite-config

Conversation

@YevheniiKotyrlo

Copy link
Copy Markdown
Contributor

Warning

Diagnostic-only draft PR — do not merge. Will close as soon as CI tells me what I need.

Companion to #703. The serve outDir fix in #703 has been failing the spa-shell-routing test in 4 of 5 stable CI runs (audited the history during today's v1.17.6 refresh). My first guess was that loadConfigFromFile was leaking plugin globals into the subsequent vxrn/serve startup; I shipped an isolation fix (save/clear/restore IS_VXRN_CLI + 3 globalThis slots) and it didn't change the result. This draft removes the loadViteBuildOutDir call entirely (outDir falls straight through to 'dist' when --outDir is missing and buildInfo.json is absent) to isolate whether the call itself is the cause.

If this draft's tests job is green, then the call is the cause and the fix has to take a different shape (e.g., reading vite.config as text, doing the resolution in a child process, or scoping the feature behind opt-in). If it still fails, the cause is elsewhere — investigating from a wrong premise on #703.

Will close this PR + the diag branch once we have the signal.

`one serve` previously resolved outDir from CLI args only, falling
through to literal 'dist' when no --outDir flag was passed. Projects
that set a custom `build.outDir` in vite.config.ts (e.g. 'build-out')
would build correctly via `one build` — which loads the vite config
and already respects `build.outDir` at cli/build.ts — but `one serve`
would then crash with ENOENT looking for `./dist/buildInfo.json`
instead of `./build-out/buildInfo.json`.

Layered precedence in serve.ts:startWorker to match the build command:
  1. --outDir CLI flag
  2. cwd has buildInfo.json — preserves the "cd into output dir
     then run" UX
  3. vite.config's build.outDir — via new `loadViteBuildOutDir()`
  4. 'dist' fallback

`loadViteBuildOutDir` calls Vite's `loadConfigFromFile` directly
rather than going through `loadUserOneOptions`. The wrapper throws
via `getUserOneOptions` when `globalThis.__oneOptions` is unset,
and the One plugin only sets that on the metro-CLI path
(`if (process.env.IS_VXRN_CLI)`). On the vxrn-as-vite-plugin path
(`bundler: 'vite'` in one()) the throw fires every time, the catch
swallows it, and the fallback to 'dist' happens silently — i.e.,
the wrapper silently does nothing for projects that use Vite-native
bundling.

To call `loadConfigFromFile` from `one serve` safely, the helper
must isolate side effects: `one()` has two branches keyed off
`IS_VXRN_CLI`. The CLI branch (which `one build` enters by setting
that env var) just stashes user options into globals and returns
an empty plugin list; the non-CLI branch pushes `vxrnVitePlugin`
and runs the full Vite-native path — `configResolved` hooks, file
watchers, server middleware. `one serve` does NOT set `IS_VXRN_CLI`,
so a naïve `loadConfigFromFile` call would instantiate the full
plugin chain and leak watchers / globals into the subsequent
`vxrn/serve` startup. Mirror the isolation `loadUserOneOptions`
already uses: save `IS_VXRN_CLI` + the three `globalThis`
slots before the call, clear them, restore in `finally`. A vitest
regression guard locks the contract.

Includes a vitest unit test at `vite/loadConfig.test.ts` with two
fixtures (with-outdir / without-outdir) plus the isolation
regression guard.
@YevheniiKotyrlo

Copy link
Copy Markdown
Contributor Author

Diagnostic complete. Removing the loadViteBuildOutDir call makes the test pass (27/27 in test-spa-shell-routing on this branch). The call is the cause of #703's test failures, even with the IS_VXRN_CLI + globalThis isolation I tried in the prior amend. The remaining test-redirect-flash failure on this run is unrelated CI flakiness (port collision: server bound to 43477 because 43476 was in use). Closing this — back to #703 to redesign the fix without runtime loadConfigFromFile.

@YevheniiKotyrlo YevheniiKotyrlo deleted the diag/703-skip-load-vite-config branch May 19, 2026 14:18
YevheniiKotyrlo added a commit to YevheniiKotyrlo/one that referenced this pull request May 19, 2026
`one serve` previously resolved outDir from CLI args + a cwd-
`buildInfo.json` check only, falling through to literal `'dist'` when
neither was present. Projects that set a custom `build.outDir` in
`vite.config.ts` (e.g. `'build-out'`) would build correctly via
`one build` — which loads the vite config and already respects
`build.outDir` at `cli/build.ts` — but `one serve` would then crash
with `ENOENT` looking for `./dist/buildInfo.json` instead of
`./build-out/buildInfo.json`. The reported issue is onejs#701.

Have `one build` write a tiny pointer file at
`node_modules/.cache/one/build-pointer.json` (`{outDir: "build-out"}`)
right after it writes `buildInfo.json`. `one serve` reads the pointer
when `--outDir` is missing and there's no cwd-`buildInfo.json`. New
resolution order:

  1. `--outDir` CLI flag (highest precedence — unchanged)
  2. cwd has `buildInfo.json` — preserves the "cd into output dir
     then run" UX (unchanged)
  3. `node_modules/.cache/one/build-pointer.json` (new)
  4. `'dist'` fallback (unchanged)

Why a marker file instead of loading vite.config at serve time: an
earlier attempt at this PR called Vite's `loadConfigFromFile` from
inside `serve.ts:startWorker`. That instantiates the One plugin chain,
and `one()` has two branches keyed off `process.env.IS_VXRN_CLI`:

- The CLI branch (which `one build` enters by setting that env var)
  just stashes user options into globals and returns an empty plugin
  list — no side effects.
- The non-CLI branch pushes `vxrnVitePlugin` and runs the full
  Vite-native dev path with `configResolved` hooks, file watchers,
  and server middleware.

`one serve` does not set `IS_VXRN_CLI`, so the auxiliary
`loadConfigFromFile` was instantiating the full Vite-native plugin
chain on every startup. Even with an `IS_VXRN_CLI` + `globalThis`
save/restore that mirrored what `loadUserOneOptions` already does,
side effects from esbuild bundling and Vite's plugin lifecycle leaked
into the subsequent `vxrn/serve` boot — `test-spa-shell-routing`
failed 5 of 5 stable CI runs (specifically `navigate from home to
/dashboard/test-app` timing out waiting for the SPA-routed
`#dashboard-app-page` selector). Diagnostic PR onejs#710 confirmed
this by removing the call entirely: the same test went from 5/5 fails
to 27/27 pass on the same CI infrastructure.

The pointer file avoids the problem at its root: the build already
knows the outDir (it called `loadConfigFromFile` itself), so persist
the answer and let `serve` read it. `node_modules/.cache/one/` is the
standard tool-cache location (vite, esbuild, swc all live under
`.cache/`), is auto-gitignored, and survives across `serve` runs
without modifying the project root. The marker write is wrapped in
try/catch so read-only deploy environments that ship only the output
directory still build cleanly.

Validated on Windows 11 / Bun 1.3.13 / Node 25 against
`onejs/one@v1.17.6`:
- `bun audit --audit-level high --ignore GHSA-3ppc-4f35-3m26`: clean
- `bun run lint` (oxlint, 1403 files): 0 warnings / 0 errors
- `bun run format:check`: 1525 files clean
- `bun run typecheck` (turbo, 36 packages): clean
- `bun run test:one` (vitest): 441 pass / 1 fail / 56 skipped
  (the 1 fail is `sourceInspectorPlugin.test.ts`, the pre-existing
  Windows-only upstream failure that PR onejs#702 fixes; not in
  this PR's scope)

Fixes onejs#701
YevheniiKotyrlo added a commit to YevheniiKotyrlo/one that referenced this pull request May 19, 2026
`one serve` previously resolved outDir from CLI args + a cwd-
`buildInfo.json` check only, falling through to literal `'dist'` when
neither was present. Projects that set a custom `build.outDir` in
`vite.config.ts` (e.g. `'build-out'`) would build correctly via
`one build` — which loads the vite config and already respects
`build.outDir` at `cli/build.ts` — but `one serve` would then crash
with `ENOENT` looking for `./dist/buildInfo.json` instead of
`./build-out/buildInfo.json`. The reported issue is onejs#701.

Have `one build` write a tiny pointer file at
`node_modules/.cache/one/build-pointer.json` (`{outDir: "build-out"}`)
right after it writes `buildInfo.json`. `one serve` reads the pointer
when `--outDir` is missing and there's no cwd-`buildInfo.json`. New
resolution order:

  1. `--outDir` CLI flag (highest precedence — unchanged)
  2. cwd has `buildInfo.json` — preserves the "cd into output dir
     then run" UX (unchanged)
  3. `node_modules/.cache/one/build-pointer.json` (new)
  4. `'dist'` fallback (unchanged)

Why a marker file instead of loading vite.config at serve time: an
earlier attempt at this PR called Vite's `loadConfigFromFile` from
inside `serve.ts:startWorker`. That instantiates the One plugin chain,
and `one()` has two branches keyed off `process.env.IS_VXRN_CLI`:

- The CLI branch (which `one build` enters by setting that env var)
  just stashes user options into globals and returns an empty plugin
  list — no side effects.
- The non-CLI branch pushes `vxrnVitePlugin` and runs the full
  Vite-native dev path with `configResolved` hooks, file watchers,
  and server middleware.

`one serve` does not set `IS_VXRN_CLI`, so the auxiliary
`loadConfigFromFile` was instantiating the full Vite-native plugin
chain on every startup. Even with an `IS_VXRN_CLI` + `globalThis`
save/restore that mirrored what `loadUserOneOptions` already does,
side effects from esbuild bundling and Vite's plugin lifecycle leaked
into the subsequent `vxrn/serve` boot — `test-spa-shell-routing`
failed 5 of 5 stable CI runs (specifically `navigate from home to
/dashboard/test-app` timing out waiting for the SPA-routed
`#dashboard-app-page` selector). Diagnostic PR onejs#710 confirmed
this by removing the call entirely: the same test went from 5/5 fails
to 27/27 pass on the same CI infrastructure.

The pointer file avoids the problem at its root: the build already
knows the outDir (it called `loadConfigFromFile` itself), so persist
the answer and let `serve` read it. `node_modules/.cache/one/` is the
standard tool-cache location (vite, esbuild, swc all live under
`.cache/`), is auto-gitignored, and survives across `serve` runs
without modifying the project root. The marker write is wrapped in
try/catch so read-only deploy environments that ship only the output
directory still build cleanly.

Validated on Windows 11 / Bun 1.3.13 / Node 25 against
`onejs/one@v1.17.6`:
- `bun audit --audit-level high --ignore GHSA-3ppc-4f35-3m26`: clean
- `bun run lint` (oxlint, 1403 files): 0 warnings / 0 errors
- `bun run format:check`: 1525 files clean
- `bun run typecheck` (turbo, 36 packages): clean
- `bun run test:one` (vitest): 441 pass / 1 fail / 56 skipped
  (the 1 fail is `sourceInspectorPlugin.test.ts`, the pre-existing
  Windows-only upstream failure that PR onejs#702 fixes; not in
  this PR's scope)

Fixes onejs#701
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant