Pin Vite cache to project root and clear on update - #374
Closed
1weiho wants to merge 1 commit into
Closed
Conversation
The dev server sets Vite's `root` inside the installed @open-slide/core package, so Vite's default `cacheDir` landed under node_modules/@open-slide/core/node_modules/.vite. The in-app updater swaps that package directory out on upgrade, leaving the optimizer referencing `.vite/deps` chunks that no longer exist — and since Vite's dep hash can't read bun's text lockfile, a version bump never invalidates the cache, so restarts don't recover. Pin `cacheDir` to <project>/node_modules/.vite and clear it after an in-app update so the restart re-optimizes against the new version. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B9t4YwFJsMczwpVLXf4C12
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughVite’s optimize-deps cache is moved to the user project’s ChangesVite cache lifecycle
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.changeset/vite-cache-dir.md (1)
5-5: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse a concise, user-facing changeset description.
-Store Vite's optimize-deps cache at the project root and clear it on in-app update, so upgrading no longer leaves the dev server failing on missing `.vite/deps` chunks. +In-app updates no longer serve stale Vite dependency chunks.As per coding guidelines, changeset descriptions must be short and direct: one line, present-tense, describing what changed from a user's perspective.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.changeset/vite-cache-dir.md at line 5, Rewrite the changeset description as one concise, direct, present-tense sentence describing the user-facing fix: Vite’s optimize-deps cache is stored at the project root and cleared during in-app updates.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core/src/vite/routes/update.ts`:
- Around line 147-166: Update updatePackage so clearViteCache always runs after
installation by placing the syncCommand execution in a try/finally block, and
normalize ctx.userCwd to the resolved project root used by Vite configuration
resolution before cleanup. Do not silently swallow cache-removal failures:
surface the cleanup error or notify the user that a manual restart is required.
---
Nitpick comments:
In @.changeset/vite-cache-dir.md:
- Line 5: Rewrite the changeset description as one concise, direct,
present-tense sentence describing the user-facing fix: Vite’s optimize-deps
cache is stored at the project root and cleared during in-app updates.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 987d3659-0ee5-4a35-8874-35cb90b5d46f
📒 Files selected for processing (5)
.changeset/vite-cache-dir.mdpackages/core/src/vite/cache-dir.test.tspackages/core/src/vite/cache-dir.tspackages/core/src/vite/config.tspackages/core/src/vite/routes/update.ts
Comment on lines
+147
to
+166
| // Drop Vite's optimize-deps cache so the post-update restart re-bundles against the | ||
| // freshly installed version. Vite keys that cache off a dep hash that doesn't reflect | ||
| // an @open-slide/core version bump (notably it can't read bun's text lockfile), so | ||
| // without this it keeps serving the previous version's deps. Best-effort: on Windows | ||
| // the outgoing server may still hold the files, and a stale cache only costs a manual | ||
| // restart, never correctness. | ||
| async function clearViteCache(cwd: string): Promise<void> { | ||
| try { | ||
| await fs.rm(resolveViteCacheDir(cwd), { recursive: true, force: true }); | ||
| } catch {} | ||
| } | ||
|
|
||
| async function updatePackage(ctx: ApiContext): Promise<UpdateResult> { | ||
| const packageManager = await detectPackageManager(ctx.userCwd); | ||
| const updateCommand = updateCommandFor(packageManager); | ||
| const syncCommand = localOpenSlideCommand(ctx.userCwd); | ||
|
|
||
| await runCommand(updateCommand, ctx.userCwd); | ||
| await runCommand(syncCommand, ctx.userCwd); | ||
| await clearViteCache(ctx.userCwd); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate update.ts =="
fd -a 'update\.ts$' . | sed 's#^\./##'
echo "== file outline =="
ast-grep outline packages/core/src/vite/routes/update.ts --view expanded || true
echo "== relevant lines 1-240 =="
sed -n '1,240p' packages/core/src/vite/routes/update.ts | cat -n
echo "== search vite cache dir / normalize function symbols =="
rg -n "resolveViteCacheDir|createViteConfig|userCwd|clearViteCache|sync:skills|runCommand" packages/core/src packages/core -S
echo "== package manager / changeset context =="
git diff --stat || true
git status --short || true
fd 'package.json' . | sed -n '1,20p'Repository: 1weiho/open-slide
Length of output: 27346
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Behavioral probe: model the update flow control and directory resolution without running repo code.
node - <<'JS'
function currentFlow(updateOk, syncStatus, currentDir) {
const reachedClear = syncStatus === 'ok';
const cleanupTarget = currentDir;
return { reachedClear, cleanupTarget };
}
function proposedFlow(updateOk, syncStatus, currentDir, rootDir) {
try {
if (syncStatus !== 'ok') throw new Error('sync failed');
return { caught: false, reachedClear: true, cleanupTarget: rootDir };
} catch {
return { caught: true, reachedClear: true, cleanupTarget: rootDir };
}
}
const cases = [
{ case: 'sync ok no change', flow: currentFlow(true, 'ok', 'relativeCwd'), expected: { reachedClear: true, cleanupTarget: 'relativeCwd' } },
{ case: 'sync fail no change', flow: currentFlow(true, 'fail', 'relativeCwd'), expected: { reachedClear: false, cleanupTarget: 'relativeCwd' } },
{ case: 'sync fail proposed', flow: proposedFlow(true, 'fail', 'relativeCwd', '/home/project'), expected: { reachedClear: true, cleanupTarget: '/home/project' } },
];
for (const {case: label, flow, expected} of cases) {
const ok = JSON.stringify(flow) === JSON.stringify(expected);
console.log(label, ok ? 'ok' : 'mismatch', JSON.stringify(flow), 'expect', JSON.stringify(expected));
}
JSRepository: 1weiho/open-slide
Length of output: 580
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Behavioral probe: model the update flow control and directory resolution without running repo code.
node - <<'JS'
function currentFlow(updateOk, syncStatus, currentDir) {
const reachedClear = syncStatus === 'ok';
const cleanupTarget = currentDir;
return { reachedClear, cleanupTarget };
}
function proposedFlow(updateOk, syncStatus, currentDir, rootDir) {
try {
if (syncStatus !== 'ok') throw new Error('sync failed');
return { caught: false, reachedClear: true, cleanupTarget: rootDir };
} catch {
return { caught: true, reachedClear: true, cleanupTarget: rootDir };
}
}
const cases = [
{ case: 'sync ok no change', flow: currentFlow(true, 'ok', 'relativeCwd'), expected: { reachedClear: true, cleanupTarget: 'relativeCwd' } },
{ case: 'sync fail no change', flow: currentFlow(true, 'fail', 'relativeCwd'), expected: { reachedClear: false, cleanupTarget: 'relativeCwd' } },
{ case: 'sync fail proposed', flow: proposedFlow(true, 'fail', 'relativeCwd', '/home/project'), expected: { caught: true, reachedClear: true, cleanupTarget: '/home/project' } },
];
for (const {case: label, flow, expected} of cases) {
const ok = JSON.stringify(flow) === JSON.stringify(expected);
console.log(label, ok ? 'ok' : 'mismatch', JSON.stringify(flow), 'expect', JSON.stringify(expected));
}
JSRepository: 1weiho/open-slide
Length of output: 588
Make cache invalidation unconditional after installation.
If sync:skills fails after the package install succeeds, execution never reaches clearViteCache(ctx.userCwd), leaving the new core package paired with stale optimized dependencies. Run cache cleanup from a finally block and normalize ctx.userCwd using the same resolved project root used by Vite config resolution; surface cleanup failures or inform the user that a manual restart is required.
stability_and_availability
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawn } from 'node:child_process';
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/core/src/vite/routes/update.ts` around lines 147 - 166, Update
updatePackage so clearViteCache always runs after installation by placing the
syncCommand execution in a try/finally block, and normalize ctx.userCwd to the
resolved project root used by Vite configuration resolution before cleanup. Do
not silently swallow cache-removal failures: surface the cleanup error or notify
the user that a manual restart is required.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes an issue where upgrading
@open-slide/corevia the in-app updater leaves the dev server failing on missing.vite/depschunks.Problem: Vite's optimize-deps cache defaults to
<nearest package.json>/node_modules/.vite. Since the framework's Vite config hasrootpointing inside the installed@open-slide/corepackage, the cache lands undernode_modules/@open-slide/core/node_modules/.vite— inside the directory that gets swapped out during upgrade. This leaves Vite referencing stale chunks that no longer exist.Solution:
resolveViteCacheDir()utility to pin the cache to the user's project root (<userCwd>/node_modules/.vite)cacheDiroption to use this pathChanges:
packages/core/src/vite/cache-dir.tswithresolveViteCacheDir()function and testpackages/core/src/vite/config.tsto setcacheDirin the Vite configpackages/core/src/vite/routes/update.tsto callclearViteCache()after package updatehttps://claude.ai/code/session_01B9t4YwFJsMczwpVLXf4C12
Summary by CodeRabbit
Bug Fixes
Maintenance