fix(dev): key route module reloads on mtime so module state survives a request - #3912
Conversation
…a request
Every request loads its route through `loadHandlerModule`, and the direct-import
path appended `?v=${Date.now()}` unconditionally. That minted a fresh module URL
per load, so module-level state reset between requests under `veryfront dev`
while persisting under `veryfront serve`.
The cost is not limited to any one feature: a module-scoped client, cache,
connection pool, or in-memory store behaves differently in dev than in
production, silently and with no error to show for it. It is what makes the
workflows guide's start-then-poll flow impossible in dev, because
`createWorkflowClient()` keeps runs in a per-instance memory backend.
Key the cache buster on the file's mtime instead. An edited route still gets a
new module, and an untouched route keeps the one it already has. A filesystem
that cannot report mtime falls back to the clock, which is no worse than today.
Both halves are pinned by tests: state survives a repeat load of an unchanged
file, and an edit is still picked up rather than served from cache.
|
Warning Review limit reached
Next review available in: 28 minutes Limit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
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 |
📦 Client bundle boundary
A server module in a client graph aborts hydration in the browser. New leaks fail CI; known leaks are tracked in |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4438e5c981
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Scope note, so the fix is not read as broader than it is.
Direct import ( Bundle and transpile ( const tempDir = await fs.makeTempDir({ prefix: "vf-api-" });
const tempFile = pathHelper.join(tempDir, "handler.mjs");— so the module URL is unique per load regardless of whether the source changed. Because bundling inlines dependencies, this also re-instantiates any module the route imports, which is why extracting a shared client module into Fixing that side means keying the temp file on a content hash and reusing it, which is a larger change than this one and worth doing separately rather than folding in here. So: after this PR, module state survives between requests on the direct-import path. Routes that fall back to bundling still reset per request. |
…nged The mtime fix covered the direct-import path only. A route that Deno cannot resolve on its own, such as one importing through the project's `@/` alias, falls back to bundling, and that path had the same defect by a different mechanism: `loadModuleFromCode` imports from a fresh `makeTempDir` every call, so the module URL is unique per load no matter what the source says. Because bundling inlines dependencies, this re-instantiated everything the route imported too, which is why extracting a shared client module did not help. Cache bundled modules on their generated source instead. The key carries the project and route path as well as the code, so two projects that happen to bundle byte-identical output never share a module, and with it module state. A failed build is not remembered, and the cache is capped so a long editing session does not grow without bound. Together with the previous commit, both load paths now keep module state across requests and still pick up an edit.
Route modules need stable instances while unchanged and fresh imports after any edit. Mtime alone can collide on coarse filesystems or same-millisecond same-size writes, so the direct import key now includes a source digest. The bundled fallback keeps equivalent generated-source caching scoped to the project route. Confidence: high Scope-risk: moderate Tested: deno test --preload=src/testing/preload.ts --no-check --allow-all --unstable-worker-options --unstable-net src/routing/api/module-loader/loader.test.ts Tested: deno check src/routing/api/module-loader/loader.ts src/routing/api/module-loader/loader.test.ts Tested: git diff --check -- src/routing/api/module-loader/loader.ts src/routing/api/module-loader/loader.test.ts
|
Pushed
Two more tests, same shape as the direct-path pair, and I confirmed the reuse one fails against the old implementation first:
The project that previously reset every request now holds state and still hot reloads:
This also unblocks #3913. With both commits applied, a workflow started in one request is readable in the next on the bundling path, which was returning 404 before: |
What
Under
veryfront dev, module-level state in an API route resets between requests. Underveryfront serveit persists. Same code, different behaviour, no error either way.Measured with a module-scoped id returned from a route (
0.1.1246-rc.13479):Cause
src/routing/api/module-loader/loader.tsappended?v=${Date.now()}to the import URL on the direct-import path, unconditionally. Every request routes throughloadHandlerModule, so every request minted a new module URL and therefore a new module instance.The intent is clearly hot reload. The defect is that it busts the cache even when the file has not changed.
Why it matters beyond one feature
Any module-scoped client, cache, connection pool, rate limiter, or in-memory store behaves differently in dev than in production, silently.
It is also what makes the workflows guide's start-then-poll flow impossible in dev:
createWorkflowClient()keeps runs in a per-instanceMemoryBackend, so with a new module per request no run is ever visible to a later one. Extracting a shared client module does not help, because the shared module is re-instantiated too. (The doc side of that is #3911; this is the runtime side.)Fix
Key the cache buster on the file's mtime. An edited route gets a new module; an untouched route keeps the one it has. A filesystem that cannot report mtime falls back to the clock, which is no worse than current behaviour.
Verification
Two tests, pinning both halves — I confirmed the first fails against the old implementation and the second already passed, so hot reload is genuinely protected rather than assumed:
reuses an unchanged route module so module state survives between requestspicks up an edited route module instead of serving the cached oneLive check against a real dev server built from this branch:
State persists, and the edit is still picked up without a restart. Full pre-push suite green.
Found while dogfooding the documented developer journey.