fix(core): harden runtime resolution + inline prebuilt constant#457
Conversation
jrusso1020
left a comment
There was a problem hiding this comment.
Verdict: approve
Clean execution of the design we discussed, split into three self-contained commits.
(2A) Guard is at the right layer now. buildHyperframesRuntimeScript gates on existsSync(entryPath) and returns null before touching esbuild — eliminating the stderr output class (not just suppressing it). Signature change to string | null propagates correctly: all callers handle null — loadHyperframeRuntimeSource loses its non-null guarantee and passes through, the dev-only test scripts (test-hyperframe-runtime-behavior.ts, test-hyperframe-runtime-contract.ts, engine's test-fitTextFontSize-browser.ts) add explicit assert(x !== null, ...) because they run in the dev workspace where entry.ts must exist, and build-hyperframes-runtime-artifact.ts throws loudly if source is missing (which would mean the build is misconfigured — correct place to fail).
(2B) Inline constant is the right shape. The build script now emits src/generated/runtime-inline.ts containing RUNTIME_IIFE as a JSON.stringify-escaped string literal — handles quotes, newlines, and any unicode in the IIFE safely. getHyperframeRuntimeScript() returns it verbatim: no file I/O, no esbuild, no import.meta.url arithmetic. Exported from src/index.ts, so consumers (including the CLI bundled via tsup's noExternal) will inline the constant directly into their bundle — which is the whole point.
Build order flip is necessary, not cosmetic. "build": "bun run build:hyperframes-runtime && tsc" — artifact gen has to run first so src/generated/runtime-inline.ts exists before tsc tries to compile + emit types for it. The artifact script uses bun run which handles TS imports at runtime, so it doesn't depend on tsc having run yet — no circular dep.
CLI consumer chain:
buildFromSource (dev only) → entry.ts via esbuild
getInlinedRuntime (production) → baked-in constant
readPrebuiltArtifact (fallback) → IIFE file on disk
Strategy 3 is effectively redundant in production once strategy 2 is reliable, but keeping it as a safety net is fine — costs nothing, covers the "old @hyperframes/core without getHyperframeRuntimeScript" transition case during rollout.
Matches the design I flagged earlier — loadHyperframeRuntimeSource stays dev-only (reaches for entry.ts), getHyperframeRuntimeScript is the new production default, consumers don't need to know the difference.
Two non-blockers:
-
Fresh-clone DX footgun.
src/generated/is gitignored, butsrc/index.tsre-exportsgetHyperframeRuntimeScriptfrom./generated/runtime-inline. On a fresh clone,tsc --noEmit(editor typecheck, IDE integration, or any CI step that runs beforebun run build) will fail withCannot find module './generated/runtime-inline'until the artifact script runs. Easy fix later: add apostinstallscript ("postinstall": "bun run build:hyperframes-runtime") inpackages/core/package.jsonsobun installalways leaves the workspace in a typecheck-clean state. Not required today sincebun run buildruns the artifact step beforetsc— but the first engineer who opens the repo in an editor without running build first will hit this. -
Tarball bloat (trivial).
@hyperframes/core's publisheddist/now ships both the IIFE as a file AND the IIFE inlined intodist/generated/runtime-inline.js. ~hundreds of KB of duplicated JS text. Fine — the DX win trades favorably.
Typecheck/Test/Lint/Windows jobs are still in-flight as I review; if any of them trip (specifically Typecheck, given the generated-file arithmetic above), worth double-checking that build:hyperframes-runtime runs before typecheck in every CI path. Local passes (bun run build, 514 core tests, 161 CLI tests, oxlint clean) are strong priors that it's wired correctly.
Ship it.
Review by hyperframes
buildHyperframesRuntimeScript() now checks existsSync(entryPath) before calling esbuild.buildSync(). When entry.ts doesn't exist (bundled or published contexts where only dist/ ships), it returns null instead of letting esbuild fail with stderr output. This prevents any future consumer that bundles @hyperframes/core from rediscovering the "esbuild can't find entry.ts" bug. Updated loadHyperframeRuntimeSource() and all call-site test scripts to handle the nullable return type.
The build script now generates src/generated/runtime-inline.ts during build:hyperframes-runtime, containing the IIFE as a string constant. tsc then compiles it into dist/generated/runtime-inline.js. getHyperframeRuntimeScript() is the production-safe path: no esbuild, no file I/O, no import.meta.url arithmetic. It is exported from the @hyperframes/core package index. Build order changed from "tsc && build:hyperframes-runtime" to "build:hyperframes-runtime && tsc" so the generated file exists before tsc runs.
loadRuntimeSource() now has three resolution strategies: 1. esbuild from source (dev only) 2. Inlined constant via getHyperframeRuntimeScript() (production) 3. Pre-built IIFE artifact file (final fallback) The inlined constant path avoids file I/O entirely, making runtime resolution work reliably in bundled/published contexts.
8134ef8 to
2df143d
Compare
## Summary Adds a CI smoke test that reproduces the exact failure from #452 — `hyperframes preview` printing `✘ [ERROR] Could not resolve "…/runtime/entry.ts"` when installed globally via npm. The job simulates what a real user does: 1. `npm pack` the CLI → install globally with `--prefix` 2. `hyperframes init test-project --example blank` 3. `hyperframes preview --port 3099` in background 4. `curl http://localhost:3099/api/runtime.js` — assert non-empty JS 5. Assert stderr has no `✘ [ERROR]` or `Failed to load runtime` Also adds `.github/workflows/**` to the change detection filter so CI jobs run when workflow files change. ## Validation: the test catches the broken state Verified locally that the grep pattern fires on the pre-#452 code and passes on the fixed code: **Broken (0.4.15-alpha.1, globally installed via `npm i -g hyperframes@alpha`):** ``` $ hyperframes preview --port 3098 2>/tmp/hf-stderr.log $ grep -E '✘ \[ERROR\]|Failed to load runtime' /tmp/hf-stderr.log ✘ [ERROR] Could not resolve "/opt/homebrew/lib/node_modules/hyperframes/runtime/entry.ts" [studio] Failed to load runtime source fallback: Error: Build failed with 1 error: … → CAUGHT: assertion fires, test fails ✓ ``` **Fixed (built from #452 fix branch):** ``` $ node packages/cli/dist/cli.js preview --port 3098 2>/tmp/hf-stderr.log $ grep -E '✘ \[ERROR\]|Failed to load runtime' /tmp/hf-stderr.log (empty) → PASS: no errors ✓ ``` If this smoke test had existed before #452, it would have caught the bug before it shipped. ## Test plan - [x] Grep pattern catches broken state — verified locally against pre-#452 global install - [x] Fixed state passes — verified locally against #452-fixed build - [x] CI job runs green — https://github.com/heygen-com/hyperframes/actions/runs/24854025990/job/72762086753
340ce52
into
fix/runtime-fallback-resolution
Summary
Eliminates the "source tree assumed co-located with the bundle" failure mode from
@hyperframes/coreso future bundler-consumers don't rediscover the #452 bug. Three atomic commits:1. Guard in core (2A):
buildHyperframesRuntimeScript()now checksexistsSync(entryPath)before callingesbuild.buildSync(). Returnsnullwhenentry.tsis missing instead of crashing with stderr output. Any consumer that bundles core is safe without needing their own guard.2. Inline prebuilt constant (2B): The build script now generates
src/generated/runtime-inline.tscontaining the IIFE as a string constant. New exportgetHyperframeRuntimeScript()returns this constant — no esbuild, no file I/O, noimport.meta.urlarithmetic. This is the production-safe default path.3. CLI consumer update:
loadRuntimeSource()resolution chain is now:buildFromSource()— esbuild fromentry.ts(dev only)getInlinedRuntime()— baked-in constant viagetHyperframeRuntimeScript()(production)readPrebuiltArtifact()— reads IIFE file fromdist/(fallback)The split follows the design Rames called out:
loadHyperframeRuntimeSourcestays as the dev-only helper that reaches forentry.ts, andgetHyperframeRuntimeScript()is the new production path. Consumers don't need to know the difference.Design
The generated file
src/generated/runtime-inline.tsis:scripts/build-hyperframes-runtime-artifact.tsduringbun run builddist/generated/runtime-inline.js"files": ["dist"]Test plan
bun run build— passes (build order changed:build:hyperframes-runtime && tscso generated file exists before tsc)bun run --cwd packages/core test— 514 tests passbun run --cwd packages/cli test— 161 tests passnpm pack --dry-runconfirmsdist/generated/runtime-inline.jsships in the published package