From e89892919395ad2e0470ecc53303a2629f755807 Mon Sep 17 00:00:00 2001 From: Ajay Ghanwat Date: Wed, 19 Aug 2026 13:32:05 +0530 Subject: [PATCH] fix(react): mount visible islands and load rebuilt code after HMR Two faults found by driving the island demo in a real browser. Both were silent: the markup, every asset, and all 48 island tests were correct either way. An island renders nothing until it mounts, so its placeholder is zero-height, and IntersectionObserver does not treat a zero-area target consistently -- client:visible islands mounted on one load and not the next. Visibility for those is now decided from the element's own rect, driven by scroll and resize; a placeholder with real size still uses the observer. The strategy had no test at all, which is why this shipped. After an island source edit the browser kept running the old code. The rebuild worked and the file was refetched, but the loader imports a URL that does not change, and the browser caches modules by URL. Remounts now carry a generation the dev loader folds into the request. Verified in the browser: mounts with start={3} as a number, clicks reach React (3 -> 5), and an edit to Counter.tsx now shows the new text and stays interactive. Co-Authored-By: Claude Opus 5 --- packages/react/src/island-runtime.ts | 72 +++++++++++++++++- packages/react/src/runtime-source.ts | 7 +- packages/react/test/island-hmr.test.ts | 23 ++++++ packages/react/test/island-runtime.test.ts | 88 ++++++++++++++++++++++ 4 files changed, 184 insertions(+), 6 deletions(-) diff --git a/packages/react/src/island-runtime.ts b/packages/react/src/island-runtime.ts index 753c20c9..b35ef2cf 100644 --- a/packages/react/src/island-runtime.ts +++ b/packages/react/src/island-runtime.ts @@ -3,7 +3,14 @@ import { createRoot, type Root } from "react-dom/client"; import { IslandErrorBoundary } from "./error-boundary.tsx"; export interface MountOptions { - loader: (name: string) => Promise<{ default: ComponentType }>; + /** + * Resolve an island module by name. + * + * `generation` counts remounts. A rebuilt island keeps its URL and the + * browser caches a module by URL, so a dev loader must fold this into the + * request or the page keeps running the code it first imported. + */ + loader: (name: string, generation: number) => Promise<{ default: ComponentType }>; development?: boolean; /** * Re-render islands that are already mounted instead of skipping them. @@ -16,6 +23,7 @@ export interface MountOptions { } const roots = new Map(); +let generation = 0; export function islandRootCount(): number { return roots.size; @@ -32,8 +40,40 @@ function readProps(element: Element): Record { } } -function whenReady(element: Element, strategy: string): Promise { - if (strategy === "visible" && typeof IntersectionObserver !== "undefined") { +function rectOf(element: Element): DOMRect | null { + const measure = (element as HTMLElement).getBoundingClientRect; + return typeof measure === "function" ? (element as HTMLElement).getBoundingClientRect() : null; +} + +function inViewport(element: Element): boolean { + const rect = rectOf(element); + if (!rect) return false; + + const height = window.innerHeight || document.documentElement?.clientHeight || 0; + const width = window.innerWidth || document.documentElement?.clientWidth || 0; + + return rect.top <= height && rect.bottom >= 0 && rect.left <= width && rect.right >= 0; +} + +/** + * Resolve once the island's placeholder has come into view. + * + * An island renders nothing until it mounts, so its placeholder is usually + * zero-height -- and IntersectionObserver does not treat a zero-area target + * consistently. When it declines to report one, the island never mounts at + * all, which is silent: the markup and every asset are present and correct. + * Those are driven from the element's own rect instead; a placeholder with + * real size (an SSR fallback, or a reserved min-height) still uses the + * observer, which is cheaper and needs no scroll listener. + */ +function whenVisible(element: Element): Promise { + if (typeof window === "undefined") return Promise.resolve(); + if (inViewport(element)) return Promise.resolve(); + + const rect = rectOf(element); + const hasArea = !!rect && rect.width > 0 && rect.height > 0; + + if (hasArea && typeof IntersectionObserver !== "undefined") { return new Promise((resolve) => { const observer = new IntersectionObserver((entries) => { if (entries.some((entry) => entry.isIntersecting)) { @@ -44,6 +84,26 @@ function whenReady(element: Element, strategy: string): Promise { observer.observe(element); }); } + + return new Promise((resolve) => { + const check = () => { + if (!inViewport(element)) return; + cleanup(); + resolve(); + }; + const cleanup = () => { + window.removeEventListener("scroll", check, true); + window.removeEventListener("resize", check); + }; + + // Capture phase so a scrolling container, not just the page, wakes it. + window.addEventListener("scroll", check, true); + window.addEventListener("resize", check); + }); +} + +function whenReady(element: Element, strategy: string): Promise { + if (strategy === "visible") return whenVisible(element); if (strategy === "idle" && typeof requestIdleCallback !== "undefined") { return new Promise((resolve) => requestIdleCallback(() => resolve())); } @@ -65,7 +125,7 @@ async function mountOne(element: Element, options: MountOptions): Promise let Component: ComponentType; try { - Component = (await options.loader(name)).default; + Component = (await options.loader(name, generation)).default; } catch (error) { console.error(`[wrnexus] failed to load island bundle for '${name}'`, error); return; @@ -118,6 +178,10 @@ export function unmountIslands(root: ParentNode): void { * a runtime and is out of scope. */ export async function remountIslands(root: ParentNode, options: MountOptions): Promise { + // A remount only happens after a rebuild, so the modules on the other side + // of the loader have changed. + generation++; + // Every mounted container is swapped for a bare clone before remounting. // // Re-rendering the existing root is not enough: HMR wipes the container's diff --git a/packages/react/src/runtime-source.ts b/packages/react/src/runtime-source.ts index 177f1763..011b1255 100644 --- a/packages/react/src/runtime-source.ts +++ b/packages/react/src/runtime-source.ts @@ -8,8 +8,11 @@ export function getIslandRuntime(development = false): string { return ` (function () { - function loader(name) { - return import("/__wrnexus/island/" + encodeURIComponent(name) + ".js"); + function loader(name, generation) { + var url = "/__wrnexus/island/" + encodeURIComponent(name) + ".js"; + // A rebuilt island keeps its URL, and the browser caches modules by URL, + // so a remount has to ask for a URL it has not imported before. + return import(generation ? url + "?v=" + generation : url); } function boot() { diff --git a/packages/react/test/island-hmr.test.ts b/packages/react/test/island-hmr.test.ts index dff62401..4ebba3ce 100644 --- a/packages/react/test/island-hmr.test.ts +++ b/packages/react/test/island-hmr.test.ts @@ -92,3 +92,26 @@ test("remount re-renders in place instead of creating a second root", async () = expect(islandRootCount()).toBe(1); expect(window.document.body.textContent).toContain("v1"); }); + +test("a remount asks the loader for a newer generation than the mount did", async () => { + // The rebuilt island keeps its URL. Without a changing generation the dev + // loader re-imports the cached module and the page keeps the old code -- + // silently, because the island still mounts and still works. + const window = domWith(marker); + const generations: number[] = []; + const loader = async (_name: string, generation: number) => { + generations.push(generation); + return { default: () => createElement("span", null, `gen ${generation}`) }; + }; + + await act(async () => { + await mountIslands(host(window), { loader }); + }); + await act(async () => { + await remountIslands(host(window), { loader }); + }); + + expect(generations.length).toBe(2); + expect(generations[1]).toBeGreaterThan(generations[0]!); + expect(window.document.body.textContent).toContain(`gen ${generations[1]}`); +}); diff --git a/packages/react/test/island-runtime.test.ts b/packages/react/test/island-runtime.test.ts index 0d01a7ef..e69abe14 100644 --- a/packages/react/test/island-runtime.test.ts +++ b/packages/react/test/island-runtime.test.ts @@ -128,3 +128,91 @@ test("malformed props JSON falls back to empty props instead of throwing", async expect(window.document.body.textContent).toContain("none"); expect(islandRootCount()).toBe(1); }); + +/** + * Stand in for the browser's IntersectionObserver, reporting only targets that + * actually have area. + * + * That is the case the real one is inconsistent about: an island placeholder + * is empty until it mounts, so it is zero-height, and an engine that declines + * to report it leaves the island unmounted forever. + */ +function installAreaOnlyObserver(window: Window, onObserve?: () => void) { + const observed: Element[] = []; + (globalThis as any).IntersectionObserver = class { + constructor(private callback: (entries: { isIntersecting: boolean }[]) => void) {} + observe(element: Element) { + observed.push(element); + onObserve?.(); + const rect = (element as unknown as HTMLElement).getBoundingClientRect(); + if (rect.width > 0 && rect.height > 0) this.callback([{ isIntersecting: true }]); + } + disconnect() {} + }; + return observed; +} + +/** Place the island marker at a given position with a given size. */ +function positionIsland(window: Window, top: number, height: number) { + const element = window.document.querySelector("[data-wrn-island]") as unknown as HTMLElement; + element.getBoundingClientRect = () => + ({ top, bottom: top + height, left: 0, right: 800, width: 800, height }) as DOMRect; + return element; +} + +test("mounts a visible island whose placeholder has no height", async () => { + const window = domWith(marker("{}", "visible")); + installAreaOnlyObserver(window); + positionIsland(window, 40, 0); + + await act(async () => { + await mountIslands(host(window), { loader }); + }); + + expect(islandRootCount()).toBe(1); + delete (globalThis as any).IntersectionObserver; +}); + +test("a visible island below the fold waits, then mounts once scrolled to", async () => { + const window = domWith(marker("{}", "visible")); + installAreaOnlyObserver(window); + positionIsland(window, 5000, 0); + + let settled = false; + // Started outside act: it stays pending until the scroll, and the render it + // then performs is what act needs to wrap. + const mounting = mountIslands(host(window), { loader }).then(() => { + settled = true; + }); + + await new Promise((resolve) => setTimeout(resolve, 10)); + expect(settled).toBe(false); + expect(islandRootCount()).toBe(0); + + positionIsland(window, 100, 0); + await act(async () => { + window.dispatchEvent(new window.Event("scroll")); + await mounting; + }); + + expect(settled).toBe(true); + expect(islandRootCount()).toBe(1); + delete (globalThis as any).IntersectionObserver; +}); + +test("a placeholder with real size still goes through the observer", async () => { + const window = domWith(marker("{}", "visible")); + let observedCount = 0; + installAreaOnlyObserver(window, () => { + observedCount++; + }); + positionIsland(window, 5000, 300); + + await act(async () => { + await mountIslands(host(window), { loader }); + }); + + expect(observedCount).toBe(1); + expect(islandRootCount()).toBe(1); + delete (globalThis as any).IntersectionObserver; +});