fix: inject client entry on islands-free pages for HMR#3810
Conversation
fibibot
changed the title
bartlomieju
left a comment
bartlomieju
left a comment
There was a problem hiding this comment.
Diagnosis and fix look correct — I traced the chain end-to-end:
- SSR-only edits broadcast
fresh:reload(plugin-vite/src/plugins/dev_server.ts:228). - The listener lives in
plugin-vite/src/client.ts, which is folded intofresh:client-entryviaclient_entry.ts:54. - The boot script is gated on
needsClientRuntime || hmrClientEntry !== undefined(preact_hooks.ts:726), and the Vite plugin previously never sethmrClientEntry, so islands-free pages never loaded the listener.
The legacy esbuild dev path already toggles hmrClientEntry at dev/builder.ts:375, so this brings the Vite plugin in line. A few small things worth addressing — see inline comments.
| * always emits the boot script so HMR listeners attach to pages that | ||
| * have no islands. Undefined outside of dev. | ||
| */ | ||
| hmrClientEntry?: string; |
There was a problem hiding this comment.
The JSDoc calls this a "Pathname for an HMR-only client entry," but in practice the value is never imported — only its presence gates the boot script in preact_hooks.ts:726 and the preload header in context.ts:420. The boot script always imports buildCache.clientEntry, never hmrClientEntry.
That's fine, but the field is effectively a bootInDev: boolean masquerading as a pathname (the legacy esbuild builder happens to assign a real chunk path; the Vite plugin assigns the same string as clientEntry). Two options:
- Keep the field, but rewrite the doc to describe it as a gate ("When defined, forces the boot script to be emitted in dev so HMR listeners attach even on island-free pages. The value itself is currently unused — only its presence matters.").
- Replace with a boolean (e.g.
forceBootInDev) in a follow-up cleanup.
Option 1 is enough for this PR.
| // a page has zero islands. Without this, edits to islands-free | ||
| // routes never trigger a browser reload because the | ||
| // `fresh:reload` WebSocket listener is never attached. | ||
| hmrClientEntry = clientEntry; |
There was a problem hiding this comment.
Assigning clientEntry here is functionally a marker — the value isn't read, only the !== undefined check matters (see preact_hooks.ts:726). Worth a short comment noting that the presence is what matters, so a future reader doesn't assume the value is plumbed through to an import.
| let revision = 0; | ||
|
|
||
| let reconnectTimer: number; | ||
| let reconnectTimer: ReturnType<typeof setTimeout>; |
There was a problem hiding this comment.
Unrelated to the HMR fix — looks like an incidental type tightening. Either mention it in the PR description, or split into its own commit so the fix commit stays minimal.
| expect(text).toContain("/@id/fresh:client-entry"); | ||
| expect(text).toMatch(/import\s*\{\s*boot\s*\}/); | ||
| }); | ||
| }, |
There was a problem hiding this comment.
Good regression test for the missing script tag. Worth considering a follow-up that exercises the actual reload flow end-to-end (edit a route file, observe fresh:reload on the WS) if the harness supports it — the current assertion catches the symptom (script not emitted) but not a regression that emits the script while breaking the listener wiring. Not a blocker for this PR.
The dev server emits `fresh:reload` over the Vite WebSocket whenever an SSR-only module changes, but the listener for that event lives in the client entry. The Vite plugin only injected the client entry when a page had at least one island, so projects with islands-free routes never had the `fresh:reload` listener attached and edits did not refresh the browser. Wire `hmrClientEntry` through the build snapshot so that in dev the SSR runtime always emits the boot script, which loads the client entry and attaches the HMR listener regardless of whether the route uses islands. Closes #3806
Reword JSDoc and inline comment to make it explicit that only the presence of hmrClientEntry matters — the value itself is not read by any consumer, so assigning clientEntry to it is intentional marker semantics, not a real path reference.
bartlomieju
force-pushed
the
orch/issue-85
branch
from
37171bc to
494cda7
Compare
bartlomieju
left a comment
bartlomieju
left a comment
There was a problem hiding this comment.
Review feedback addressed. Rebased on main (dropping the duplicate reconnectTimer fix that landed in #3820). Merging.
2 participants
Summary
Fixes #3806: when a Fresh project has no islands, edits to
a route file in dev mode never refreshed the browser.
The dev server already broadcasts
fresh:reloadover Vite's WebSocketwhen an SSR-only module changes (
packages/plugin-vite/src/plugins/dev_server.ts),but the listener for that event lives in
@fresh/plugin-vite/client,which is only loaded transitively via the client entry boot script. The
SSR runtime only emitted that boot script when the page had at least
one island or
hmrClientEntrywas defined. The Vite plugin never sethmrClientEntry, so islands-free pages had no listener and edits weresilently dropped.
Fix
Wire
hmrClientEntrythrough the build snapshot so the SSR runtimealways emits the boot script in dev:
hmrClientEntry?: stringtoBuildSnapshotand surface it onProdBuildCache(used by the Vite snapshot loader).generateSnapshotServeremit ahmrClientEntryexport when oneis provided.
hmrClientEntryto the client entry virtual id in the Viteplugin's
server_snapshot.tswhenever the server is in dev mode.This piggybacks on the existing FreshRuntimeScript branch that was
clearly written with this case in mind (note the existing comment:
"Full-document boot: islands / partials / client nav, or Vite/HMR
dev"); it just wasn't being triggered for the Vite plugin.
Test plan
vite dev - injects client entry on islands-free pages for HMRinpackages/plugin-vite/tests/dev_server_test.tsasserts theno-islands fixture's SSR response contains both
/@id/fresh:client-entryand theimport { boot }call.deno task okpasses locally (613 tests passed, 0 failed).loads islands,partial island,nested islands,remote island,vite build - *) all still pass.Closes bartlomieju/orchid-inbox#85