fix(cli): the block snippet from add carries the block's own composition id (#4432)

* fix(cli): the snippet hyperframes add prints for a block passes hyperframes check

* fix(cli): the block snippet carries the id the block's own root declares

* fix(cli): warn when an added block declares no composition id for its snippet

* fix(cli): add <tag> --json reports each block's warnings
This commit is contained in:
Miguel Ángel
2026-09-24 17:39:13 -04:00
committed by GitHub
parent 7129340ae8
commit 359106aa3a
2 changed files with 86 additions and 11 deletions
+48 -5
View File
@@ -3,13 +3,17 @@ 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 { lintHyperframeHtml } from "@hyperframes/lint";
import type { RunAddResult } from "./add.js";
import {
AddError,
buildSnippet,
compositionRootId,
describeInstallFailure,
parseVariableValues,
remapTarget,
runAdd,
tagAddJson,
} from "./add.js";
import { trackRegistryItemAdded } from "../telemetry/events.js";
@@ -158,6 +162,12 @@ const ITEM_BY_NAME: Record<string, RegistryItem> = {
const DEP_BLOCK_HTML = `<div data-composition-variables='[{ "id": "maths", "type": "boolean", "label": "Maths", "default": false }]'></div>`;
const FILE_BODIES: Record<string, string> = {
"dep-block.html": DEP_BLOCK_HTML,
"deprecated-block.html": `<div data-composition-id="deprecated-block"></div>`,
"my-block.html": `<div data-composition-id="my-block-root" data-width="1080" data-height="1350"></div>`,
};
function mockFetch(): void {
vi.stubGlobal(
"fetch",
@@ -173,11 +183,8 @@ function mockFetch(): void {
}
// File fetch — match `/<type-dir>/<name>/<rest>` and serve synthetic content.
const f = /\/(examples|blocks|components)\/([^/]+)\/(.+)$/.exec(url);
if (f?.[3] === "dep-block.html") {
return new Response(DEP_BLOCK_HTML, { status: 200 });
}
if (f) {
return new Response(`/* ${f[3]} */\n`, { status: 200 });
return new Response(FILE_BODIES[f[3]!] ?? `/* ${f[3]} */\n`, { status: 200 });
}
return new Response("not found", { status: 404 });
}),
@@ -255,6 +262,26 @@ describe("add command pure helpers", () => {
expect(snip).toContain('data-duration="6"');
});
it("reads a composition's root id, inside its <template> when it has one", () => {
expect(compositionRootId(`<div data-composition-id="plain" data-width="1"></div>`)).toBe(
"plain",
);
expect(
compositionRootId(
`<html><head><template id="t"><div data-composition-id="templated"></div></template></head><body></body></html>`,
),
).toBe("templated");
expect(compositionRootId(`<div>no root</div>`)).toBeUndefined();
});
it("gives the block host the composition id that check requires", async () => {
const snip = buildSnippet(BLOCK_ITEM, "compositions/my-block.html", null, "my-block-root");
const html = `<!doctype html><html><body><div data-composition-id="root" data-width="1080" data-height="1350">${snip}</div></body></html>`;
const { findings } = await lintHyperframeHtml(html);
expect(findings.map((f) => f.code)).not.toContain("host_missing_composition_id");
expect(snip).toContain('data-composition-id="my-block-root"');
});
it("emits a paste hint for components", () => {
const snip = buildSnippet(COMPONENT_ITEM, "src/fx/my-component/my-component.html");
expect(snip).toContain("paste from");
@@ -292,8 +319,9 @@ describe("runAdd (integration, mocked registry)", () => {
expect(existsSync(join(dir, "compositions/my-block.html"))).toBe(true);
const installed = readFileSync(join(dir, "compositions/my-block.html"), "utf-8");
expect(installed).toContain("<!-- hyperframes-registry-item: my-block -->");
expect(installed).toContain("my-block.html");
expect(installed).toContain('data-composition-id="my-block-root"');
expect(result.snippet).toContain("compositions/my-block.html");
expect(result.snippet).toContain('data-composition-id="my-block-root"');
} finally {
rmSync(dir, { recursive: true, force: true });
}
@@ -392,6 +420,9 @@ describe("runAdd (integration, mocked registry)", () => {
expect(existsSync(join(dir, "compositions/dep-block.html"))).toBe(true);
// Snippet points at the requested block, not the dependency.
expect(result.snippet).toContain("compositions/dep-block.html");
expect(result.warnings).toEqual([
expect.stringContaining("compositions/dep-block.html declares no data-composition-id"),
]);
} finally {
rmSync(dir, { recursive: true, force: true });
}
@@ -531,3 +562,15 @@ describe("describeInstallFailure", () => {
expect(message).toBe('Install failed: Unsafe target "../x"');
});
});
describe("tagAddJson", () => {
it("carries every installed item's warnings", () => {
const result = (name: string, warnings: string[]) => ({ name, warnings }) as RunAddResult;
expect(tagAddJson("lower-thirds", [result("a", []), result("b", ["no id"])])).toEqual({
ok: true,
tag: "lower-thirds",
installed: ["a", "b"],
warnings: ["b: no id"],
});
});
});
+38 -6
View File
@@ -11,7 +11,8 @@ export const examples: Example[] = [
["Skip the clipboard copy (CI/headless)", "hyperframes add shader-wipe --no-clipboard"],
];
import { existsSync } from "node:fs";
import { existsSync, readFileSync } from "node:fs";
import { parseHTML } from "linkedom";
import { resolve, relative } from "node:path";
import { ITEM_TYPE_DIRS, type RegistryItem } from "@hyperframes/core";
import { c } from "../ui/colors.js";
@@ -103,10 +104,24 @@ function primaryInstalledTarget(item: RegistryItem): string {
return primary?.target ?? "";
}
/** The id a composition file's root declares (inside its `<template>` when templated); a host must carry the same one. */
export function compositionRootId(html: string): string | undefined {
const { document } = parseHTML(html);
const content = document.querySelector("template")?.content ?? document;
return (
content.querySelector("[data-composition-id]")?.getAttribute("data-composition-id") ?? undefined
);
}
function installedRootId(file: string): string | undefined {
return existsSync(file) ? compositionRootId(readFileSync(file, "utf-8")) : undefined;
}
export function buildSnippet(
item: RegistryItem,
relativeTarget: string,
values: Record<string, unknown> | null = null,
compositionId?: string,
): string {
if (item.type === "hyperframes:block") {
// data-start omitted — adjust to your timeline position after pasting.
@@ -115,7 +130,10 @@ export function buildSnippet(
? ` data-width="${item.dimensions.width}" data-height="${item.dimensions.height}"`
: "";
const vars = variableValuesAttribute(values);
return `<div data-composition-src="${relativeTarget}" data-duration="${item.duration}"${dims}${vars}></div>`;
const id = compositionId
? ` data-composition-id="${compositionId.replace(/"/g, "&quot;")}"`
: "";
return `<div${id} data-composition-src="${relativeTarget}" data-duration="${item.duration}"${dims}${vars}></div>`;
}
if (item.type === "hyperframes:component") {
return `<!-- paste from ${relativeTarget} into your composition -->`;
@@ -123,6 +141,16 @@ export function buildSnippet(
return "";
}
/** `add <tag> --json` output: each item's warnings, prefixed with its name. */
export function tagAddJson(tag: string, results: RunAddResult[]) {
return {
ok: true,
tag,
installed: results.map((r) => r.name),
warnings: results.flatMap((r) => r.warnings.map((w) => `${r.name}: ${w}`)),
};
}
/** `--vars` is JSON an agent or the catalog page produced; a malformed one is
* worth a clear error rather than a snippet that silently drops the values. */
export function parseVariableValues(raw: string | undefined): Record<string, unknown> | null {
@@ -365,7 +393,13 @@ export async function runAdd(opts: RunAddArgs): Promise<RunAddResult> {
// 6. Build include snippet + clipboard copy for the requested item.
const itemForInstall = installPlan[installPlan.length - 1]!;
const snippetTargetRel = primaryInstalledTarget(itemForInstall);
const snippet = buildSnippet(item, snippetTargetRel, variableValues);
const compositionId = installedRootId(resolve(projectDir, snippetTargetRel));
if (item.type === "hyperframes:block" && !compositionId) {
warnings.push(
`${snippetTargetRel} declares no data-composition-id, so the snippet has none; give its root one and put the same id on the snippet`,
);
}
const snippet = buildSnippet(item, snippetTargetRel, variableValues, compositionId);
const clipboardCopied = !opts.skipClipboard && snippet ? copyToClipboard(snippet) : false;
if (variablesUnknown.length > 0) {
@@ -553,9 +587,7 @@ export default defineCommand({
}
if (json) {
console.log(
JSON.stringify({ ok: true, tag: args.name, installed: results.map((r) => r.name) }),
);
console.log(JSON.stringify(tagAddJson(args.name, results)));
} else {
console.log("");
console.log(`${c.success("✓")} Installed ${results.length}/${items.length} blocks`);