feat(lazy): retry a failed chunk import once, cache-busted

src/primitives/LazyImport.ts

@@ -8,19 +8,71 @@ import {
   sharedConfig,
 } from 'solid-js';
 
+// The chunk URL, as named in the failure message by either module system:
+// native ESM reports `Failed to fetch dynamically imported module: <url>`,
+// SystemJS reports `<url>,  (SystemJS https://…/errors.md#3)`. The first
+// http(s) URL ending in `.js` is the chunk in both cases — SystemJS's trailing
+// docs link has no `.js` in it, so it cannot be matched by mistake.
+const CHUNK_URL = /https?:\/\/\S+?\.js/;
+
+/**
+ * @internal Exported for tests. The primitives barrel re-exports `lazy` by
+ * name, so this stays out of the package's public API.
+ */
+export const chunkUrlFromError = (error: unknown): string | undefined =>
+  CHUNK_URL.exec(error instanceof Error ? error.message : String(error))?.[0];
+
+/** @internal Exported for tests — see `chunkUrlFromError`. */
+export const cacheBust = (url: string): string =>
+  `${url}${url.indexOf('?') === -1 ? '?' : '&'}chunkRetry=1`;
+
 // lazy load a function component asynchronously
 export function lazy<T extends Component<any>>(
   fn: () => Promise<{ default: T }>,
 ): T & { preload: () => Promise<{ default: T }> } {
   let comp: () => T | undefined;
   let p: Promise<{ default: T }> | undefined;
+
+  // Retry the import exactly once before giving up. On a TV the chunk fetch is
+  // the fragile part — a brief dropout as the viewer presses OK on a rail
+  // rejects the import, and because `p` memoises the promise that single
+  // rejection is replayed for the rest of the session: the route never renders
+  // again, not even on a later navigation. One retry turns the common transient
+  // failure into a marginally slower navigation.
+  //
+  // The retry re-imports under a cache-busting URL rather than re-running `fn`,
+  // because re-running only works on one of the two module systems we ship to.
+  // Under SystemJS — the `@vitejs/plugin-legacy` output older TV browsers run —
+  // the failed load is dropped from the registry, so calling `fn` again really
+  // does re-fetch. Native ESM does the opposite: the module map memoises the
+  // *failure*, so a second `import()` of the same specifier resolves straight to
+  // the cached rejection without ever touching the network, and the retry is a
+  // no-op precisely where it is needed. A distinct URL gets a fresh module-map
+  // entry and actually re-fetches.
+  //
+  // The cost is a duplicate module record for this one chunk. Its own imports
+  // are unaffected — they resolve to their normal, already-cached URLs — so the
+  // duplication does not spread, and it only happens on a retry that would
+  // otherwise have left the route dead.
+  //
+  // `fn` is still the fallback when the message names no URL: no worse than not
+  // retrying, and it keeps non-URL loaders (tests, custom resolvers) working.
+  const load = (): Promise<{ default: T }> =>
+    fn().catch((error: unknown) => {
+      const url = chunkUrlFromError(error);
+      if (url === undefined) return fn();
+      return import(/* @vite-ignore */ cacheBust(url)) as Promise<{
+        default: T;
+      }>;
+    });
+
   const wrap: T & { preload?: () => void } = ((props: any) => {
     const ctx = sharedConfig.context;
     if (ctx) {
       const [s, set] = createSignal<T>();
       sharedConfig.count || (sharedConfig.count = 0);
       sharedConfig.count++;
-      (p || (p = fn()))
+      (p || (p = load()))
         .then((mod) => {
           !sharedConfig.done && (sharedConfig.context = ctx);
           sharedConfig.count!--;
@@ -31,7 +83,7 @@ export function lazy<T extends Component<any>>(
       comp = s;
     } else if (!comp) {
       const [s] = createResource<T>(() =>
-        (p || (p = fn())).then((mod) => mod.default),
+        (p || (p = load())).then((mod) => mod.default),
       );
       comp = s;
     }
@@ -50,7 +102,15 @@ export function lazy<T extends Component<any>>(
         : null;
     }) as unknown as JSX.Element;
   }) as T;
+  // The `.then` here is a fire-and-forget side effect (it caches the resolved
+  // component); `p` is what the caller gets back. Without the `.catch` that
+  // side-effect promise has no rejection handler of its own, so a failed
+  // preload raises an unhandledrejection even when the caller dutifully catches
+  // the promise it was handed — a warmed route that fails to fetch would report
+  // as an uncaught exception. Mirrors the `.catch(() => {})` on the hydration
+  // path above; the real failure still reaches the caller through `p`.
   wrap.preload = () =>
-    p || ((p = fn()).then((mod) => (comp = () => mod.default)), p);
+    p ||
+    ((p = load()).then((mod) => (comp = () => mod.default)).catch(() => {}), p);
   return wrap as T & { preload: () => Promise<{ default: T }> };
 }

src/primitives/index.ts

@@ -5,7 +5,9 @@ export * from './borderBox.jsx';
 export * from './useMouse.js';
 export * from './portal.jsx';
 export * from './Lazy.jsx';
-export * from './LazyImport.js';
+// Named rather than `export *`: LazyImport also exports internal helpers for
+// its tests, and those must not become part of the public API.
+export { lazy } from './LazyImport.js';
 export * from './Image.jsx';
 export * from './Visible.jsx';
 export * from './Column.jsx';

tests/lazyImport.spec.ts

@@ -0,0 +1,150 @@
+import { describe, it, expect, vi } from 'vitest';
+import {
+  cacheBust,
+  chunkUrlFromError,
+  lazy,
+} from '../src/primitives/LazyImport.ts';
+
+// `preload()` is the seam: it drives the same `load()` the render paths use, so
+// the retry can be exercised without standing up a renderer.
+
+const Page = () => null;
+const mod = { default: Page };
+
+// The two real failure messages, one per module system.
+const NATIVE_ESM_ERROR = new Error(
+  'Failed to fetch dynamically imported module: https://ott.angel.com/vizio/assets/Theater.page-Bl9KRTxZ.js',
+);
+const SYSTEMJS_ERROR = new Error(
+  'https://ott.angel.com/webos/assets/Theater.page-legacy-C2p81iuu.js,  (SystemJS https://github.com/systemjs/systemjs/blob/main/docs/errors.md#3)',
+);
+
+describe('chunkUrlFromError', () => {
+  it('reads the chunk URL out of a native ESM failure', () => {
+    expect(chunkUrlFromError(NATIVE_ESM_ERROR)).toBe(
+      'https://ott.angel.com/vizio/assets/Theater.page-Bl9KRTxZ.js',
+    );
+  });
+
+  // The SystemJS message wraps the URL in punctuation — a trailing comma, then
+  // its own docs link. The `.js` anchor is what stops the match running past
+  // the chunk into that comma; dropping it yields a URL that 404s on retry.
+  it('reads the chunk URL, not the docs link, out of a SystemJS failure', () => {
+    expect(chunkUrlFromError(SYSTEMJS_ERROR)).toBe(
+      'https://ott.angel.com/webos/assets/Theater.page-legacy-C2p81iuu.js',
+    );
+  });
+
+  it('returns undefined when the message names no chunk', () => {
+    expect(chunkUrlFromError(new Error('boom'))).toBeUndefined();
+    expect(chunkUrlFromError('not an error at all')).toBeUndefined();
+  });
+});
+
+describe('cacheBust', () => {
+  it('starts a query string when the URL has none', () => {
+    expect(cacheBust('https://x/a.js')).toBe('https://x/a.js?chunkRetry=1');
+  });
+
+  it('appends to an existing query string', () => {
+    expect(cacheBust('https://x/a.js?v=1')).toBe(
+      'https://x/a.js?v=1&chunkRetry=1',
+    );
+  });
+});
+
+describe('lazy() chunk-load retry', () => {
+  it('does not retry an import that succeeds', async () => {
+    const fn = vi.fn().mockResolvedValue(mod);
+
+    await expect(lazy(fn).preload()).resolves.toBe(mod);
+    expect(fn).toHaveBeenCalledTimes(1);
+  });
+
+  // When the failure names a chunk, the retry must NOT re-run the loader:
+  // re-running the same specifier is exactly the no-op that leaves Vizio
+  // broken, because native ESM serves the memoised failure from the module map.
+  // The loader staying on one call is what proves the cache-busting branch ran.
+  //
+  // The import itself cannot resolve here — Node's ESM loader only accepts
+  // `file:` and `data:` — so the promise rejects. That rejection is the test
+  // environment's, not the behaviour under test, hence no assertion on its
+  // message; `chunkUrlFromError` and `cacheBust` above pin the URL that is
+  // actually requested.
+  it.each([
+    ['native ESM', NATIVE_ESM_ERROR],
+    ['SystemJS', SYSTEMJS_ERROR],
+  ])(
+    're-imports under a cache-busting URL after a %s failure',
+    async (_label, error) => {
+      const fn = vi.fn().mockRejectedValue(error);
+
+      await expect(lazy(fn).preload()).rejects.toThrow();
+      expect(fn).toHaveBeenCalledTimes(1);
+    },
+  );
+
+  // A loader that is not a URL import at all — a test stub, a custom resolver —
+  // has no URL to bust, so re-running it is the best available retry.
+  it('re-runs the loader when the failure names no chunk URL', async () => {
+    const fn = vi
+      .fn()
+      .mockRejectedValueOnce(new Error('boom'))
+      .mockResolvedValue(mod);
+
+    await expect(lazy(fn).preload()).resolves.toBe(mod);
+    expect(fn).toHaveBeenCalledTimes(2);
+  });
+
+  // Bounded at one retry: a TV that has genuinely lost its connection should
+  // surface the failure rather than sit on a blank screen retrying.
+  it('gives up after exactly one retry and rejects with the second error', async () => {
+    const fn = vi
+      .fn()
+      .mockRejectedValueOnce(new Error('first'))
+      .mockRejectedValueOnce(new Error('second'));
+
+    await expect(lazy(fn).preload()).rejects.toThrow('second');
+    expect(fn).toHaveBeenCalledTimes(2);
+  });
+
+  // `p` memoises the promise, so a component mounting after a preload — or
+  // several routes preloading at once — must not refetch the chunk.
+  it('loads the chunk once across repeated preloads', async () => {
+    const fn = vi.fn().mockResolvedValue(mod);
+    const Comp = lazy(fn);
+
+    await Promise.all([Comp.preload(), Comp.preload()]);
+    await Comp.preload();
+
+    expect(fn).toHaveBeenCalledTimes(1);
+  });
+});
+
+describe('lazy() preload rejection handling', () => {
+  // A warmed route that fails to fetch must not surface as an uncaught
+  // exception. `preload()`'s internal `.then` is a fire-and-forget side effect,
+  // so without its own catch it raises an unhandledRejection even when the
+  // caller catches the promise it was handed.
+  //
+  // The listener goes on `process`, not `window`: under the jsdom environment a
+  // Node-level promise rejection is reported there, and a `window`
+  // 'unhandledrejection' listener never fires — a version of this test written
+  // that way passes whether or not the bug is present.
+  it('does not raise an unhandled rejection when a preload fails', async () => {
+    const unhandled: unknown[] = [];
+    const onUnhandled = (reason: unknown) => unhandled.push(reason);
+    process.on('unhandledRejection', onUnhandled);
+
+    try {
+      const fn = vi.fn().mockRejectedValue(new Error('chunk gone'));
+      await expect(lazy(fn).preload()).rejects.toThrow('chunk gone');
+
+      // Node reports a rejection as unhandled a macrotask after the fact.
+      await new Promise((resolve) => setTimeout(resolve, 20));
+      expect(unhandled).toEqual([]);
+    } finally {
+      process.off('unhandledRejection', onUnhandled);
+    }
+  });
+});