diff --git a/packages/plugin-vite/src/plugins/server_snapshot.ts b/packages/plugin-vite/src/plugins/server_snapshot.ts index ae1487652ff..6751b427d1c 100644 --- a/packages/plugin-vite/src/plugins/server_snapshot.ts +++ b/packages/plugin-vite/src/plugins/server_snapshot.ts @@ -50,12 +50,41 @@ export function serverSnapshot(options: ResolvedFreshViteConfig): Plugin[] { const islands = new Map(); const islandsByFile = new Set(); const islandSpecByName = new Map(); + const islandNameBySpec = new Map(); const routeNamer = new UniqueNamer(); const routeFileToName = new Map(); // deno-lint-ignore no-explicit-any const routes: Map> = new Map(); + // Rebuild the module-scoped island maps from scratch; a deleted island would + // otherwise linger and get emitted as a stale `import` in the snapshot. + function rebuildIslands(fileSpecs: string[]): void { + islands.clear(); + islandsByFile.clear(); + islandSpecByName.clear(); + + const add = (spec: string, name: string) => { + islands.set(spec, { name, chunk: null }); + islandSpecByName.set(name, spec); + }; + + // Remote islands are re-seeded first. + options.islandSpecifiers.forEach((name, spec) => add(spec, name)); + + for (const spec of fileSpecs) { + // Reuse the cached name so the namer doesn't suffix `_1` on each rebuild. + let name = islandNameBySpec.get(spec); + if (name === undefined) { + name = options.namer.getUniqueName(specToName(spec)); + islandNameBySpec.set(spec, name); + } + + add(spec, name); + islandsByFile.add(spec); + } + } + return [ { name: "fresh:server-snapshot", @@ -78,11 +107,7 @@ export function serverSnapshot(options: ResolvedFreshViteConfig): Plugin[] { config.root, ); - options.islandSpecifiers.forEach((name, spec) => { - islands.set(spec, { name, chunk: null }); - islandSpecByName.set(name, spec); - // islandsByFile.add(spec); - }); + rebuildIslands([]); }, configureServer(viteServer) { server = viteServer; @@ -112,21 +137,17 @@ export function serverSnapshot(options: ResolvedFreshViteConfig): Plugin[] { } } - // Check for route files. We need to invalidate the snapshot if - // they are removed or added. - if ( - (ev === "add" || ev === "unlink") && - !/[\\/]+\(_[^)]+\)[\\/]+/.test(filePath) - ) { - const relRoutes = path.relative(options.routeDir, filePath); - if (!relRoutes.startsWith("..")) { + // Check for route and island files. We need to invalidate the + // snapshot if they are removed or added. + if (ev === "add" || ev === "unlink") { + const inRoutes = !/[\\/]+\(_[^)]+\)[\\/]+/.test(filePath) && + !path.relative(options.routeDir, filePath).startsWith(".."); + const inIslands = !path.relative(options.islandsDir, filePath) + .startsWith(".."); + + if (inRoutes || inIslands) { const mod = ssr.moduleGraph.getModuleById(`\0${modName}`); if (mod !== undefined) { - // Clear state - islands.clear(); - islandsByFile.clear(); - islandSpecByName.clear(); - ssr.moduleGraph.invalidateModule(mod); } } @@ -158,15 +179,7 @@ export function serverSnapshot(options: ResolvedFreshViteConfig): Plugin[] { ignore: options.ignore, }); - for (let i = 0; i < result.islands.length; i++) { - const spec = result.islands[i]; - const specName = specToName(spec); - const name = options.namer.getUniqueName(specName); - - islands.set(spec, { name, chunk: null }); - islandSpecByName.set(name, spec); - islandsByFile.add(spec); - } + rebuildIslands(result.islands); for (let i = 0; i < result.routes.length; i++) { const route = result.routes[i]; @@ -196,7 +209,10 @@ export function serverSnapshot(options: ResolvedFreshViteConfig): Plugin[] { const mod = server.environments.client.moduleGraph.getModuleById( id, ); - if (mod !== undefined) { + // Skip a `fresh-island::Name` virtual `mod.url` (from the fallback + // resolver when the client graph was cold): emitting it as the chunk + // leaves a bare import the browser can't resolve. Keep the `/@id/` URL. + if (mod !== undefined && !mod.url.startsWith("fresh-island::")) { const def = islands.get(id); if (def !== undefined) def.chunk = mod.url; } diff --git a/packages/plugin-vite/tests/dev_server_test.ts b/packages/plugin-vite/tests/dev_server_test.ts index 1572154f96a..e9abb6a376c 100644 --- a/packages/plugin-vite/tests/dev_server_test.ts +++ b/packages/plugin-vite/tests/dev_server_test.ts @@ -110,6 +110,49 @@ integrationTest( }, ); +// Deleting an auto-discovered island used to break the dev server two ways: the +// stale island lingered in the snapshot map as a dead import (permanent 500), +// and the surviving sibling's boot import got rewritten to the unresolvable +// `fresh-island::Name` specifier, killing its hydration after a reload. +integrationTest( + "vite dev - deleting an island recovers and keeps siblings interactive", + async () => { + const fixture = path.join(FIXTURE_DIR, "delete_island"); + await withDevServer(fixture, async (address, dir) => { + await withBrowser(async (page) => { + // Wire the surviving island into the client graph and hydrate it. + await page.goto(`${address}/`, { waitUntil: "networkidle2" }); + await waitForText(page, ".keep", "keep 0"); + await page.locator(".keep").click(); + await waitForText(page, ".keep", "keep 1"); + + // Delete the sibling: the watcher rebuilds the island map and reloads. + await Deno.remove(path.join(dir, "islands", "Extra.tsx")); + + // The delete is async, so require several consecutive 200s to be sure + // the server didn't wedge on the dead import. + await waitFor(async () => { + for (let i = 0; i < 5; i++) { + const res = await fetch(`${address}/`); + const html = await res.text(); + expect(res.status).toEqual(200); + // Boot import must stay resolvable, no virtual specifier leaked. + expect(/from\s+"fresh-island::/.test(html)).toBe(false); + await new Promise((r) => setTimeout(r, 150)); + } + return true; + }); + + // The survivor still hydrates and stays interactive after the reload. + await page.goto(`${address}/`, { waitUntil: "networkidle2" }); + await waitForText(page, ".keep", "keep 0"); + await page.locator(".keep").click(); + await waitForText(page, ".keep", "keep 1"); + }); + }); + }, +); + integrationTest("vite dev - starts without routes/ dir", async () => { const fixture = path.join(FIXTURE_DIR, "no_routes"); await withDevServer(fixture, async (address) => { diff --git a/packages/plugin-vite/tests/fixtures/delete_island/islands/Extra.tsx b/packages/plugin-vite/tests/fixtures/delete_island/islands/Extra.tsx new file mode 100644 index 00000000000..856e7eb0608 --- /dev/null +++ b/packages/plugin-vite/tests/fixtures/delete_island/islands/Extra.tsx @@ -0,0 +1,10 @@ +import { useSignal } from "@preact/signals"; + +export default function Extra() { + const s = useSignal(0); + return ( + + ); +} diff --git a/packages/plugin-vite/tests/fixtures/delete_island/islands/Keep.tsx b/packages/plugin-vite/tests/fixtures/delete_island/islands/Keep.tsx new file mode 100644 index 00000000000..2c8f659cfdc --- /dev/null +++ b/packages/plugin-vite/tests/fixtures/delete_island/islands/Keep.tsx @@ -0,0 +1,10 @@ +import { useSignal } from "@preact/signals"; + +export default function Keep() { + const s = useSignal(0); + return ( + + ); +} diff --git a/packages/plugin-vite/tests/fixtures/delete_island/main.ts b/packages/plugin-vite/tests/fixtures/delete_island/main.ts new file mode 100644 index 00000000000..d98e91b183d --- /dev/null +++ b/packages/plugin-vite/tests/fixtures/delete_island/main.ts @@ -0,0 +1,3 @@ +import { App } from "@fresh/core"; + +export const app = new App().fsRoutes(); diff --git a/packages/plugin-vite/tests/fixtures/delete_island/routes/index.tsx b/packages/plugin-vite/tests/fixtures/delete_island/routes/index.tsx new file mode 100644 index 00000000000..12bf2c33617 --- /dev/null +++ b/packages/plugin-vite/tests/fixtures/delete_island/routes/index.tsx @@ -0,0 +1,10 @@ +import Keep from "../islands/Keep.tsx"; + +export default function Hello() { + return ( +
+

ok

+ +
+ ); +}