fix: ship lottieReadiness + guard studio import.meta.env for non-Vite consumers#861
Conversation
b506dc7 to
623f948
Compare
jrusso1020
left a comment
There was a problem hiding this comment.
LGTM — two surgical fixes with sharp root-cause analysis.
Audited
Fix 1: lottieReadiness move (src/runtime/adapters/ → src/)
The root-cause framing is precise: packages/core/tsconfig.json excludes src/runtime/ (browser-context files bundled separately into the IIFE), and lottieReadiness.ts was a pure helper sitting under that excluded path. Nothing in the included tree imported it, so tsc never emitted it to dist/runtime/adapters/lottieReadiness.js — silently missing from the published tarball, even though the subpath export claimed to ship it. Consumers (studio's Player.tsx + the hyperframes-internal demo-next app) hit module-resolution failures at runtime.
Move is structurally correct: pure helper (unknown → boolean, no DOM/window) doesn't belong under src/runtime/. Moving it out gets it back into the standard library build path.
Backward compat preserved: the subpath export name ./runtime/lottie-readiness stays unchanged — only the underlying file path in the exports map flips from ./dist/runtime/adapters/lottieReadiness.js to ./dist/lottieReadiness.js. Existing consumers don't need a code change. ✓
Path enum on the file move:
- ✓
package.jsonexports./runtime/lottie-readiness(dev path) updated - ✓
package.jsonpublishConfig.exports./runtime/lottie-readinessupdated - ✓ Internal re-export at
packages/core/src/runtime/adapters/lottie.ts:3updated (./lottieReadiness→../../lottieReadiness) - Test colocated with source — moved from
src/runtime/adapters/lottieReadiness.test.tstosrc/lottieReadiness.test.ts✓
The Typecheck + Build CI checks green on this commit confirm no other internal imports point at the old path. (Per the grep-every-implementation discipline I'd want to verify with git grep "runtime/adapters/lottieReadiness" returns zero matches, but TS resolution covers this.)
Fix 2: import.meta.env guard for non-Vite consumers (studio/manualEditingAvailability.ts)
const env = (import.meta.env ?? {}) as StudioFeatureFlagEnv;Direct property access on import.meta.env is important — Vite's compile-time replacement transform pattern-matches the import.meta.env.SOMETHING shape statically and inlines values at build time. Wrapping behind typeof import.meta !== "undefined" ? ... would defeat that optimization (the hyperframes-internal#328 inline patch did the over-defensive shape, but ESM guarantees import.meta is defined, so the simpler form here is correct).
The 5-line explanatory comment is excellent — names the specific affected hosts (Next.js/Turbopack, Node, jest), explains the {} fallback semantics ("every flag resolves to its declared default outside Vite"), and notes the direct-access requirement for Vite's transform. Future maintainers won't undo this defensively wrapping it. ✓
Body claim verification
- "missing from the published 0.6.6 and 0.6.7 tarballs" — verified (this is exactly the bug that drove the hyperframes-internal#328 patch)
- "
packages/core/tsconfig.jsonexcludessrc/runtime" — the fix's correctness depends on this; CI Build green implies the exclusion is real - "
lottieReadiness.tsis a pure helper — takesunknown, returnsboolean, no DOM/windowdependencies" — supported by the move working with a simple path rewrite (no module-side imports broken) - "Vite's compile-time transform happy" — direct property access preserved ✓
CI
Test: runtime contract, Typecheck, Lint, Build, Format, Preflight (lint+format), Detect changes all green on this commit. CodeQL neutral (no new findings). Test, CLI smoke (required), Smoke: global install, Preview parity, Tests/Render on windows-latest, Perf:*, regression-shards all in-progress. Miguel already APPROVED.
Non-blocking observations
-
Simpler guard shape vs the hyperframes-internal#328 patch — the inline patch I reviewed there used
((typeof import.meta !== "undefined" ? (import.meta as { env?: unknown }).env : undefined) ?? {}) as StudioFeatureFlagEnv;. The hf#861 version is cleaner and still correct (ESM guaranteesimport.metais defined). Once this lands andhyperframes-internalbumps to 0.6.8, the inline patches can be dropped per the README tracking note. -
Test plan has two unchecked items — "Publish 0.6.8 and verify the tarball contains
dist/lottieReadiness.js" and "Verify a non-Vite ESM consumer (e.g. a Next.js / Turbopack app) imports@hyperframes/studiowithoutimport.meta.enverrors". The first is post-merge; the second could be exercised with thehyperframes-internal/demo-nextapp this PR will unblock. Worth checking before publishing 0.6.8. -
packages/core/tsconfig.jsonitself is unchanged — the fix sidesteps the runtime-exclude issue by moving the file, but doesn't address the broader pattern that "pure helpers must not be placed undersrc/runtime/." Worth a comment intsconfig.jsonexplaining the exclusion's purpose, or a CI check that scans forsrc/runtime/*.tsfiles that don't referencewindow/DOM/documentand warns. Future-proofing.
— Review by Rames Jusso (pr-review)
The "./runtime/lottie-readiness" subpath export claimed to point at
./dist/runtime/adapters/lottieReadiness.js, but the published 0.6.6 and
0.6.7 tarballs never contained that file. Reason: packages/core/tsconfig.json
excludes "src/runtime" (those files run in a browser context and are
bundled separately into the IIFE artifact), so tsc never emitted the
compiled output. Consumers that import the subpath — most notably
@hyperframes/studio's Player.tsx — fail to resolve the module and the
build breaks at the consumer site.
lottieReadiness.ts is a pure helper: takes `unknown`, returns `boolean`,
no DOM or `window` access. It doesn't belong under src/runtime/ in the
first place — that's why the runtime-exclude rule rightly caught it.
Move it to src/lottieReadiness.ts so the standard library build picks
it up. The subpath export name stays "./runtime/lottie-readiness" so
existing consumers (studio) don't need a code change; only the exports
map's underlying file path moves to ./dist/lottieReadiness.{js,d.ts}.
- mv src/runtime/adapters/lottieReadiness.{ts,test.ts} -> src/
- update src/runtime/adapters/lottie.ts re-export path
- update package.json + publishConfig.exports to point at new dist path
Verified: full core test suite passes (862/862), dist now contains
lottieReadiness.{js,d.ts}, studio typechecks against the moved file.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
manualEditingAvailability.ts unconditionally reads `import.meta.env`. That's a Vite-only extension; in plain ESM hosts (Next.js / Turbopack, Node, jest in some configs) `import.meta` exists but `import.meta.env` is `undefined`. Reading any property off undefined throws at module evaluation time, so the studio fails to load the moment a non-Vite host imports anything from `@hyperframes/studio`. Guard the read so the module is loadable everywhere; outside Vite, every flag falls back to its declared default, preserving Vite behavior. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
623f948 to
4426ffa
Compare
Summary
Two small fixes that together make
@hyperframes/core+@hyperframes/studioconsumable from non-Vite hosts (Next.js / Turbopack, Node, etc.).1.
core: ship the missinglottieReadinessmoduleThe
"./runtime/lottie-readiness"subpath export in@hyperframes/coreclaims to ship at./dist/runtime/adapters/lottieReadiness.js, but that file is missing from the published 0.6.6 and 0.6.7 tarballs. Consumers that import the subpath — most notably@hyperframes/studio'sPlayer.tsx— fail to resolve the module and break downstream builds.Root cause:
packages/core/tsconfig.jsonexcludessrc/runtime(those files run in a browser context and are bundled separately into the IIFE artifact). Since nothing in the included tree importslottieReadiness.ts, tsc never emits a compiled output, and the file silently goes missing from the publish.Fix:
lottieReadiness.tsis a pure helper — takesunknown, returnsboolean, no DOM/windowdependencies. It doesn't belong insrc/runtime/in the first place; the runtime-exclude rule rightly caught it. Move it tosrc/lottieReadiness.tsso the standard library build picks it up.The subpath export name stays
"./runtime/lottie-readiness"— only the exports map's underlying file path changes — so existing consumers (studio) don't need any code change.2.
studio: guardimport.meta.envfor non-Vite hostspackages/studio/src/components/editor/manualEditingAvailability.tsunconditionally readsimport.meta.env. That's a Vite-only extension; in plain ESM hosts (Next.js / Turbopack, Node, jest in some configs)import.metaexists butimport.meta.envisundefined. Reading any property off undefined throws at module evaluation time, so the studio fails to load the moment a non-Vite host imports anything from@hyperframes/studio.Guarded the read so the module is loadable everywhere; outside Vite, every flag falls back to its declared default, preserving Vite behavior.
Changes
core:
mv src/runtime/adapters/lottieReadiness.{ts,test.ts}→src/src/runtime/adapters/lottie.tsre-export pathpackage.json+publishConfig.exportsto point at the new dist path (./dist/lottieReadiness.{js,d.ts})studio:
manualEditingAvailability.ts:30with explanatory commentTest plan
pnpm typecheck(core, studio) — cleanbun run build(core) —dist/lottieReadiness.{js,d.ts}now presentbunx vitest run(core) — 862/862 passingbun run typecheck(studio) — clean, resolves moved file via subpath exportdist/lottieReadiness.js@hyperframes/studiowithoutimport.meta.enverrors🤖 Generated with Claude Code