Skip to content

fix: inject client entry on islands-free pages for HMR #3810

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
bartlomieju merged 2 commits into from
May 27, 2026
+41 −0
Merged

fix: inject client entry on islands-free pages for HMR #3810

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6b6da69
fix(vite): inject client entry on islands-free pages for HMR
fibibot May 14, 2026
494cda7
address review: clarify hmrClientEntry is a presence marker, not a path
bartlomieju May 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions packages/fresh/src/build_cache.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@ export interface FileSnapshot {
export interface BuildSnapshot<State> {
version: string;
clientEntry: string;
/**
* 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. Undefined outside of dev.
*/
hmrClientEntry?: string;
Copy link
Copy Markdown
Contributor

@bartlomieju bartlomieju May 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

  1. 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.").
  2. Replace with a boolean (e.g. forceBootInDev) in a follow-up cleanup.

Option 1 is enough for this PR.

fsRoutes: FsRouteFile<State>[];
staticFiles: Map<string, FileSnapshot>;
islands: ServerIslandRegistry;
Expand Down Expand Up @@ -51,13 +57,15 @@ export class ProdBuildCache<State> implements BuildCache<State> {
#snapshot: BuildSnapshot<State>;
islandRegistry: ServerIslandRegistry;
clientEntry: string;
hmrClientEntry: string | undefined;
features = { errorOverlay: false };

constructor(public root: string, snapshot: BuildSnapshot<State>) {
setBuildId(snapshot.version);
this.#snapshot = snapshot;
this.islandRegistry = snapshot.islands;
this.clientEntry = snapshot.clientEntry;
this.hmrClientEntry = snapshot.hmrClientEntry;
}

getEntryAssets(): string[] {
Expand Down
6 changes: 6 additions & 0 deletions packages/fresh/src/dev/dev_build_cache.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@
Array.from(this.islandModNameToChunk.entries()).map(
async ([name, chunk]) => {
const fileUrl = maybeToFileUrl(chunk.server);
const mod = await import(fileUrl);

Check warning on line 215 in packages/fresh/src/dev/dev_build_cache.ts

View workflow job for this annotation

GitHub Actions / test (v2.x, ubuntu-latest) 

unable to analyze dynamic import

if (chunk.browser === null) {
throw new Error(`Unexpected missing browser chunk`);
Expand All @@ -230,7 +230,7 @@
const fileUrl = maybeToFileUrl(file.filePath);
return {
...file,
mod: file.lazy ? () => import(fileUrl) : await import(fileUrl),

Check warning on line 233 in packages/fresh/src/dev/dev_build_cache.ts

View workflow job for this annotation

GitHub Actions / test (v2.x, ubuntu-latest) 

unable to analyze dynamic import

Check warning on line 233 in packages/fresh/src/dev/dev_build_cache.ts

View workflow job for this annotation

GitHub Actions / test (v2.x, ubuntu-latest) 

unable to analyze dynamic import
};
}));
this.#commands = fsItemsToCommands(files);
Expand Down Expand Up @@ -458,6 +458,7 @@
outDir: string;
buildId: string;
clientEntry: string;
hmrClientEntry?: string;
islands: IslandModChunk[];
// deno-lint-ignore no-explicit-any
fsRoutesFiles: FsRouteFileNoMod<any>[];
Expand Down Expand Up @@ -524,12 +525,17 @@
const entryAssets = options.entryAssets.map((url) => JSON.stringify(url))
.join(",\n");

const hmrClientEntryDecl = options.hmrClientEntry !== undefined
? `export const hmrClientEntry = ${JSON.stringify(options.hmrClientEntry)}`
: "";

return `${EDIT_WARNING}
import { IslandPreparer } from "fresh/internal";
${islandImports}
${fsRouteImports}

export const clientEntry = ${JSON.stringify(options.clientEntry)}
${hmrClientEntryDecl}
export const version = ${JSON.stringify(options.buildId)}

export const islands = new Map();
Expand Down
10 changes: 10 additions & 0 deletions packages/plugin-vite/src/plugins/server_snapshot.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,10 +165,19 @@ export function serverSnapshot(options: ResolvedFreshViteConfig): Plugin[] {
const staticFiles: PendingStaticFile[] = [];
let islandMods: IslandModChunk[] = [];
let clientEntry = "/@id/fresh:client-entry";
let hmrClientEntry: string | undefined;
let buildId = "";
const entryAssets: string[] = [];

if (isDev && server !== undefined) {
// Set hmrClientEntry so the SSR runtime always emits a boot script
// in dev, even when a page has zero islands. Without this, edits
// to island-free routes never trigger a browser reload because the
// `fresh:reload` WebSocket listener is never attached. The value
// is used as a marker only — its presence is what matters, not
// what it points to.
hmrClientEntry = clientEntry;
Copy link
Copy Markdown
Contributor

@bartlomieju bartlomieju May 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


for (const id of islands.keys()) {
const mod = server.environments.client.moduleGraph.getModuleById(
id,
Expand Down Expand Up @@ -380,6 +389,7 @@ export function serverSnapshot(options: ResolvedFreshViteConfig): Plugin[] {
staticFiles,
buildId,
clientEntry,
hmrClientEntry,
entryAssets,
fsRoutesFiles: result.routes,
islands: islandMods,
Expand Down
17 changes: 17 additions & 0 deletions packages/plugin-vite/tests/dev_server_test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,23 @@ integrationTest("vite dev - starts without islands/ dir", async () => {
});
});

// Issue: https://github.com/denoland/fresh/issues/3806
// Pages without islands must still load the client entry in dev so the
// HMR `fresh:reload` listener attaches and route edits trigger a refresh.
integrationTest(
"vite dev - injects client entry on islands-free pages for HMR",
async () => {
const fixture = path.join(FIXTURE_DIR, "no_islands");
await withDevServer(fixture, async (address) => {
const res = await fetch(`${address}/`);
const text = await res.text();
expect(text).toContain("ok");
expect(text).toContain("/@id/fresh:client-entry");
expect(text).toMatch(/import\s*\{\s*boot\s*\}/);
});
},
Copy link
Copy Markdown
Contributor

@bartlomieju bartlomieju May 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

);

integrationTest("vite dev - starts without routes/ dir", async () => {
const fixture = path.join(FIXTURE_DIR, "no_routes");
await withDevServer(fixture, async (address) => {
Expand Down
Loading