diff --git a/src/primitives/LazyImport.ts b/src/primitives/LazyImport.ts index c5faffc6..c1f73c3c 100644 --- a/src/primitives/LazyImport.ts +++ b/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: `, +// SystemJS reports `, (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>( 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(); 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>( comp = s; } else if (!comp) { const [s] = createResource(() => - (p || (p = fn())).then((mod) => mod.default), + (p || (p = load())).then((mod) => mod.default), ); comp = s; } @@ -50,7 +102,15 @@ export function lazy>( : 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 }> }; } diff --git a/src/primitives/index.ts b/src/primitives/index.ts index 176ad26e..bb0b5333 100644 --- a/src/primitives/index.ts +++ b/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'; diff --git a/tests/lazyImport.spec.ts b/tests/lazyImport.spec.ts new file mode 100644 index 00000000..3749e36d --- /dev/null +++ b/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); + } + }); +});