fix: gate fresh:reload on SSR-relevant changes (plugin-vite)#3809
fix: gate fresh:reload on SSR-relevant changes (plugin-vite)#3809fibibot wants to merge 2 commits into
Conversation
The chokidar watcher attached by `serverSnapshot` was calling
`viteServer.ws.send("fresh:reload")` on every file change, irrespective
of whether the changed file had anything to do with SSR. Because Fresh
attaches directly to Vite's chokidar instance, the user's
`server.watch.ignored` config does not filter events before Fresh sees
them, so on Windows journal/temp writes (Deno KV `main-shm`/`main-wal`,
`*.tmp.*`, `.timestamp-*`) were triggering a full page reload on every
KV write.
Only emit `fresh:reload` when the changed path is in the SSR module
graph or is a route file under `routeDir`. Island and other client-graph
edits keep flowing through Vite's normal HMR path, and route add/unlink
still invalidates the snapshot.
Closes #3637
fibibot
changed the title
|
Quick note on the failing |
bartlomieju
left a comment
bartlomieju
left a comment
There was a problem hiding this comment.
Diagnosis is correct, fix is well-scoped, test is solid. The watcher callback at server_snapshot.ts:76 was indeed sending fresh:reload for every chokidar event after the island early-return — gating on SSR-relevant changes is the right move, and the legitimate SSR-modification path through dev_server.ts's hotUpdate hook keeps working.
A couple of optional notes:
-
Pre-existing behavior worth knowing. Adding a new island file (one not yet in
islandsByFile) no longer triggersfresh:reload. The pre-fix reload was effectively useless anyway — the snapshot wouldn't pick up the new island without a route add/unlink invalidating it — so this isn't a meaningful regression, just a behavior to be aware of. -
Test timing. The 1500ms negative wait is fine in principle (false negatives are tolerable for "no event arrived" assertions). On a busy CI runner you may occasionally see a spurious event sneak in just after the window — consider bumping to ~2s if it ever flakes.
-
Follow-up. The original issue also reported
server.watch = nullnot stopping reloads. Worth verifying separately whether Vite respects that with our directviteServer.watcheraccess — if not, that's a Vite-side concern worth filing upstream.
Left one inline note on the comment-block phrasing.
| // relevant to SSR — either a route file, or something already | ||
| // tracked in the SSR module graph. Vite attaches us directly to | ||
| // its chokidar instance, so `server.watch.ignored` does not | ||
| // filter events before we see them. Without this gate, any |
There was a problem hiding this comment.
Small nit on phrasing: chokidar's ignored option does drop events at the source, so server.watch.ignored isn't strictly bypassed. The real reason this gate matters is that the user's globs are hard to author correctly (especially on Windows with mixed separators) and shouldn't be load-bearing for avoiding KV temp file reloads. Could we reword to focus on "defense in depth: we shouldn't depend on user ignore globs being perfect" rather than the bypass claim?
2 participants
Summary
Fixes #3637.
serverSnapshotattaches aviteServer.watcher.on(\"all\", ...)handler that runs alongside Vite's own watcher. After the island-file early-return and the route add/unlink branch, it was callingviteServer.ws.send(\"fresh:reload\")unconditionally — regardless of whether the changed file had anything to do with SSR.Because Fresh attaches directly to Vite's chokidar instance, the user's
server.watch.ignoredconfig does not filter events before Fresh sees them. On Windows, journaling and delayed-close semantics mean files like Deno KV'smain-shm/main-wal,*.tmp.*, and.timestamp-*emit watcher events on every write, so each KV write reloaded the page.This change gates
fresh:reloadso it only fires when:ssr.moduleGraph.getModulesByFile(filePath)non-empty), oroptions.routeDir.Files unknown to either are ignored: Vite's own HMR pipeline already handles client-graph files (the island early-return below covers islands;
dev_server.ts'shotUpdatecovers SSR modules through Vite's normal HMR path which does honourserver.watch.ignored).The route add/unlink branch that invalidates the snapshot is preserved.
Test plan
vite dev - unrelated file changes do not trigger fresh:reloadwrites a file (main-shm) outsideroutes/and the SSR module graph, then asserts nofresh:reloadmessage arrives on the Vite HMR WebSocket, and finally confirms that editing the route file still producesfresh:reloadso the gate isn't too aggressive.deno fmt --check,deno lint,deno task check:types, anddeno test -A packages/plugin-vite/tests/(73/73 passing) locally.Closes bartlomieju/orchid-inbox#64