Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
111 changes: 111 additions & 0 deletions IMPROVEMENT_OPPORTUNITIES.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
# Improvement Opportunities (Repo Scan)

This is a quick, code-level opportunity scan based on static inspection. It is intentionally practical: each item includes why it matters and where to start.

## 1) Implement transitive registry dependency resolution (CLI)

**Opportunity**
The registry resolver explicitly documents that transitive dependency resolution is not implemented yet.

**Why this matters**
As soon as blocks/components start depending on other registry items, install/add flows can become incomplete or order-dependent.

**Suggested next step**
Implement a dependency graph walk (`registryDependencies`) with cycle detection and topological sort, and return a deterministic install list.

**Evidence**
- `packages/cli/src/registry/resolver.ts` contains an explicit TODO for transitive walk + topo ordering.

## 2) Reduce monolithic file size in high-churn areas

**Opportunity**
Several files are very large (roughly 800–1,600 LOC), especially around runtime, rendering, and Studio UI orchestration.

**Why this matters**
Large files increase review risk and regression probability, and they make ownership boundaries unclear.

**Suggested next step**
Define soft limits (for example, 400–600 LOC per module), then incrementally extract feature-focused modules (state selectors, side-effect services, view helpers, protocol adapters).

**Evidence (largest files found)**
- `packages/core/src/runtime/init.ts` (~1632 LOC)
- `packages/producer/src/services/renderOrchestrator.ts` (~1267 LOC)
- `packages/producer/src/services/htmlCompiler.ts` (~1150 LOC)
- `packages/studio/src/App.tsx` (~951 LOC)
- `packages/studio/src/components/editor/FileTree.tsx` (~944 LOC)

## 3) Pay down `any`/unsafe cast usage in production paths

**Opportunity**
There are multiple `as any` and `: any` usages in runtime paths (not only tests), especially around browser page evaluation and CDP hooks.

**Why this matters**
This weakens type contracts at boundary seams where failures are expensive (capture/render/snapshot pipelines).

**Suggested next step**
Create small boundary types (e.g., typed `window` facades and CDP payload interfaces), then enforce a gradual reduction target per package.

**Evidence (sample production locations)**
- `packages/cli/src/commands/snapshot.ts`
- `packages/cli/src/server/studioServer.ts`
- `packages/cli/src/capture/animationCataloger.ts`
- `packages/engine/src/utils/urlDownloader.ts`
- `packages/producer/src/services/renderOrchestrator.ts`

## 4) Add bounded concurrency + retry strategy for registry item loading

**Opportunity**
`loadAllItems` currently uses unbounded `Promise.allSettled` across all entries.

**Why this matters**
If registry size grows, this can create bursty outbound load and noisy partial failures under transient network pressure.

**Suggested next step**
Use bounded concurrency (e.g., 8–16 workers) and retry/backoff for retryable HTTP failures; keep warning behavior for hard failures.

**Evidence**
- `packages/cli/src/registry/resolver.ts` loads all manifests in parallel via `Promise.allSettled(entries.map(...))`.

## 5) Revisit very large generated font payload strategy

**Opportunity**
A generated source file embeds a ~964 KB base64 payload directly in TypeScript.

**Why this matters**
Large generated literals can slow diffs/indexing and inflate package/bundle processing.

**Suggested next step**
Consider moving large assets to external artifacts with checksums/versioning, or split by font family/weight for selective loading.

**Evidence**
- `packages/producer/src/services/fontData.generated.ts` is a large generated payload file (~964 KB).

## 6) Align lint suppression comments with current lint stack

**Opportunity**
Many files include `eslint-disable` comments while repo standards/tools center around `oxlint`/`oxfmt`.

**Why this matters**
Suppression comments that do not map to active linters create confusion and can hide true intent.

**Suggested next step**
Audit suppression comments and either:
1. Replace with active-linter suppressions where needed, or
2. Refactor code so suppressions are unnecessary.

**Evidence (sample locations)**
- `packages/studio/src/App.tsx`
- `packages/studio/src/components/nle/NLELayout.tsx`
- `packages/studio/src/player/hooks/useTimelinePlayer.ts`
- `packages/engine/src/utils/urlDownloader.ts`

---

## Suggested execution order

1. Registry dependency resolver (correctness blocker for scalable catalog installs).
2. Bounded concurrency/retries in registry loading (reliability).
3. Type boundary hardening (`any` reductions) in CLI/engine/producer pathways.
4. Monolithic file decomposition in Studio + producer + core runtime.
5. Generated font payload strategy update.
6. Lint-suppression cleanup.
22 changes: 21 additions & 1 deletion packages/cli/src/commands/add.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ const MANIFEST: RegistryManifest = {
homepage: "https://example.com",
items: [
{ name: "my-block", type: "hyperframes:block" },
{ name: "base-component", type: "hyperframes:component" },
{ name: "my-component", type: "hyperframes:component" },
{ name: "my-example", type: "hyperframes:example" },
],
Expand Down Expand Up @@ -41,6 +42,7 @@ const COMPONENT_ITEM: RegistryItem = {
type: "hyperframes:component",
title: "My Component",
description: "Component for tests",
registryDependencies: ["base-component"],
files: [
{
path: "my-component.html",
Expand All @@ -55,6 +57,21 @@ const COMPONENT_ITEM: RegistryItem = {
],
};

const BASE_COMPONENT_ITEM: RegistryItem = {
$schema: "https://hyperframes.heygen.com/schema/registry-item.json",
name: "base-component",
type: "hyperframes:component",
title: "Base Component",
description: "Base component dependency for tests",
files: [
{
path: "base-component.css",
target: "compositions/components/base-component/base-component.css",
type: "hyperframes:style",
},
],
};

const EXAMPLE_ITEM: RegistryItem = {
$schema: "https://hyperframes.heygen.com/schema/registry-item.json",
name: "my-example",
Expand All @@ -68,6 +85,7 @@ const EXAMPLE_ITEM: RegistryItem = {

const ITEM_BY_NAME: Record<string, RegistryItem> = {
"my-block": BLOCK_ITEM,
"base-component": BASE_COMPONENT_ITEM,
"my-component": COMPONENT_ITEM,
"my-example": EXAMPLE_ITEM,
};
Expand Down Expand Up @@ -206,7 +224,9 @@ describe("runAdd (integration, mocked registry)", () => {
projectDir: dir,
skipClipboard: true,
});
expect(result.written.length).toBe(2);
expect(result.written.length).toBe(3);
expect(result.installed).toEqual(["base-component", "my-component"]);
expect(existsSync(join(dir, "src/fx/base-component/base-component.css"))).toBe(true);
expect(existsSync(join(dir, "src/fx/my-component/my-component.html"))).toBe(true);
expect(existsSync(join(dir, "src/fx/my-component/my-component.css"))).toBe(true);
expect(result.snippet).toContain("src/fx/my-component/my-component.html");
Expand Down
45 changes: 28 additions & 17 deletions packages/cli/src/commands/add.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,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, resolveItem } from "../registry/index.js";
import { installItem, resolveItemWithDependencies } from "../registry/index.js";
import {
DEFAULT_PROJECT_CONFIG,
loadProjectConfig,
Expand Down Expand Up @@ -81,6 +81,7 @@ export interface RunAddResult {
type: RegistryItem["type"];
typeDir: string;
written: string[];
installed: string[];
snippet: string;
clipboardCopied: boolean;
}
Expand All @@ -106,13 +107,17 @@ export async function runAdd(opts: RunAddArgs): Promise<RunAddResult> {
config = DEFAULT_PROJECT_CONFIG;
}

// 2. Resolve the item from the registry.
let item: RegistryItem;
// 2. Resolve the item (and transitive dependencies) from the registry.
let resolved: RegistryItem[];
try {
item = await resolveItem(opts.name, { baseUrl: config.registry });
resolved = await resolveItemWithDependencies(opts.name, { baseUrl: config.registry });
} catch (err) {
throw new AddError(err instanceof Error ? err.message : String(err), "unknown-item");
}
const item = resolved[resolved.length - 1];
if (!item) {
throw new AddError(`Item "${opts.name}" not found — registry unreachable or empty.`, "unknown-item");
}

if (item.type === "hyperframes:example") {
throw new AddError(
Expand All @@ -122,20 +127,24 @@ export async function runAdd(opts: RunAddArgs): Promise<RunAddResult> {
}

// 3. Remap targets per project config.
const remappedFiles = item.files.map((f) => ({
...f,
target: remapTarget(item, f.target, config.paths),
const installPlan = resolved.map((resolvedItem) => ({
...resolvedItem,
files: resolvedItem.files.map((f) => ({
...f,
target: remapTarget(resolvedItem, f.target, config.paths),
})),
}));
const itemForInstall: RegistryItem = { ...item, files: remappedFiles };

// 4. Install — the installer validates every target before any write.
let written: string[];
let written: string[] = [];
try {
const result = await installItem(itemForInstall, {
destDir: projectDir,
baseUrl: config.registry,
});
written = result.written;
for (const resolvedItem of installPlan) {
const result = await installItem(resolvedItem, {
Comment on lines +141 to +142
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Block example items from transitive add installs

runAdd rejects hyperframes:example only for the requested item, but this loop installs every resolved dependency without type checks. Because registryDependencies are name-only, a component/block can depend on an example and add will then write example targets (including files like index.html) into an existing project, which can overwrite user content. Add a guard that rejects example-type dependencies before installation.

Useful? React with 👍 / 👎.

destDir: projectDir,
baseUrl: config.registry,
});
written.push(...result.written);
}
} catch (err) {
throw new AddError(
`Install failed: ${err instanceof Error ? err.message : String(err)}`,
Expand All @@ -144,10 +153,11 @@ export async function runAdd(opts: RunAddArgs): Promise<RunAddResult> {
}

// 5. Build include snippet + clipboard copy.
const installTarget = installPlan[installPlan.length - 1] ?? item;
const primaryFile =
itemForInstall.files.find((f) => f.type === "hyperframes:snippet") ??
itemForInstall.files.find((f) => f.type === "hyperframes:composition") ??
itemForInstall.files[0];
installTarget.files.find((f) => f.type === "hyperframes:snippet") ??
installTarget.files.find((f) => f.type === "hyperframes:composition") ??
installTarget.files[0];
const snippetTargetRel = primaryFile?.target ?? "";
const snippet = buildSnippet(item, snippetTargetRel);
const clipboardCopied = !opts.skipClipboard && snippet ? copyToClipboard(snippet) : false;
Expand All @@ -158,6 +168,7 @@ export async function runAdd(opts: RunAddArgs): Promise<RunAddResult> {
type: item.type,
typeDir: ITEM_TYPE_DIRS[item.type],
written,
installed: installPlan.map((resolvedItem) => resolvedItem.name),
snippet,
clipboardCopied,
};
Expand Down
8 changes: 7 additions & 1 deletion packages/cli/src/registry/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,13 @@ export {
fetchItemFile,
} from "./remote.js";

export { listRegistryItems, loadAllItems, resolveItem, type ResolveOptions } from "./resolver.js";
export {
listRegistryItems,
loadAllItems,
resolveItem,
resolveItemWithDependencies,
type ResolveOptions,
} from "./resolver.js";

export {
installItem,
Expand Down
54 changes: 50 additions & 4 deletions packages/cli/src/registry/resolver.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
import { describe, expect, it, vi, beforeEach, afterEach } from "vitest";
import type { RegistryItem, RegistryManifest } from "@hyperframes/core";
import { listRegistryItems, loadAllItems, resolveItem } from "./resolver.js";
import {
listRegistryItems,
loadAllItems,
resolveItem,
resolveItemWithDependencies,
} from "./resolver.js";

const MANIFEST: RegistryManifest = {
$schema: "https://hyperframes.heygen.com/schema/registry.json",
Expand All @@ -13,6 +18,11 @@ const MANIFEST: RegistryManifest = {
],
};

const DEPENDENCIES: Record<string, string[] | undefined> = {
beta: ["alpha"],
gamma: ["beta"],
};

function buildItem(name: string, type: "hyperframes:example" | "hyperframes:block"): RegistryItem {
if (type === "hyperframes:example") {
return {
Expand All @@ -22,6 +32,7 @@ function buildItem(name: string, type: "hyperframes:example" | "hyperframes:bloc
description: `${name} desc`,
dimensions: { width: 1920, height: 1080 },
duration: 10,
registryDependencies: DEPENDENCIES[name],
files: [{ path: "index.html", target: "index.html", type: "hyperframes:composition" }],
};
}
Expand All @@ -32,6 +43,7 @@ function buildItem(name: string, type: "hyperframes:example" | "hyperframes:bloc
description: `${name} desc`,
dimensions: { width: 1080, height: 1350 },
duration: 6,
registryDependencies: DEPENDENCIES[name],
files: [
{
path: `${name}.html`,
Expand All @@ -42,7 +54,13 @@ function buildItem(name: string, type: "hyperframes:example" | "hyperframes:bloc
};
}

function mockFetch(overrides: Record<string, unknown> = {}): void {
function mockFetch(
overrides: {
registryFails?: boolean;
missing?: string[];
dependencies?: Record<string, string[] | undefined>;
} = {},
): void {
vi.stubGlobal(
"fetch",
vi.fn(async (urlInput: string | URL) => {
Expand All @@ -51,9 +69,13 @@ function mockFetch(overrides: Record<string, unknown> = {}): void {
return new Response(JSON.stringify(MANIFEST), { status: 200 });
}
const m = /\/(examples|blocks|components)\/([^/]+)\/registry-item\.json$/.exec(url);
if (m && !(overrides.missing as string[] | undefined)?.includes(m[2]!)) {
if (m && !overrides.missing?.includes(m[2]!)) {
const type = m[1] === "examples" ? "hyperframes:example" : "hyperframes:block";
return new Response(JSON.stringify(buildItem(m[2]!, type)), { status: 200 });
const item = buildItem(m[2]!, type);
if (overrides.dependencies && item.name in overrides.dependencies) {
item.registryDependencies = overrides.dependencies[item.name];
}
return new Response(JSON.stringify(item), { status: 200 });
}
return new Response("not found", { status: 404 });
}),
Expand Down Expand Up @@ -140,4 +162,28 @@ describe("registry resolver", () => {
await expect(resolveItem("alpha", { baseUrl })).rejects.toThrow(/unreachable/);
});
});

describe("resolveItemWithDependencies", () => {
it("returns dependencies first, then the requested item", async () => {
const baseUrl = uniqueBaseUrl();
const items = await resolveItemWithDependencies("gamma", { baseUrl });
expect(items.map((item) => item.name)).toEqual(["alpha", "beta", "gamma"]);
});

it("throws when a transitive dependency is missing from the registry", async () => {
mockFetch({ dependencies: { beta: ["does-not-exist"], gamma: ["beta"] } });
const baseUrl = uniqueBaseUrl();
await expect(resolveItemWithDependencies("gamma", { baseUrl })).rejects.toThrow(
/Dependency "does-not-exist" not found in registry/,
);
});

it("throws a clear cycle error for circular dependencies", async () => {
mockFetch({ dependencies: { alpha: ["gamma"], beta: ["alpha"], gamma: ["beta"] } });
const baseUrl = uniqueBaseUrl();
await expect(resolveItemWithDependencies("gamma", { baseUrl })).rejects.toThrow(
/Circular registryDependencies detected/,
);
});
});
});
Loading