diff --git a/packages/cli/src/commands/add.test.ts b/packages/cli/src/commands/add.test.ts index e11e63974..2f3d7ce5e 100644 --- a/packages/cli/src/commands/add.test.ts +++ b/packages/cli/src/commands/add.test.ts @@ -3,7 +3,14 @@ import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "no import { tmpdir } from "node:os"; import { join } from "node:path"; import type { RegistryItem, RegistryManifest } from "@hyperframes/core"; -import { AddError, buildSnippet, parseVariableValues, remapTarget, runAdd } from "./add.js"; +import { + AddError, + buildSnippet, + describeInstallFailure, + parseVariableValues, + remapTarget, + runAdd, +} from "./add.js"; import { trackRegistryItemAdded } from "../telemetry/events.js"; // Assert the emitted payload rather than the transport: `shouldTrack()` is @@ -462,3 +469,46 @@ describe("variable values in the snippet", () => { expect(parseVariableValues(undefined)).toBeNull(); }); }); + +describe("describeInstallFailure", () => { + it("explains a bare transport failure instead of echoing it", () => { + // What the user actually sees after copying a command off the catalog page. + // Item FILES are not cached, so a blip surfaces as node's `fetch failed` + // with no URL and no cause, and reads like the command was wrong. + const message = describeInstallFailure(new Error("fetch failed")); + + expect(message).toContain("could not download the item's files"); + expect(message).toContain("rather than a bad command"); + expect(message).toContain("HTTPS_PROXY"); + }); + + it("names the project's own registry when it is not the public one", () => { + // The reported failure: hyperframes.json pointed at a private host with a + // self-signed certificate. Telling that reader to check their connection + // sends them to debug the one thing that was working. + const message = describeInstallFailure( + new Error("fetch failed"), + "https://private.example/registry", + ); + + expect(message).toContain("https://private.example/registry"); + expect(message).toContain("not the public registry"); + }); + + it("stays quiet about the registry when it is the default one", () => { + const message = describeInstallFailure( + new Error("fetch failed"), + "https://raw.githubusercontent.com/heygen-com/hyperframes/main/registry", + ); + + expect(message).not.toContain("not the public registry"); + }); + + it("leaves a non-transport failure exactly as it was", () => { + // An unsafe target or a malformed item is the caller's problem to read; a + // connectivity lecture there would send them to fix the wrong thing. + const message = describeInstallFailure(new Error('Unsafe target "../x"')); + + expect(message).toBe('Install failed: Unsafe target "../x"'); + }); +}); diff --git a/packages/cli/src/commands/add.ts b/packages/cli/src/commands/add.ts index e892065f6..fd6ed2aba 100644 --- a/packages/cli/src/commands/add.ts +++ b/packages/cli/src/commands/add.ts @@ -15,7 +15,7 @@ import { existsSync } from "node:fs"; import { resolve, relative } from "node:path"; import { ITEM_TYPE_DIRS, type RegistryItem } from "@hyperframes/core"; import { c } from "../ui/colors.js"; -import { installItem, resolveItemsByTag } from "../registry/index.js"; +import { DEFAULT_REGISTRY_URL, installItem, resolveItemsByTag } from "../registry/index.js"; import { resolveItemWithDependencies } from "../registry/resolver.js"; import { gateRegistryItemsCompatibility, @@ -140,6 +140,8 @@ export interface RunAddResult { installed: string[]; snippet: string; clipboardCopied: boolean; + /** Variable ids whose default was baked into an installed component. */ + variablesApplied: string[]; warnings: string[]; } @@ -181,22 +183,75 @@ async function installAll( destDir: string, baseUrl: string | undefined, force: boolean, -): Promise<{ written: string[]; preserved: string[] }> { + requestedName: string, + variableValues: Record | null, +): Promise<{ + written: string[]; + preserved: string[]; + variablesApplied: string[]; + variablesUnknown: string[]; + variablesInvalid: { id: string; reason: string }[]; +}> { const written: string[] = []; const preserved: string[] = []; + let variablesApplied: string[] = []; + let variablesUnknown: string[] = []; + let variablesInvalid: { id: string; reason: string }[] = []; try { for (const planItem of installPlan) { - const result = await installItem(planItem, { destDir, baseUrl, force }); + const result = await installItem(planItem, { + destDir, + baseUrl, + force, + // Only the item the user named. A dependency dragged in behind it never + // declared these variables and must not be rewritten by them. + variableValues: planItem.name === requestedName ? variableValues : null, + }); written.push(...result.written); preserved.push(...result.preserved); + if (planItem.name === requestedName) { + variablesApplied = result.variablesApplied; + variablesUnknown = result.variablesUnknown; + variablesInvalid = result.variablesInvalid; + } } } catch (err) { - throw new AddError( - `Install failed: ${err instanceof Error ? err.message : String(err)}`, - "install-failed", - ); + throw new AddError(describeInstallFailure(err, baseUrl), "install-failed"); } - return { written, preserved }; + return { written, preserved, variablesApplied, variablesUnknown, variablesInvalid }; +} + +/** + * Turn a transport failure into something a reader can act on. + * + * Item FILES are not cached (only manifests are), so a network blip surfaces + * here as node's bare `fetch failed` with no URL, no cause and no suggestion. + * That is what a user sees after copying a command off the catalog page, and + * it reads like the command was wrong rather than the network. + */ +export function describeInstallFailure(err: unknown, registry?: string): string { + const message = err instanceof Error ? err.message : String(err); + const cause = err instanceof Error && err.cause instanceof Error ? err.cause.message : ""; + const transport = + /fetch failed|ENOTFOUND|ECONNRESET|ECONNREFUSED|ETIMEDOUT|EAI_AGAIN|socket hang up|aborted/i; + if (!transport.test(`${message} ${cause}`)) return `Install failed: ${message}`; + + // Name the registry first. A project that set `registry` in hyperframes.json + // points at a private host, and when that host is down the failure has + // nothing to do with the user's connection -- telling them to check their + // network sends them to debug the one thing that is working. + const custom = + registry && !registry.startsWith(DEFAULT_REGISTRY_URL) + ? `\n This project's hyperframes.json sets registry to ${registry}, so that is the host ` + + "being contacted, not the public registry. If it is down or private, that is the failure." + : ""; + return ( + `Install failed: could not download the item's files.\n ${message}` + + "\n Item files are not cached, so every install fetches them. This is usually the " + + "registry host or the network rather than a bad command." + + custom + + "\n Retry, or set HTTPS_PROXY if you are behind a proxy." + ); } export async function runAdd(opts: RunAddArgs): Promise { @@ -244,12 +299,16 @@ export async function runAdd(opts: RunAddArgs): Promise { })); // 5. Install — dependencies first, requested item last. - const { written, preserved } = await installAll( - installPlan, - projectDir, - config.registry, - opts.force ?? false, - ); + const variableValues = parseVariableValues(opts.vars); + const { written, preserved, variablesApplied, variablesUnknown, variablesInvalid } = + await installAll( + installPlan, + projectDir, + config.registry, + opts.force ?? false, + item.name, + variableValues, + ); // Report what landed, not what was asked for: a failed install throws above, // and the bulk `add ` path re-enters here per item, so this one place @@ -269,9 +328,16 @@ export async function runAdd(opts: RunAddArgs): Promise { itemForInstall.files.find((f) => f.type === "hyperframes:composition") ?? itemForInstall.files[0]; const snippetTargetRel = primaryFile?.target ?? ""; - const snippet = buildSnippet(item, snippetTargetRel, parseVariableValues(opts.vars)); + const snippet = buildSnippet(item, snippetTargetRel, variableValues); const clipboardCopied = !opts.skipClipboard && snippet ? copyToClipboard(snippet) : false; + for (const { id, reason } of variablesInvalid) { + warnings.push(`--vars ${id} ignored: ${reason}`); + } + if (variablesUnknown.length > 0) { + warnings.push(`--vars ignored (not declared by ${item.name}): ${variablesUnknown.join(", ")}`); + } + return { ok: true, name: item.name, @@ -282,6 +348,7 @@ export async function runAdd(opts: RunAddArgs): Promise { installed: installPlan.map((planItem) => planItem.name), snippet, clipboardCopied, + variablesApplied, warnings, }; } @@ -374,6 +441,12 @@ export default defineCommand({ for (const file of result.written) { console.log(` ${c.dim(relative(projectDir, file))}`); } + if (result.variablesApplied.length > 0) { + // Say it out loud. A component's values are baked into the file rather + // than shown in the snippet, so without this the command looks + // identical whether --vars worked or was thrown away. + console.log(` ${c.dim(`variables applied: ${result.variablesApplied.join(", ")}`)}`); + } if (result.snippet) { console.log(""); console.log(c.dim("Include snippet:")); diff --git a/packages/cli/src/commands/transcribe.test.ts b/packages/cli/src/commands/transcribe.test.ts index 621210c90..5c4bd521d 100644 --- a/packages/cli/src/commands/transcribe.test.ts +++ b/packages/cli/src/commands/transcribe.test.ts @@ -48,8 +48,16 @@ describe("transcribe command", () => { it("explicit run exits non-zero and is NOT reported as a command failure", async () => { const { dir, input } = dummyAudio(); dirs.push(dir); - await transcribeCmd.run!({ args: { input, json: true, optional: false } } as never); + // Pin the engine. `auto` picks Parakeet whenever parakeet-mlx happens to be + // installed, and only the whisper path is mocked here -- so on those + // machines this test used to shell out to a real ASR binary, fail with + // "Parakeet did not produce output", and land in the generic failure branch + // instead of the soft-skip it is asserting. + await transcribeCmd.run!({ + args: { input, json: true, optional: false, engine: "whisper" }, + } as never); + expect(transcribeMock).toHaveBeenCalled(); expect(consumeCommandResult().exitCode).toBe(1); expect(trackTranscribeUnavailable).toHaveBeenCalledWith({ optional: false }); expect(trackCommandFailure).not.toHaveBeenCalled(); @@ -58,8 +66,14 @@ describe("transcribe command", () => { it("--optional skips cleanly with exit 0", async () => { const { dir, input } = dummyAudio(); dirs.push(dir); - await transcribeCmd.run!({ args: { input, json: true, optional: true } } as never); + await transcribeCmd.run!({ + args: { input, json: true, optional: true, engine: "whisper" }, + } as never); + // Asserting the mock ran is what keeps this honest: without it the test + // passes on a machine with no Parakeet and silently tests nothing on one + // that has it. + expect(transcribeMock).toHaveBeenCalled(); expect(consumeCommandResult().exitCode).toBe(0); expect(trackTranscribeUnavailable).toHaveBeenCalledWith({ optional: true }); expect(trackCommandFailure).not.toHaveBeenCalled(); diff --git a/packages/cli/src/registry/index.ts b/packages/cli/src/registry/index.ts index c7ca5f984..b81f5f12a 100644 --- a/packages/cli/src/registry/index.ts +++ b/packages/cli/src/registry/index.ts @@ -1,21 +1,8 @@ -export { - DEFAULT_REGISTRY_URL, - fetchRegistryManifest, - fetchItemManifest, - fetchItemFile, -} from "./remote.js"; +// Only what other modules actually import. Everything else in this folder is +// reached through its own module, so re-exporting it here just creates surface +// that has to be kept working without anyone depending on it. +export { DEFAULT_REGISTRY_URL } from "./remote.js"; -export { - listRegistryItems, - loadAllItems, - resolveItem, - resolveItemsByTag, - type ResolveOptions, -} from "./resolver.js"; +export { listRegistryItems, loadAllItems, resolveItemsByTag } from "./resolver.js"; -export { - installItem, - assertSafeTarget, - type InstallOptions, - type InstallResult, -} from "./installer.js"; +export { installItem } from "./installer.js"; diff --git a/packages/cli/src/registry/installer.ts b/packages/cli/src/registry/installer.ts index eb3d71f61..266439aae 100644 --- a/packages/cli/src/registry/installer.ts +++ b/packages/cli/src/registry/installer.ts @@ -11,6 +11,7 @@ import { existsSync, readFileSync, writeFileSync } from "node:fs"; import { resolve, relative, isAbsolute } from "node:path"; import type { FileTarget, RegistryItem } from "@hyperframes/core"; import { fetchItemFile, DEFAULT_REGISTRY_URL } from "./remote.js"; +import { applyVariableDefaults, type ApplyResult } from "./variableDefaults.js"; export interface InstallOptions { /** Project root where files land. Every target resolves relative to this. */ @@ -19,6 +20,12 @@ export interface InstallOptions { baseUrl?: string; /** Overwrite files the project has changed since they were installed. */ force?: boolean; + /** + * `--vars` values to bake into a COMPONENT's declared defaults. A block + * carries its values on the mount element instead, so this is ignored there: + * per-mount values are strictly better when a mount exists. + */ + variableValues?: Record | null; } export interface InstallResult { @@ -26,6 +33,11 @@ export interface InstallResult { written: string[]; /** Absolute paths left alone because the project had changed them. */ preserved: string[]; + /** Variable ids whose default was rewritten in an installed component. */ + variablesApplied: string[]; + /** Ids the item does not declare, and ids it declares but cannot accept. */ + variablesUnknown: string[]; + variablesInvalid: { id: string; reason: string }[]; } /** @@ -91,7 +103,7 @@ export function hasLocalEdits( * install time so a registry that bypasses schema validation still can't write * outside the project. */ -export function assertSafeTarget(destDir: string, target: string): void { +function assertSafeTarget(destDir: string, target: string): void { if (isAbsolute(target)) { throw new Error(`Unsafe target "${target}": absolute paths are not allowed.`); } @@ -108,6 +120,11 @@ export function assertSafeTarget(destDir: string, target: string): void { } } +/** A component's pasteable markup: the file whose declared defaults `--vars` edits. */ +function isInstalledComponentSnippet(item: RegistryItem, file: FileTarget): boolean { + return item.type === "hyperframes:component" && file.target.toLowerCase().endsWith(".html"); +} + function isInstalledRegistryBlockComposition(item: RegistryItem, file: FileTarget): boolean { return ( item.type === "hyperframes:block" && @@ -124,6 +141,61 @@ function addRegistryItemMarker(source: string, item: RegistryItem): string { return `\n${source}`; } +interface FileOutcome { + destPath: string; + target: string; + preserved: boolean; + hash: string | null; + vars: ApplyResult | null; +} + +/** Fetch, write and post-process one file. Extracted so installItem stays readable. */ +async function installOneFile( + item: RegistryItem, + file: FileTarget, + destDir: string, + baseUrl: string, + record: InstallRecord, + options: InstallOptions, +): Promise { + const destPath = resolve(destDir, file.target); + + // Decided before fetching rather than after: a file we are going to keep + // should never be overwritten and then put back, because a crash in + // between would lose it for real. + if ( + !options.force && + existsSync(destPath) && + hasLocalEdits(record, file.target, readFileSync(destPath)) + ) { + return { destPath, target: file.target, preserved: true, hash: null, vars: null }; + } + + await fetchItemFile(item, file, destPath, baseUrl); + if (isInstalledRegistryBlockComposition(item, file)) { + const source = readFileSync(destPath, "utf-8"); + writeFileSync(destPath, addRegistryItemMarker(source, item), "utf-8"); + } + // A component has no mount element to hang values on, so the chosen + // values go into its own declaration or they go nowhere. See + // variableDefaults.ts for why that is the only surviving home. + let vars: ApplyResult | null = null; + if (options.variableValues && isInstalledComponentSnippet(item, file)) { + const source = readFileSync(destPath, "utf-8"); + vars = applyVariableDefaults(source, options.variableValues); + if (vars.applied.length > 0) writeFileSync(destPath, vars.html, "utf-8"); + } + // Hash what actually landed, marker and baked defaults included, or the + // next install reads its own output as the project's edit. + return { + destPath, + target: file.target, + preserved: false, + hash: digest(readFileSync(destPath)), + vars, + }; +} + /** * Install a resolved `RegistryItem` into `destDir` by fetching each file in * parallel and writing it to its validated target path. @@ -143,34 +215,9 @@ export async function installItem( const record = readInstallRecord(destDir); const outcomes = await Promise.all( - item.files.map(async (file: FileTarget) => { - const destPath = resolve(destDir, file.target); - - // Decided before fetching rather than after: a file we are going to keep - // should never be overwritten and then put back, because a crash in - // between would lose it for real. - if ( - !options.force && - existsSync(destPath) && - hasLocalEdits(record, file.target, readFileSync(destPath)) - ) { - return { destPath, target: file.target, preserved: true, hash: null }; - } - - await fetchItemFile(item, file, destPath, baseUrl); - if (isInstalledRegistryBlockComposition(item, file)) { - const source = readFileSync(destPath, "utf-8"); - writeFileSync(destPath, addRegistryItemMarker(source, item), "utf-8"); - } - // Hash what actually landed, marker included, or the next install reads - // its own marker as the project's edit. - return { - destPath, - target: file.target, - preserved: false, - hash: digest(readFileSync(destPath)), - }; - }), + item.files.map((file: FileTarget) => + installOneFile(item, file, destDir, baseUrl, record, options), + ), ); const written = outcomes.filter((o) => !o.preserved).map((o) => o.destPath); @@ -183,5 +230,19 @@ export async function installItem( writeInstallRecord(destDir, record); } - return { written, preserved }; + const vars = outcomes.map((o) => o.vars).filter((v): v is ApplyResult => v !== null); + return { + written, + preserved, + variablesApplied: vars.flatMap((v) => v.applied), + // An id nothing declared is only genuinely unknown once every file has had + // a chance at it, so intersect rather than union. + variablesUnknown: vars.length + ? vars.reduce( + (acc, v) => acc.filter((id) => v.unknown.includes(id)), + vars[0]!.unknown, + ) + : [], + variablesInvalid: vars.flatMap((v) => v.invalid), + }; } diff --git a/packages/cli/src/registry/localSearch.test.ts b/packages/cli/src/registry/localSearch.test.ts index 9cd94f245..8d31029c0 100644 --- a/packages/cli/src/registry/localSearch.test.ts +++ b/packages/cli/src/registry/localSearch.test.ts @@ -183,3 +183,53 @@ describe("hasNoSearchableTokens", () => { expect(hasNoSearchableTokens("the and of !!!")).toBe(true); }); }); + +describe("the two spellings of a compound word find the same items", () => { + // Reduced from the real failure: `countdown` returned exactly one item (the + // only thing tagged with that spelling) while `count down timer` returned + // sixteen that did not include it. Whichever phrasing an author happened to + // type decided which half of the answer they saw. + const items = [ + named("yt-circle-pointer", "Circle Pointer", "An annotation overlay with a countdown chip."), + named("count-up", "Count Up", "A stat counter that eases between two values."), + named("decline-chart", "Decline Chart", "A line that counts down as its value falls."), + named("aurora-drift", "Aurora Drift", "A slow gradient background."), + ]; + const namesFor = (q: string) => searchByWords(q, items, fieldsOf).map((i) => i.name); + + it("finds the one-word item from the two-word query", () => { + expect(namesFor("count down timer")).toContain("yt-circle-pointer"); + }); + + it("finds the two-word items from the one-word query", () => { + // The half of the answer the compound spelling used to hide. + expect(namesFor("countdown")).toEqual(expect.arrayContaining(["count-up", "decline-chart"])); + }); + + it("returns the same set either way, which is the actual defect", () => { + expect(namesFor("countdown").sort()).toEqual(namesFor("count down").sort()); + }); + + it("still ranks the exact compound match first", () => { + // Splitting must not cost the item that spells it the way you asked. The + // compound is kept and is rare, so its weight survives the added halves. + expect(namesFor("countdown")[0]).toBe("yt-circle-pointer"); + }); + + it("leaves a word the catalog never uses alone", () => { + // `timer` appears in none of these items, and inventing a match for it + // would be widening the query into fiction rather than into phrasing. + expect(namesFor("timer")).toEqual([]); + }); + + it("does not let a split dislodge the item named for the whole word", () => { + // `typewriter` splits into `type` + `writer` if both are known. The item + // literally called typewriter must still win. + const typing = [ + named("typewriter", "Typewriter", "Character-by-character reveal."), + named("type-match-cut", "Type Match Cut", "A cut matched on a writer's type."), + ]; + + expect(searchByWords("typewriter", typing, fieldsOf)[0]?.name).toBe("typewriter"); + }); +}); diff --git a/packages/cli/src/registry/localSearch.ts b/packages/cli/src/registry/localSearch.ts index 7c94b9e7f..7e1f3148a 100644 --- a/packages/cli/src/registry/localSearch.ts +++ b/packages/cli/src/registry/localSearch.ts @@ -100,6 +100,71 @@ export interface ItemText { weak: string; } +/** + * Reconcile the two spellings of one compound word. + * + * The tokenizer splits on word boundaries, so `countdown` is one token and + * `count down` is two, and neither can ever match the other. That made the two + * spellings of a single idea return disjoint result sets: `countdown` returned + * only the one item tagged with that exact word, while `count down timer` + * returned sixteen that did not include it. Whichever phrasing an author + * happened to type decided which half of the answer they saw, and neither half + * was the whole answer. + * + * Both directions, and both gated on the catalog's own vocabulary so this can + * only ever add signal: + * + * - A query token is split when both halves are words the catalog actually + * uses. The compound is always kept, so nothing is lost: `countdown` is rare + * and keeps its high inverse-document-frequency weight, while the common + * halves it adds bring in the items written the other way and carry almost + * no weight of their own. That is why splitting `typewriter` into `type` and + * `writer` cannot dislodge the item literally called typewriter. + * - Adjacent query tokens are joined when the compound is a word the catalog + * actually uses, so `count down` also reaches items written `countdown`. + * + * A word in neither form, like `timer` (which appears in none of the catalog's + * items), is left exactly as it was: this widens phrasing, it does not invent + * matches. + * + * Everything added here is INFERRED rather than asked for, so it carries a + * fraction of a real token's weight. Without that the inference can outvote the + * question: `type` matching the name of `type-match-cut` at full strength beats + * `typewriter` matching the name of `typewriter`, and searching a word returns + * something that merely contains half of it. Relying on the halves being + * statistically common in a large catalog is not the same as making them + * count for less, and only one of the two holds when the corpus is small. + */ +const INFERRED_TOKEN_WEIGHT = 0.35; + +function expandCompounds( + want: Map, + order: string[], + vocabulary: Set, +): void { + const infer = (token: string): void => { + if (!want.has(token)) want.set(token, INFERRED_TOKEN_WEIGHT); + }; + + for (const token of order) { + if (token.length < 6) continue; + // Shortest useful part is 3 characters, matching the tokenizer's own floor. + for (let cut = 3; cut <= token.length - 3; cut++) { + const head = token.slice(0, cut); + const tail = token.slice(cut); + if (vocabulary.has(head) && vocabulary.has(tail)) { + infer(head); + infer(tail); + break; + } + } + } + for (let i = 0; i < order.length - 1; i++) { + const joined = `${order[i]}${order[i + 1]}`; + if (vocabulary.has(joined)) infer(joined); + } +} + /** * Rank every item by shared vocabulary, best first. * @@ -111,7 +176,10 @@ export function rankByWords( items: T[], textOf: (item: T) => ItemText, ): Scored[] { - const want = new Set(tokenize(query)); + const asked = tokenize(query); + // Token -> how much a match on it is worth. Asked-for words count fully; + // words inferred from a compound count for a fraction. + const want = new Map(asked.map((token) => [token, 1])); if (want.size === 0) return items.map((item) => ({ item, score: 0 })); const parsed = items.map((item) => { @@ -120,6 +188,10 @@ export function rankByWords( return { item, strongTokens, allTokens: new Set([...strongTokens, ...tokenize(weak)]) }; }); + const vocabulary = new Set(); + for (const entry of parsed) for (const token of entry.allTokens) vocabulary.add(token); + expandCompounds(want, asked, vocabulary); + // How rare each queried word is across the catalog. Without this a common // word carries the same weight as a distinctive one, and field weighting // makes that worse rather than better: searching "reveal a headline one line @@ -127,7 +199,7 @@ export function rankByWords( // strong hit on the catalog's most common word outscored several weak hits // on the words that actually narrowed it down. const idf = new Map(); - for (const token of want) { + for (const token of want.keys()) { const df = parsed.reduce((count, p) => count + (p.allTokens.has(token) ? 1 : 0), 0); // +1 inside the log keeps a token present in every item at a small // positive weight rather than exactly zero: still nearly worthless, but @@ -138,8 +210,8 @@ export function rankByWords( return parsed .map(({ item, strongTokens, allTokens }) => { let shared = 0; - for (const token of want) { - const weight = idf.get(token) ?? 1; + for (const [token, asking] of want) { + const weight = (idf.get(token) ?? 1) * asking; if (strongTokens.has(token)) shared += STRONG_FIELD_WEIGHT * weight; else if (allTokens.has(token)) shared += weight; } diff --git a/packages/cli/src/registry/remote.test.ts b/packages/cli/src/registry/remote.test.ts index a6b046caf..1fe299f23 100644 --- a/packages/cli/src/registry/remote.test.ts +++ b/packages/cli/src/registry/remote.test.ts @@ -12,8 +12,13 @@ vi.mock("node:os", async (importOriginal) => ({ homedir: () => scratchHome, })); -const { fetchItemManifest, fetchRegistryManifest, DEFAULT_REGISTRY_URL } = - await import("./remote.js"); +const { + describeCauseChain, + fetchItemFile, + fetchItemManifest, + fetchRegistryManifest, + DEFAULT_REGISTRY_URL, +} = await import("./remote.js"); const MANIFEST = { name: "hyperframes", items: [{ name: "count-up" }] }; const ITEM = { name: "count-up", type: "hyperframes:component", files: [] }; @@ -160,3 +165,103 @@ describe("fetchItemManifest", () => { ).rejects.toThrow("HTTP 404"); }); }); + +describe("describeCauseChain", () => { + it("surfaces the reason undici hides under cause", () => { + // The reported failure. `fetch failed` alone describes every network + // problem equally badly; the sentence that tells you what to do is one + // level down, and it was being dropped. + const err = new Error("fetch failed", { + cause: new Error("self-signed certificate in certificate chain"), + }); + + expect(describeCauseChain(err)).toBe( + "fetch failed (self-signed certificate in certificate chain)", + ); + }); + + it("includes an errno code when the message does not already carry it", () => { + const inner = Object.assign(new Error("getaddrinfo ENOTFOUND example.invalid"), { + code: "ENOTFOUND", + }); + + // The code is already in the text, so repeating it would be noise. + expect(describeCauseChain(new Error("fetch failed", { cause: inner }))).toBe( + "fetch failed (getaddrinfo ENOTFOUND example.invalid)", + ); + }); + + it("walks more than one level", () => { + const deep = new Error("a", { cause: new Error("b", { cause: new Error("c") }) }); + + expect(describeCauseChain(deep)).toBe("a (b; c)"); + }); + + it("survives a cause cycle rather than hanging", () => { + const a = new Error("a"); + const b = new Error("b", { cause: a }); + (a as { cause?: unknown }).cause = b; + + expect(describeCauseChain(a)).toBe("a (b)"); + }); + + it("returns the plain message when there is no cause", () => { + expect(describeCauseChain(new Error("HTTP 404"))).toBe("HTTP 404"); + }); +}); + +describe("fetchItemFile retries", () => { + const item = { name: "blur-in", type: "hyperframes:component" } as never; + const file = { path: "blur-in.html", target: "compositions/components/blur-in.html" } as never; + const dest = () => join(scratchHome, `dl-${Math.random().toString(36).slice(2)}.html`); + + it("recovers from a transient blip instead of failing the whole install", async () => { + // Item files are the one uncached path, so a single blip used to kill the + // command outright. Two cheap retries is a better trade than that. + const fetchSpy = vi + .spyOn(globalThis, "fetch") + .mockRejectedValueOnce(new Error("fetch failed", { cause: new Error("ECONNRESET") })) + .mockResolvedValueOnce({ + ok: true, + status: 200, + arrayBuffer: async () => new TextEncoder().encode("
ok
").buffer, + } as unknown as Response); + + await expect(fetchItemFile(item, file, dest(), DEFAULT_REGISTRY_URL)).resolves.toBeUndefined(); + expect(fetchSpy).toHaveBeenCalledTimes(2); + }); + + it("does not retry a certificate failure, which fails identically every time", async () => { + // The reported case: a private registry with a self-signed certificate. + // Retrying only makes the user wait three times as long for one answer. + const fetchSpy = vi.spyOn(globalThis, "fetch").mockRejectedValue( + new Error("fetch failed", { + cause: new Error("self-signed certificate in certificate chain"), + }), + ); + + await expect(fetchItemFile(item, file, dest(), DEFAULT_REGISTRY_URL)).rejects.toThrow( + /self-signed certificate/, + ); + expect(fetchSpy).toHaveBeenCalledTimes(1); + }); + + it("names the URL it could not reach", async () => { + vi.spyOn(globalThis, "fetch").mockRejectedValue( + new Error("fetch failed", { cause: new Error("self-signed certificate in chain") }), + ); + + await expect( + fetchItemFile(item, file, dest(), "https://private.example/registry"), + ).rejects.toThrow(/https:\/\/private\.example\/registry\/components\/blur-in\/blur-in\.html/); + }); + + it("gives up after a bounded number of attempts", async () => { + const fetchSpy = vi + .spyOn(globalThis, "fetch") + .mockRejectedValue(new Error("fetch failed", { cause: new Error("ECONNRESET") })); + + await expect(fetchItemFile(item, file, dest(), DEFAULT_REGISTRY_URL)).rejects.toThrow(); + expect(fetchSpy).toHaveBeenCalledTimes(3); + }); +}); diff --git a/packages/cli/src/registry/remote.ts b/packages/cli/src/registry/remote.ts index 6bc0f3b1c..76a760479 100644 --- a/packages/cli/src/registry/remote.ts +++ b/packages/cli/src/registry/remote.ts @@ -150,6 +150,72 @@ export async function fetchItemManifest( } } +/** + * Flatten an error and its `cause` chain into one readable line. + * + * `fetch failed` on its own is useless. `fetch failed (self-signed certificate + * in certificate chain)` tells the reader exactly which knob to turn, and that + * string only exists one or two levels down the chain. + */ +export function describeCauseChain(err: unknown): string { + const parts: string[] = []; + let current: unknown = err; + const seen = new Set(); + while (current instanceof Error && !seen.has(current)) { + seen.add(current); + const code = (current as { code?: unknown }).code; + const text = + code && !current.message.includes(String(code)) + ? `${current.message} [${String(code)}]` + : current.message; + if (text && !parts.includes(text)) parts.push(text); + current = current.cause; + } + if (parts.length === 0) return String(err); + const [head, ...rest] = parts; + return rest.length ? `${head} (${rest.join("; ")})` : head!; +} + +/** + * A transient failure worth trying again, as opposed to a settled answer. + * + * A refused connection, a reset socket or a DNS hiccup usually clears on the + * next attempt. A TLS failure does not: a self-signed certificate on a private + * registry fails identically every time, and retrying it only makes the user + * wait three times as long for the same message. + */ +function isRetryableTransport(err: unknown): boolean { + const text = describeCauseChain(err).toLowerCase(); + if (/certificate|self-signed|self signed|unable to verify|altname|ssl|tls/.test(text)) { + return false; + } + return /fetch failed|econnreset|econnrefused|etimedout|eai_again|socket hang up|timeouterror|aborted|network/.test( + text, + ); +} + +/** + * Item files are the one uncached path: manifests fall back to a stale copy, + * but every install downloads its files fresh. That made a single blip fatal to + * the whole command, which is a bad trade for two extra attempts costing under + * a second when the network is healthy. + */ +async function fetchWithRetry(url: string, attempts = 3): Promise { + let lastErr: unknown; + for (let attempt = 1; attempt <= attempts; attempt++) { + try { + return await fetch(url, { signal: AbortSignal.timeout(FETCH_TIMEOUT_MS) }); + } catch (err) { + lastErr = err; + if (attempt === attempts || !isRetryableTransport(err)) break; + // Short, bounded backoff. Long enough to clear a blip, short enough that + // a genuinely offline machine still fails promptly. + await new Promise((resolve) => setTimeout(resolve, 150 * attempt)); + } + } + throw lastErr; +} + /** * Download a single file referenced by an item to a local destination. * Caller is responsible for target-path validation (see installer.ts). @@ -165,7 +231,17 @@ export async function fetchItemFile( throw new Error(`Unsafe file.path "${file.path}": path segments may not contain "..".`); } const url = `${baseUrl}/${ITEM_TYPE_DIRS[item.type]}/${item.name}/${file.path}`; - const res = await fetch(url, { signal: AbortSignal.timeout(FETCH_TIMEOUT_MS) }); + let res: Response; + try { + res = await fetchWithRetry(url); + } catch (err) { + // undici throws a bare "fetch failed" and buries the real reason in + // `cause`, sometimes a level deeper again. That is where the diagnosis + // lives: a self-signed certificate on a private registry, a DNS failure, a + // refused connection. Without it the message is two words that describe + // every possible network problem equally badly. + throw new Error(`File fetch failed: ${url} — ${describeCauseChain(err)}`, { cause: err }); + } if (!res.ok) { throw new Error(`File fetch failed: ${url} — HTTP ${res.status}`); } diff --git a/packages/cli/src/registry/resolver.test.ts b/packages/cli/src/registry/resolver.test.ts index e9b7d46a7..d8f6b2611 100644 --- a/packages/cli/src/registry/resolver.test.ts +++ b/packages/cli/src/registry/resolver.test.ts @@ -5,6 +5,7 @@ import { loadAllItems, resolveItem, resolveItemWithDependencies, + unreachableRegistryMessage, } from "./resolver.js"; const MANIFEST: RegistryManifest = { @@ -205,3 +206,30 @@ describe("registry resolver", () => { }); }); }); + +describe("unreachableRegistryMessage", () => { + it("names a private registry, so the reader looks at the right host", () => { + // Same dead end the item-file failure used to be: without the host, a + // project that set `registry` in hyperframes.json reads this as the public + // catalog having lost the item. + const message = unreachableRegistryMessage("blur-in", "https://private.example/registry"); + + expect(message).toContain("https://private.example/registry"); + expect(message).toContain("hyperframes.json"); + }); + + it("stays quiet when the registry is the public one", () => { + const message = unreachableRegistryMessage( + "blur-in", + "https://raw.githubusercontent.com/heygen-com/hyperframes/main/registry", + ); + + expect(message).toBe('Item "blur-in" not found \u2014 registry unreachable or empty.'); + }); + + it("stays quiet when no registry was supplied at all", () => { + expect(unreachableRegistryMessage("blur-in")).toBe( + 'Item "blur-in" not found \u2014 registry unreachable or empty.', + ); + }); +}); diff --git a/packages/cli/src/registry/resolver.ts b/packages/cli/src/registry/resolver.ts index 8ba98749a..a41502198 100644 --- a/packages/cli/src/registry/resolver.ts +++ b/packages/cli/src/registry/resolver.ts @@ -91,11 +91,25 @@ export async function resolveItem( } const item = items[items.length - 1]; if (!item) { - throw new Error(`Item "${name}" not found — registry unreachable or empty.`); + throw new Error(unreachableRegistryMessage(name, options.baseUrl)); } return item; } +/** + * "registry unreachable or empty" without saying WHICH registry is the same + * dead end the item-file failure used to be: a project that sets `registry` in + * hyperframes.json reads it as the public catalog having lost the item, and + * goes looking in the wrong place. Naming the host is the diagnosis. + */ +export function unreachableRegistryMessage(name: string, baseUrl?: string): string { + const where = + baseUrl && !baseUrl.startsWith(DEFAULT_REGISTRY_URL) + ? ` Contacted ${baseUrl}, set by this project's hyperframes.json, not the public registry.` + : ""; + return `Item "${name}" not found — registry unreachable or empty.${where}`; +} + /** * Resolve an item and all of its transitive `registryDependencies` in * topological order — dependencies first, the requested item last — so callers @@ -122,7 +136,7 @@ export async function resolveItemWithDependencies( throw new Error( available.length > 0 ? `Item "${name}" not found in registry. Available: ${available}` - : `Item "${name}" not found — registry unreachable or empty.`, + : unreachableRegistryMessage(name, options.baseUrl), ); } @@ -146,7 +160,7 @@ export async function resolveItemWithDependencies( throw new Error( available.length > 0 ? `Dependency "${itemName}" not found in registry. Available: ${available}` - : `Dependency "${itemName}" not found — registry unreachable or empty.`, + : unreachableRegistryMessage(itemName, options.baseUrl), ); } diff --git a/packages/cli/src/registry/variableDefaults.test.ts b/packages/cli/src/registry/variableDefaults.test.ts new file mode 100644 index 000000000..04b07b155 --- /dev/null +++ b/packages/cli/src/registry/variableDefaults.test.ts @@ -0,0 +1,108 @@ +import { describe, expect, it } from "vitest"; + +import { applyVariableDefaults } from "./variableDefaults.js"; + +/** The shape the registry actually ships: single-quoted attribute, JSON inside. */ +const COMPONENT = `
+ Design +
`; + +function defaultOf(html: string, id: string): unknown { + const raw = html.match(/data-composition-variables='([\s\S]*?)'/)![1]!; + const decl = JSON.parse(raw.replace(/'/g, "'")) as { id: string; default: unknown }[]; + return decl.find((d) => d.id === id)!.default; +} + +describe("applyVariableDefaults", () => { + it("rewrites the declared default so a pasted component carries the chosen value", () => { + // The whole point: a component has no mount element, so this is the only + // place a value picked on the catalog page can survive being pasted. + const r = applyVariableDefaults(COMPONENT, { size: 76, tone: "accent" }); + + expect(r.applied.sort()).toEqual(["size", "tone"]); + expect(defaultOf(r.html, "size")).toBe(76); + expect(defaultOf(r.html, "tone")).toBe("accent"); + }); + + it("leaves untouched variables at their shipped defaults", () => { + const r = applyVariableDefaults(COMPONENT, { size: 76 }); + + expect(defaultOf(r.html, "tone")).toBe("strong"); + }); + + it("reports an id the item does not declare instead of silently dropping it", () => { + const r = applyVariableDefaults(COMPONENT, { nope: 1 }); + + expect(r.unknown).toEqual(["nope"]); + expect(r.applied).toEqual([]); + expect(r.html).toBe(COMPONENT); + }); + + it("refuses an enum value outside the declared options", () => { + // Writing it would produce a file that renders as if the value were + // ignored, because the composition's own guard falls back to the default. + const r = applyVariableDefaults(COMPONENT, { tone: "chartreuse" }); + + expect(r.invalid).toEqual([{ id: "tone", reason: "not one of strong, muted, accent" }]); + expect(defaultOf(r.html, "tone")).toBe("strong"); + }); + + it("refuses a number outside its declared range", () => { + expect(applyVariableDefaults(COMPONENT, { size: 9999 }).invalid).toEqual([ + { id: "size", reason: "above max 120" }, + ]); + expect(applyVariableDefaults(COMPONENT, { size: 1 }).invalid).toEqual([ + { id: "size", reason: "below min 24" }, + ]); + }); + + it("coerces a numeric string, because a URL and a form both produce one", () => { + // The catalog page puts values in the query string, where every value is a + // string. Writing "76" where the composition expects a number would make + // the guard fall back and look like the value was ignored. + const r = applyVariableDefaults(COMPONENT, { size: "76" }); + + expect(r.applied).toEqual(["size"]); + expect(defaultOf(r.html, "size")).toBe(76); + }); + + it("escapes a value containing the attribute's own delimiter", () => { + const withText = COMPONENT.replace( + '{ "id": "size", "type": "number", "role": "style", "label": "Size", "default": 52, "min": 24, "max": 120 }', + '{ "id": "label", "type": "string", "label": "Label", "default": "hi" }', + ); + const r = applyVariableDefaults(withText, { label: "it's fine" }); + + expect(r.applied).toEqual(["label"]); + // A raw apostrophe would close the attribute early and break the markup. + expect(r.html).not.toMatch(/data-composition-variables='[^']*it's/); + expect(defaultOf(r.html, "label")).toBe("it's fine"); + }); + + it("does nothing when the item declares no variables at all", () => { + const plain = "
no declaration here
"; + + expect(applyVariableDefaults(plain, { size: 1 })).toEqual({ + html: plain, + applied: [], + unknown: ["size"], + invalid: [], + }); + }); + + it("refuses to rewrite a declaration it cannot parse", () => { + const broken = `
x
`; + + expect(applyVariableDefaults(broken, { id: 1 }).html).toBe(broken); + }); + + it("is a no-op for an empty value set", () => { + expect(applyVariableDefaults(COMPONENT, {}).html).toBe(COMPONENT); + }); +}); diff --git a/packages/cli/src/registry/variableDefaults.ts b/packages/cli/src/registry/variableDefaults.ts new file mode 100644 index 000000000..6b1b631af --- /dev/null +++ b/packages/cli/src/registry/variableDefaults.ts @@ -0,0 +1,151 @@ +/** + * Bake chosen variable values into an installed item's declared defaults. + * + * A block is mounted by a `
`, so `add --vars` can put + * the values on that mount as `data-variable-values` and two mounts of the same + * block can differ. A component has no mount element: it is markup you paste + * into a host composition, and it reads its values through + * `__hyperframes.getVariables()`, which merges the declared defaults of every + * `[data-composition-variables]` element in the document with render-time + * overrides. + * + * So for a component the only place a chosen value can live and survive being + * pasted is the component's own declaration. Rewriting the defaults there is + * what makes "customise it on the catalog page, copy the command, run it" end + * with the look you picked. Before this, `--vars` was accepted, documented, and + * silently discarded for every component in the catalog. + */ + +import { isCompositionVariable, type CompositionVariable } from "@hyperframes/core/variables"; + +export interface ApplyResult { + /** The source with defaults rewritten. Unchanged when nothing applied. */ + html: string; + /** Variable ids whose default was replaced. */ + applied: string[]; + /** Ids the item does not declare. */ + unknown: string[]; + /** Ids declared but given a value the declaration does not allow. */ + invalid: { id: string; reason: string }[]; +} + +const ATTR = "data-composition-variables"; + +/** Locate the attribute's quoted value, tolerating either delimiter. */ +function findDeclaration(source: string): { start: number; end: number; raw: string } | null { + const at = source.indexOf(`${ATTR}=`); + if (at === -1) return null; + const quote = source[at + ATTR.length + 1]; + if (quote !== "'" && quote !== '"') return null; + const start = at + ATTR.length + 2; + const end = source.indexOf(quote, start); + if (end === -1) return null; + return { start, end, raw: source.slice(start, end) }; +} + +function decode(raw: string): string { + return raw.replace(/'/g, "'").replace(/"/g, '"'); +} + +/** Mirrors the escaping the block-mount path uses, so either delimiter is safe. */ +function encode(json: string, quote: string): string { + return quote === "'" ? json.replace(/'/g, "'") : json.replace(/"/g, """); +} + +function optionValues(decl: CompositionVariable): string[] | null { + return decl.type === "enum" ? decl.options.map((option) => option.value) : null; +} + +/** + * Reject a value the declaration cannot represent, rather than writing it. + * + * A bad enum falls back to the default at runtime and warns, so writing one + * here would produce a file that renders as if the value had been ignored -- + * which is the exact failure this function exists to remove. + */ +function rejectEnum(decl: CompositionVariable, value: unknown): string | null { + const options = optionValues(decl); + if (!options) return null; + return options.includes(String(value)) ? null : `not one of ${options.join(", ")}`; +} + +function rejectNumber(decl: CompositionVariable, value: unknown): string | null { + if (decl.type !== "number") return null; + const n = typeof value === "number" ? value : Number(value); + if (!Number.isFinite(n)) return "not a number"; + if (decl.min !== undefined && n < decl.min) return `below min ${decl.min}`; + if (decl.max !== undefined && n > decl.max) return `above max ${decl.max}`; + return null; +} + +function reject(decl: CompositionVariable, value: unknown): string | null { + return rejectEnum(decl, value) ?? rejectNumber(decl, value); +} + +export function applyVariableDefaults( + source: string, + values: Record, +): ApplyResult { + const ids = Object.keys(values); + if (ids.length === 0) return { html: source, applied: [], unknown: [], invalid: [] }; + + const found = findDeclaration(source); + if (!found) return { html: source, applied: [], unknown: ids, invalid: [] }; + + // isCompositionVariable is the predicate parseCompositionVariables filters + // with -- the schema's own definition of a well-formed declaration. Using it + // here means everything below works on a real discriminated union instead of + // a bag of `unknown` re-checked at each use, and a declaration the schema + // rejects is one we must not rewrite, because we would be guessing at its + // shape. parseCompositionVariables itself takes a DOM Element, which the CLI + // has no business constructing to read a string. + let parsed: unknown; + try { + parsed = JSON.parse(decode(found.raw)); + } catch { + return { html: source, applied: [], unknown: ids, invalid: [] }; + } + if (!Array.isArray(parsed)) return { html: source, applied: [], unknown: ids, invalid: [] }; + const declared: CompositionVariable[] = parsed.filter(isCompositionVariable); + if (declared.length !== parsed.length) { + // Rewriting a partially understood declaration would drop the entries we + // could not model, so leave the file exactly as the registry shipped it. + return { html: source, applied: [], unknown: ids, invalid: [] }; + } + + const applied: string[] = []; + const invalid: { id: string; reason: string }[] = []; + const byId = new Map(declared.map((decl) => [decl.id, decl])); + const updated = new Map(); + + for (const [id, value] of Object.entries(values)) { + const decl = byId.get(id); + if (!decl) continue; + const reason = reject(decl, value); + if (reason) { + invalid.push({ id, reason }); + continue; + } + // The declaration's own type decides how the value is stored. A number + // written as the string "76" would trip the composition's guard and fall + // back, which looks exactly like the value being ignored. + updated.set(id, decl.type === "number" ? Number(value) : String(value)); + applied.push(id); + } + + const unknown = ids.filter((id) => !byId.has(id)); + if (applied.length === 0) return { html: source, applied, unknown, invalid }; + + // One declaration per line, matching how the registry authors these files, so + // a re-install produces a readable diff rather than one enormous line. + const quote = source[found.start - 1]!; + const body = declared + .map((decl) => { + const next = updated.has(decl.id) ? { ...decl, default: updated.get(decl.id)! } : decl; + return ` ${JSON.stringify(next)}`; + }) + .join(",\n"); + const rewritten = encode(`[\n${body}\n ]`, quote); + const html = source.slice(0, found.start) + rewritten + source.slice(found.end); + return { html, applied, unknown, invalid }; +} diff --git a/packages/player/src/composition-probe.ts b/packages/player/src/composition-probe.ts index b7fdfc0fd..b3c500b10 100644 --- a/packages/player/src/composition-probe.ts +++ b/packages/player/src/composition-probe.ts @@ -19,19 +19,9 @@ import { isRuntimeDurationAdapter, } from "./timeline-adapters.js"; -declare const __HYPERFRAMES_RUNTIME_CDN_URL__: string; +import { RUNTIME_CDN_URL, runtimeCdnUrlForVersion } from "./runtime-url.js"; -export function runtimeCdnUrlForVersion(version: string): string { - if (!/^\d+\.\d+\.\d+(?:-[0-9A-Za-z.-]+)?(?:\+[0-9A-Za-z.-]+)?$/.test(version)) { - throw new Error(`Invalid HyperFrames runtime version: ${version}`); - } - return `https://cdn.jsdelivr.net/npm/@hyperframes/core@${version}/dist/hyperframe.runtime.iife.js`; -} - -const RUNTIME_CDN_URL = - typeof __HYPERFRAMES_RUNTIME_CDN_URL__ === "string" - ? __HYPERFRAMES_RUNTIME_CDN_URL__ - : runtimeCdnUrlForVersion("0.0.0-dev"); +export { runtimeCdnUrlForVersion }; export interface ProbeResult { duration: number; diff --git a/packages/player/src/hyperframes-player.test.ts b/packages/player/src/hyperframes-player.test.ts index f065f0978..ae853eb8e 100644 --- a/packages/player/src/hyperframes-player.test.ts +++ b/packages/player/src/hyperframes-player.test.ts @@ -1473,7 +1473,11 @@ describe("HyperframesPlayer srcdoc attribute", () => { player.setAttribute("srcdoc", html); document.body.appendChild(player); - expect(player.iframe.getAttribute("srcdoc")).toBe(html); + // Not byte-identical: srcdoc now also carries the runtime, injected ahead + // of body scripts so a pasted component can read its variables during + // parse. The composition itself must still arrive intact. + expect(player.iframe.getAttribute("srcdoc")).toContain("hello"); + expect(player.iframe.getAttribute("srcdoc")).toContain("hyperframe.runtime.iife.js"); player.remove(); }); @@ -1487,7 +1491,8 @@ describe("HyperframesPlayer srcdoc attribute", () => { const html = "after connect"; player.setAttribute("srcdoc", html); - expect(player.iframe.getAttribute("srcdoc")).toBe(html); + expect(player.iframe.getAttribute("srcdoc")).toContain("after connect"); + expect(player.iframe.getAttribute("srcdoc")).toContain("hyperframe.runtime.iife.js"); player.remove(); }); @@ -1548,7 +1553,9 @@ describe("HyperframesPlayer srcdoc attribute", () => { document.body.appendChild(player); expect(player.iframe.getAttribute("src")).toBe("/api/projects/foo/preview"); - expect(player.iframe.getAttribute("srcdoc")).toBe(""); + // srcdoc carries the runtime now; what matters here is that both + // attributes are present so the browser can arbitrate. + expect(player.iframe.getAttribute("srcdoc")).toContain(""); player.remove(); }); diff --git a/packages/player/src/runtime-in-srcdoc.test.ts b/packages/player/src/runtime-in-srcdoc.test.ts new file mode 100644 index 000000000..ca923d949 --- /dev/null +++ b/packages/player/src/runtime-in-srcdoc.test.ts @@ -0,0 +1,71 @@ +import { describe, expect, it } from "vitest"; + +import { ensureRuntimeBeforeBodyScripts } from "./runtime-in-srcdoc.js"; + +const URL = "https://cdn.example/hyperframe.runtime.iife.js"; + +/** What a pasted component looks like: it reads its variables during parse. */ +const COMPONENT_PAGE = `t +
+ +`; + +describe("ensureRuntimeBeforeBodyScripts", () => { + it("puts the runtime ahead of the script that reads variables", () => { + // The whole bug: the probe appended the runtime after load, so this guard + // always took the empty branch and the component used its hardcoded + // defaults. badge-pop with count 10 rendered 3. + const out = ensureRuntimeBeforeBodyScripts(COMPONENT_PAGE, URL); + + expect(out).toContain(``); + expect(out.indexOf(URL)).toBeLessThan(out.indexOf("getVariables")); + }); + + it("puts it inside head, where an external script blocks the parser", () => { + const out = ensureRuntimeBeforeBodyScripts(COMPONENT_PAGE, URL); + + // Landing after would not block body parsing in the same way. + expect(out.indexOf(URL)).toBeGreaterThan(out.indexOf("")); + expect(out.indexOf(URL)).toBeLessThan(out.indexOf("")); + }); + + it("does not add a second copy when the page already links it", () => { + const already = ``; + + expect(ensureRuntimeBeforeBodyScripts(already, URL)).toBe(already); + }); + + it("leaves a CLI-rendered page alone, which inlines the runtime already", () => { + // The engine inlines it and defines the global on the way in. A second + // copy would re-initialise the runtime underneath a live composition. + const rendered = ``; + + expect(ensureRuntimeBeforeBodyScripts(rendered, URL)).toBe(rendered); + }); + + it("falls back to before when there is no head", () => { + const out = ensureRuntimeBeforeBodyScripts("", URL); + + expect(out.indexOf(URL)).toBeLessThan(out.indexOf("")); + }); + + it("handles a bare fragment by going first", () => { + const out = ensureRuntimeBeforeBodyScripts("
hi
", URL); + + expect(out.startsWith(``)).toBe(true); + }); + + it("is a no-op on empty input", () => { + expect(ensureRuntimeBeforeBodyScripts("", URL)).toBe(""); + }); + + it("survives a head tag carrying attributes", () => { + const out = ensureRuntimeBeforeBodyScripts( + ``, + URL, + ); + + expect(out.indexOf(URL)).toBeLessThan(out.indexOf("read()")); + }); +}); diff --git a/packages/player/src/runtime-in-srcdoc.ts b/packages/player/src/runtime-in-srcdoc.ts new file mode 100644 index 000000000..5027c8b6c --- /dev/null +++ b/packages/player/src/runtime-in-srcdoc.ts @@ -0,0 +1,54 @@ +/** + * Put the runtime in the document's head, before anything in the body runs. + * + * The probe injects the runtime by appending a ``; + const head = /]*>/i.exec(html); + if (head) { + const at = head.index + head[0].length; + return html.slice(0, at) + tag + html.slice(at); + } + // No head: get in before so body scripts still see the runtime. A + // fragment with neither lands at the front, which is the same guarantee. + const body = /]*>/i.exec(html); + if (body) return html.slice(0, body.index) + tag + html.slice(body.index); + const htmlTag = /]*>/i.exec(html); + if (htmlTag) { + const at = htmlTag.index + htmlTag[0].length; + return html.slice(0, at) + tag + html.slice(at); + } + return tag + html; +} diff --git a/packages/player/src/runtime-url.ts b/packages/player/src/runtime-url.ts new file mode 100644 index 000000000..7191647e1 --- /dev/null +++ b/packages/player/src/runtime-url.ts @@ -0,0 +1,19 @@ +/** + * Where the runtime comes from. + * + * Split out of composition-probe so the probe's late injection and the srcdoc's + * parse-time injection cannot drift onto different URLs. + */ +declare const __HYPERFRAMES_RUNTIME_CDN_URL__: string; + +export function runtimeCdnUrlForVersion(version: string): string { + if (!/^\d+\.\d+\.\d+(?:-[0-9A-Za-z.-]+)?(?:\+[0-9A-Za-z.-]+)?$/.test(version)) { + throw new Error(`Invalid HyperFrames runtime version: ${version}`); + } + return `https://cdn.jsdelivr.net/npm/@hyperframes/core@${version}/dist/hyperframe.runtime.iife.js`; +} + +export const RUNTIME_CDN_URL = + typeof __HYPERFRAMES_RUNTIME_CDN_URL__ === "string" + ? __HYPERFRAMES_RUNTIME_CDN_URL__ + : runtimeCdnUrlForVersion("0.0.0-dev"); diff --git a/packages/player/src/shader-options.ts b/packages/player/src/shader-options.ts index 1b59bc786..791f7a6da 100644 --- a/packages/player/src/shader-options.ts +++ b/packages/player/src/shader-options.ts @@ -4,6 +4,9 @@ * URLs and srcdoc HTML. */ +import { ensureRuntimeBeforeBodyScripts } from "./runtime-in-srcdoc.js"; +import { RUNTIME_CDN_URL } from "./runtime-url.js"; + export const SHADER_CAPTURE_SCALE_ATTR = "shader-capture-scale"; export const SHADER_LOADING_ATTR = "shader-loading"; const SHADER_CAPTURE_SCALE_PARAM = "__hf_shader_capture_scale"; @@ -141,9 +144,15 @@ export function prepareSrcForElement(el: Element, src: string): string { } export function prepareSrcdocForElement(el: Element, srcdoc: string): string { - return injectShaderOptionsIntoSrcdoc( - srcdoc, - normalizeShaderCaptureScale(el.getAttribute(SHADER_CAPTURE_SCALE_ATTR)), - getShaderModeFromElement(el), + // Runtime first, and in the head: a component's inline script reads its + // variables while the body is parsing, long before the probe's own injection + // could land. See runtime-in-srcdoc.ts. + return ensureRuntimeBeforeBodyScripts( + injectShaderOptionsIntoSrcdoc( + srcdoc, + normalizeShaderCaptureScale(el.getAttribute(SHADER_CAPTURE_SCALE_ATTR)), + getShaderModeFromElement(el), + ), + RUNTIME_CDN_URL, ); }