Skip to content

fix: disable cache for synthetic dev/preview commands#588

Merged
branchseer merged 5 commits intomainfrom
02-10-fix_disable_cache_for_synthetic_dev_preview_install
Feb 11, 2026
Merged

fix: disable cache for synthetic dev/preview commands#588
branchseer merged 5 commits intomainfrom
02-10-fix_disable_cache_for_synthetic_dev_preview_install

Conversation

@branchseer
Copy link
Copy Markdown
Member

@branchseer branchseer commented Feb 10, 2026

Update SyntheticPlanRequest to use new cache_config field from vite-task.
Synthetic dev and preview commands now explicitly use
UserCacheConfig::disabled() so they are never cached, even when
cacheScripts is enabled.

  • Bump vite-task to 9338d15d (adds cache_config to SyntheticPlanRequest)
  • Use UserCacheConfig::with_config/disabled helpers for all subcommands
  • Add rustc-hash dep for FxHashMap compatibility with vite-task
  • Add snap tests validating cache disabled behavior for synthetic commands

Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com

Copy link
Copy Markdown
Member Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@branchseer branchseer changed the title fix: disable cache for synthetic dev/preview/install commands fix: disable cache for synthetic dev/preview commands Feb 10, 2026
@branchseer branchseer marked this pull request as ready for review February 11, 2026 00:25
@branchseer branchseer requested a review from fengmk2 February 11, 2026 00:25
@branchseer branchseer force-pushed the 02-10-fix_disable_cache_for_synthetic_dev_preview_install branch from 33afac9 to b46b754 Compare February 11, 2026 01:11
branchseer and others added 5 commits February 11, 2026 01:33
Update SyntheticPlanRequest to use new cache_config field from vite-task.
Synthetic dev, preview, and install commands now explicitly use
UserCacheConfig::disabled() so they are never cached, even when
cacheScripts is enabled.

- Bump vite-task to 9338d15d (adds cache_config to SyntheticPlanRequest)
- Use UserCacheConfig::with_config/disabled helpers for all subcommands
- Add rustc-hash dep for FxHashMap compatibility with vite-task
- Add snap tests validating cache disabled behavior for synthetic commands

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ting tests

- SynthesizableSubcommand::Install and auto-install both use enabled cache
- Add cacheScripts: true to existing snap test fixtures so synthetic commands
  within scripts retain cache behavior
- Update plain-terminal-ui-nested snap for new vite.config.ts file count

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@branchseer branchseer force-pushed the 02-10-fix_disable_cache_for_synthetic_dev_preview_install branch from b46b754 to 3889fa5 Compare February 11, 2026 01:33
Copy link
Copy Markdown
Member Author

branchseer commented Feb 11, 2026

Merge activity

  • Feb 11, 1:33 AM UTC: Graphite rebased this pull request as part of a merge.
  • Feb 11, 1:48 AM UTC: @branchseer merged this pull request with Graphite.

@branchseer branchseer merged commit dca0482 into main Feb 11, 2026
17 checks passed
@branchseer branchseer deleted the 02-10-fix_disable_cache_for_synthetic_dev_preview_install branch February 11, 2026 01:48
branchseer added a commit that referenced this pull request Feb 16, 2026
## Summary

- Pin the rolldown binding version in `download-rolldown-binaries` action to match the checked-out source, instead of fetching the latest from npm
- Remove the `VITE_TEST_BUILD=1 vp run tests-e2e#test` line from VitePress E2E (fix in follow-up #599)

## Background on the VitePress `VITE_TEST_BUILD=1` failure

There is a bug in vite's `ModuleRunner.directRequest()` where `dynamicRequest` uses the raw `url` parameter for relative path resolution (`posixResolve(posixDirname(url), dep)`). When a module is loaded via a `file://` URL (e.g. VitePress's `import(pathToFileURL(entryPath).href)`), `pathe.resolve` doesn't recognize `file://` as absolute, producing malformed paths like `<cwd>/file:<path>`.

### Why it doesn't happen with vanilla vitest

With vanilla vitest, VitePress is a regular npm dependency living in `node_modules/`. Vitest's externalization check (`id.includes('/node_modules/')`) returns `true`, so VitePress is **externalized** — loaded via native Node.js `import()` which handles `file://` URLs correctly. The module runner's `dynamicRequest` is never involved.

In vite-plus's ecosystem-ci, VitePress is a `workspace:*` dependency. Its files aren't in `node_modules/`, so it's **inlined** (SSR-transformed). Its `import(pathToFileURL(...))` becomes `__vite_ssr_dynamic_import__('file://...')`, which flows through `dynamicRequest` and hits the bug.

### Why it didn't fail before #588

Before #588, `vp test` commands had caching enabled. When CI ran `vp run tests-e2e#test` (without `VITE_TEST_BUILD`), it succeeded and created a cache entry. The next step `VITE_TEST_BUILD=1 vp run tests-e2e#test` ignored `VITE_TEST_BUILD` since it wasn't in the fingerprinted envs, hit the cache, and `vp test` never actually ran with `VITE_TEST_BUILD=1`. #588 disabled caching on `vp test` commands, so the second step actually ran for the first time and exposed the bug.

The fix is in follow-up #599.
branchseer added a commit that referenced this pull request Feb 16, 2026
…599)

## Summary

- Fix a bug in vite's `ModuleRunner.directRequest()` where `dynamicRequest` uses the raw URL for relative path resolution, which breaks for `file://` URLs
- Re-enable the `VITE_TEST_BUILD=1 vp run tests-e2e#test` VitePress E2E test in CI

Stacked on #597.

## Root Cause

In `ModuleRunner.directRequest()` (`runner.ts:347-352`), the `dynamicRequest` closure resolves relative deps with:

```typescript
dep = posixResolve(posixDirname(url), dep)
```

The `url` captured here is the **raw, unnormalized URL** that flows through `import()` → `cachedRequest()` → `directRequest()`. But the server already resolved and normalized this URL — the normalized version is stored in `mod.url` (e.g. `/@fs/path/.temp/app.js`). The code uses the wrong one.

When `url` is a `file://` URL (e.g. `file:///path/.temp/app.js` from VitePress's `import(pathToFileURL(entryPath).href)`), `pathe.resolve` doesn't recognize it as absolute (it starts with `f`, not `/`), so it prepends CWD, producing malformed paths like `<cwd>/file:/path/.temp/page.md.js`.

### Why it doesn't happen with vanilla vitest

With vanilla vitest, VitePress is a regular npm dependency living in `node_modules/`. Vitest's externalization check (`id.includes('/node_modules/')`) returns `true`, so VitePress is **externalized** — loaded via native Node.js `import()` which handles `file://` URLs correctly. The module runner's `dynamicRequest` is never involved.

In vite-plus's ecosystem-ci, VitePress is a `workspace:*` dependency. Its files aren't in `node_modules/`, so it's **inlined** (SSR-transformed). Its `import(pathToFileURL(...))` becomes `__vite_ssr_dynamic_import__('file://...')`, which flows through `dynamicRequest` and hits the bug.

### Why it didn't fail before #588

Before #588, `vp test` commands had caching enabled. When CI ran `vp run tests-e2e#test` (without `VITE_TEST_BUILD`), it succeeded and created a cache entry. The next step `VITE_TEST_BUILD=1 vp run tests-e2e#test` ignored `VITE_TEST_BUILD` since it wasn't in the fingerprinted envs, hit the cache, and `vp test` never actually ran with `VITE_TEST_BUILD=1`. #588 disabled caching on `vp test` commands, so the second step actually ran for the first time and exposed the bug.

## Fix

Use `mod.url` (the server-normalized URL) instead of the raw `url` parameter in the `dynamicRequest` closure:

```typescript
// Before (broken for file:// URLs):
dep = posixResolve(posixDirname(url), dep)

// After (uses server-normalized URL):
dep = posixResolve(posixDirname(mod.url), dep)
```

`mod.url` is always a properly normalized URL from the server (e.g. `/@fs/...` or `/src/...`) that `posixResolve` handles correctly. Applied as a build-time transform in `packages/core/build.ts`.
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.

2 participants