feat: add init tailwind flag#577
Conversation
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
jrusso1020
left a comment
jrusso1020
left a comment
There was a problem hiding this comment.
Staff-engineer review. Small, well-scoped DX add — implementation is clean and the test bar is reasonable for the scaffolding side. The bulk of my comments are on the runtime-rendering implications of shipping @tailwindcss/browser@4 into the render pipeline, which the PR description doesn't address.
Things to fix before merge
1. Pin the Tailwind CDN version (determinism)
packages/cli/src/commands/init.ts:58
const TAILWIND_BROWSER_SRC = "https://cdn.jsdelivr.net/npm/@tailwindcss/browser@4";This pins to the major, so jsdelivr returns whatever 4.x.y is current at fetch time. Two renders of the same composition months apart can produce different output frames as Tailwind ships JIT/preflight changes. For a tool whose value prop is "HTML → reproducible video," that's a quiet trap.
Pin to a specific minor at minimum (@4.1.x), and ideally add SRI:
<script
src="https://cdn.jsdelivr.net/npm/@tailwindcss/browser@4.1.13/dist/index.global.js"
integrity="sha384-..."
crossorigin="anonymous"
></script>The constant should live next to a comment explaining why the version is pinned, so the next person doesn't bump it carelessly.
2. Render-time correctness: Tailwind's browser runtime is async
@tailwindcss/browser scans the DOM, compiles utilities, and injects a <style> tag after load. The capture engine has no synchronization with that pass. For long-running fixtures and the warm-grain example this likely hides — but for compositions whose first useful frame is at t=0 (title cards, fast cuts), the renderer can capture pre-compile, unstyled frames.
The PR description says the proof composition rendered correctly. That's one fixture with a static title. I'd want one of:
- A producer-side baseline test that renders a Tailwind-using composition and verifies frame 0 already has utilities applied (i.e. compositor waited), OR
- An explicit
await window.__tailwindReady(or equivalent) hook the runtime pipeline can poll, OR - A clear note in
docs/packages/cli.mdxthat Tailwind is "best-effort at the start of the timeline; insert a 100–200 ms intro buffer if you see unstyled first frames."
This is the single most important issue. Without it, --tailwind will eventually produce a non-reproducible "first frame is broken" bug report and we won't know where it came from.
3. The injection regex has fragile fallbacks
packages/cli/src/commands/init.ts:232-241
if (/<script[\s>]/i.test(html)) {
return html.replace(/(\n[ \t]*)(<script)([\s>])/i, `$1${script}$1$2$3`);
}
return html.replace(/(\n[ \t]*)<\/head>/i, `$1${script}$1</head>`);Both branches require a \n followed by whitespace before the anchor. Any template that gets minified, gets a single-line <head>...</head>, or has a <script> immediately after > with no whitespace will silently no-op — --tailwind reports success, the user gets no Tailwind, and the failure mode is "my classes aren't applying."
Two cheap fixes:
- Drop the leading-newline requirement:
/([ \t]*)(<script)/iand prepend the script with\ninstead, OR - Just always inject before
</head>(case-insensitive) and stop trying to "preserve script ordering" — if Tailwind needs to come before user<script>s anyway, the</head>position is always correct (and matches v4 docs).
The current "before first script tag" branch is also the more dangerous one — it places Tailwind ahead of <script src="…gsap.min.js"> in the bundled blank template, which means GSAP load is now serialized behind Tailwind's compile. For renders where you're driving cold-start time down, that's a measurable regression. Recommend: insert just before </head> with defer, or after the existing <script> rather than before it.
4. Bundled-but-unrelated refactors
Two changes in this PR shouldn't be part of "feat: add init tailwind flag":
.github/workflows/preview-regression.yml— swapping apt forFedericoCarboni/setup-ffmpeg. Worth doing (and the SHA pin is correct), but it's a CI tweak, separate blast radius.buildPackageScripts()extraction atinit.ts:195-204— pure cosmetic cleanup; the new code path doesn't touchpackage.json.
Not blocking, but next time these belong in their own commits or PRs so a revert of the Tailwind feature doesn't drag CI infra with it.
Things worth a comment but not blocking
- No interactive prompt for
--tailwind. The wizard mode (--human-friendly) silently can't reach this feature. Add a confirm step, or accept that it's a flag-only opt-in and call that out incli.mdx. listHtmlFileswalks recursively after copy. Today templates are flat, but the moment someone ships an example with anode_modules-like dir or adist/of HTML fragments, Tailwind gets injected into all of them. Consider scoping to top-level*.htmlor excludingnode_modules/dist.- No
trackInitTailwindtelemetry.trackInitTemplate(templateId)already runs; emitting whether--tailwindwas set would let you measure adoption. Cheap to add now. - Test coverage gap. The
--tailwind enables Tailwind utilitiestest only asserts the script tag is in the file. It doesn't assert idempotency (running with--tailwindagainst a destDir that already had Tailwind), and doesn't exercise the</head>-only branch. Both are one-liners againstinjectTailwindBrowserScript(already exported for testing). Scaffold with Tailwind CSShelp example atinit.ts:9—--tailwindalone falls back to--example blankin non-interactive mode, but in interactive mode it'll prompt. Worth showing it ashyperframes init my-video --example blank --tailwindso the example is copy-pasteable in both modes.
What's good
- Test for uppercase
<SCRIPT>shows you thought about real-world template variance — right instinct. injectTailwindBrowserScriptexported and pure — clean testability.- Idempotent guard (
if (html.includes(TAILWIND_BROWSER_SRC)) return html) is correct. - Docs updated alongside code.
- TDD red→green note in the PR description is the right hygiene.
Summary
Approve once (1) version pin and (3) regex robustness are addressed. Item (2) render synchronization is the real long-term concern — if you don't want to handle it in this PR, please file a follow-up issue ("Tailwind browser runtime: synchronize style application with first frame capture") and link it in the docs note so the trap is at least visible.
|
@jrusso1020 thanks for the detailed review. I addressed the blocking Tailwind runtime concerns and updated the PR description with the new proof. What changed:
Verification added to the PR body:
Latest replacement CI run is green for the core checks, CLI smoke/global install, Windows tests/render, docs, preview regression, and CodeQL. Some broader regression shards are still pending. |
jrusso1020
left a comment
jrusso1020
left a comment
There was a problem hiding this comment.
Re-review on da37d5ef. All three "fix before merge" items resolved; render-synchronization was solved properly rather than papered over with a docs note. Approving — remaining comments are nits and one small maintainability suggestion.
How my blocking items were resolved
1. Pin version + SRI ✅
packages/cli/src/commands/init.ts:58-63
const TAILWIND_BROWSER_VERSION = "4.2.4";
const TAILWIND_BROWSER_SRC = `https://cdn.jsdelivr.net/npm/@tailwindcss/browser@${TAILWIND_BROWSER_VERSION}/dist/index.global.js`;
const TAILWIND_BROWSER_INTEGRITY = "sha384-v5YF9xS+gLRWdvrQ0u/WRbCkjSIH0NjHIPe8tBL1ZRrmI7PiSH6LLdzs0aAIMCuh";Exact pin, SRI hash, crossorigin="anonymous", comment explaining why. Version split into its own constant so a future bump is a one-token diff. Good.
2. Render-time correctness — solved properly ✅
This is the part I want to call out. Instead of taking the docs-note escape hatch I offered, you built a real synchronization primitive:
- The CLI injects an inline shim before the Tailwind CDN script that exposes
window.__tailwindReadyas a Promise. - The Promise resolves when both (a)
document.readyState === "complete"and (b) a<style>tag containingtailwindcss vis observed in the DOM. - Detection uses
MutationObserveronly — noDate.now,setTimeout, orrequestAnimationFrame. That's deliberate (and unit-tested), and it matters because BeginFrame mode virtualizes the page's clock; using rAF or wall-time timers in the page can deadlock against the renderer driving the event loop. - Engine-side,
frameCapture.ts:263-281addswaitForOptionalTailwindReady, gated on the presence of the global. Wired into both warmup (:370) and BeginFrame (:464) paths, afterdocument.fonts?.ready. - Failure mode is a loud, actionable error rather than silently-unstyled frame 0:
[FrameCapture] window.__tailwindReady not resolved after ${timeoutMs}ms. Tailwind browser runtime must finish before frame capture starts.
This is the kind of fix I'd expect from somebody who actually understood the constraint, not just patched around the comment. Nice work.
3. Regex robustness ✅
init.ts:268-273
if (/<\/head>/i.test(html)) {
return html.replace(/<\/head>/i, (closingHead) => `\n${script}\n${closingHead}`);
}
return `${script}\n${html}`;One case-insensitive anchor, no leading-newline assumption, single-line head test added. Fallback prepends to the document instead of silently no-op'ing — also good.
4. (Non-blocking) The unrelated CI / buildPackageScripts refactors stayed
That's fine — I called it non-blocking originally.
Other addressed items
listHtmlFilesnow skips.git,dist,node_modules(init.ts:231-249)- Telemetry:
trackInitTemplate(templateId, { tailwind })in both code paths - Help example:
hyperframes init my-video --example blank --tailwind(copy-pasteable in both modes) - New tests: exact CDN+SRI script string, idempotency, single-line head, uppercase
</HEAD>, and the absence-of-render-loop-APIs test
Remaining nits (none blocking)
a. The tailwindcss v string marker is undocumented coupling
init.ts:289
if (text.indexOf("tailwindcss v") !== -1) return text;This is reading the comment header Tailwind's browser runtime emits in the injected <style>. If a future version drops or rephrases that header, the readiness Promise never resolves and renders fail at pageReadyTimeout with a misleading "Tailwind didn't finish" error.
The version pin protects you (any bump requires explicit re-verification), but a one-line comment in the shim noting the coupling would help the next person:
// Detects Tailwind's "/*! tailwindcss v… */" header in the injected <style>.
// If the version constant above changes, re-verify this marker still matches.b. Consider extracting the inline shim to its own file
The shim is a 12-line ES5 string concatenated inside init.ts:254-267. oxfmt could re-flow that array of string fragments and subtly change the emitted JS. Moving it to packages/cli/src/templates/_shared/tailwind-ready.shim.js and reading it at scaffold time would:
- Get syntax highlighting and lint coverage on the shim itself
- Decouple shim formatting from the surrounding TS file
- Make the shim diffable when Tailwind's marker changes
Optional — not worth a re-review round.
c. Late-resolution leak in waitForOptionalTailwindReady
frameCapture.ts:266-274
const ready = await Promise.race([
page.evaluate(`Promise.resolve(window.__tailwindReady).then(() => true, () => false)`),
new Promise<boolean>((resolve) => setTimeout(() => resolve(false), timeoutMs)),
]);Promise.race doesn't cancel the loser. If the timeout wins, the inner page.evaluate is still running on the page and may emit an unhandled-rejection warning if the page closes before it resolves. Cheap fix: stash the inner Promise, attach a no-op .catch(() => {}) to it after the race. Optional.
d. Producer-side integration test (follow-up)
The CLI side now exhaustively tests the shim string and the injection logic. There's no producer-side baseline that actually renders a Tailwind composition through frameCapture and asserts frame 0 has utilities applied. Worth a follow-up issue to add one when the producer baseline harness has time available — that's the only thing standing between us and a real regression guard.
Summary
Approving. The blocking items are all resolved, and the __tailwindReady design is the right one. The four nits above are all post-merge follow-ups at most.
|
@jrusso1020 @vanceingalls shipped the Tailwind skill follow-up in cbd85e6. What changed:
Extra validation:
CI is running on the new head now. |
jrusso1020
left a comment
jrusso1020
left a comment
There was a problem hiding this comment.
Re-review on cbd85e6c (docs: add tailwind composition skill). Pure documentation + skill addition — no code or test changes from da37d5ef. The earlier approval still stands; posting this as informational follow-up.
What changed
- New
skills/tailwind/SKILL.md— 150-line skill teaching agents Tailwind v4 browser-runtime patterns specific to HyperFrames compositions - Cross-references in repo
CLAUDE.md,README.md,docs/quickstart.mdx,docs/packages/cli.mdx - Scaffolded-project docs (
packages/cli/src/templates/_shared/{AGENTS,CLAUDE}.md) advertise/tailwindalongside the existing skill table skills/hyperframes-cli/SKILL.mdadds the--tailwindexample and a "invoke/tailwindfirst" pointer- Plugin manifests (
.codex-plugin/plugin.json,.cursor-plugin/plugin.json) get "tailwind" in keywords and the description
Skill quality
A few things worth calling out as deliberately well-handled:
- Version contract is explicit. Pinned to
@tailwindcss/browser@4.2.4, and the skill warns against substitutingcdn.tailwindcss.com(which is v3). Matches the code pin ininit.ts. - The
__tailwindReadyinvariant is documented as a constraint, not a footnote. "The readiness shim must stay deterministic: no render-loop polling APIs, no clock-based retries…" — exactly the design intent from the harden commit, now codified for future agents who might be tempted to boltsetTimeoutretries on. - Dynamic class warning ("Tailwind's browser runtime scans the current document and generates CSS for class names it can see. Do not build render-critical class names only at seek time") is a real gotcha for animated compositions and the right thing to flag.
- v4 vs v3 nuance is correct: directs users away from
@tailwind base/components/utilitiesandtailwind.config.js, but acknowledges@configfor the compiled-CSS path. Doesn't oversimplify. - Studio carve-out — the skill is explicit that it's only for scaffolded projects, not for
packages/studiowhich runs Tailwind v3 internally. Good — prevents agents from "fixing" Studio.
Nits (none blocking)
- The two community-skill reference URLs at the bottom (
skills.sh/...,agent-skills.md/...) are third-party links that will eventually 404. Either drop the links and keep attribution prose, or accept that they'll rot. - Worth a quick sanity check that
npx skills add heygen-com/hyperframesactually registers/tailwindas a bare slash command (vs. namespaced). Since/gsap,/animejs, etc. presumably already work that way, you're likely fine. - The example
@utility text-balance-tight { text-wrap: balance; ... }is illustrative; real compositions in the skill use the built-in v4text-balanceutility. Slight inconsistency but not confusing.
No further action needed from me.
jrusso1020
left a comment
jrusso1020
left a comment
There was a problem hiding this comment.
LGTM — solid implementation of the --tailwind scaffold + a well-disambiguated v3-vs-v4 skill.
Mechanics:
- Pinned to exact
@tailwindcss/browser@4.2.4/dist/index.global.js. SRI hash matches the published artifact (re-computed locally and confirmed).crossorigin="anonymous"set. __tailwindReadyPromise shim is deterministic — usesMutationObserverwatching for the Tailwind-generated<style>block containingtailwindcss vto appear. NosetTimeout, noDate.now, norequestAnimationFrame. The "keeps readiness shim free of render-loop APIs" test enforces this string-level contract; if someone reverts to a timing-API-based shim, it fails.frameCapture.ts:waitForOptionalTailwindReadywired into both BeginFrame and Screenshot mode afterdocument.fonts?.ready. Soft no-op when Tailwind isn't present (so non-tailwind projects aren't penalized); hard error if__tailwindReadyexists but doesn't resolve withinpageReadyTimeout. Right shape.listHtmlFilesrewritten to walk explicitly + skip.git,dist,node_modulesinstead ofreaddirSync({ recursive: true }). Important fix — the recursive variant would have descended intonode_modulesand corrupted vendored HTML.- Idempotency check (
html.includes(TAILWIND_BROWSER_SRC)) prevents double-injection on re-runs. Insert-before-</head>works for both single-line and multi-line head tags + uppercase variants (test coverage explicit on each).
Symmetric proof: reverted only init.ts while keeping the new tests → 5 of 7 init tests fail red. Restoring → 192/192 cli tests pass. New tests genuinely cover the new behavior.
Skill content (skills/tailwind/SKILL.md):
- Pinned to
@tailwindcss/browser@4.2.4consistent with the CLI. - Explicitly calls out v4-vs-v3 confusion: 13 in-context v3 references, all prescribing AGAINST v3 patterns (no
@tailwind base/components/utilities, notailwind.config.js, use@theme/@utilityin<style type="text/tailwindcss">instead). - Important note that
packages/studiostill uses Tailwind v3 internally — agents shouldn't attempt a v3→v4 migration there based on this skill. - Calls out v4 utility renames (
shadow-xs,rounded-xs,outline-hidden,shrink-*,grow-*) and the v4 default-border behavior change (borderalone may not work; useborder border-white/20). Real gotchas that agents will hit. - Dynamic-class-safety section is correct — the browser runtime scans current DOM, so
element.className = \bg-${color}-500`` may not be detected before capture. Solid agent-UX. - Frontmatter follows the same shape as existing skills. Plugin manifest descriptions + keywords updated; CLAUDE.md template includes the new skill in its table.
Two non-blocking nits:
-
Plugin sync follow-up:
.codex-plugin/plugin.jsondescription and keywords got updated to mention Tailwind. After merge, this needs a sync PR toopenai/plugins/plugins/hyperframes/per the manual-mirror gotcha I documented earlier (.cursor-plugin/.claude-pluginconsume the hyperframes repo directly so they auto-pick-up; openai/plugins doesn't). -
Two external community references in "Credits And References":
skills.sh/tlq5l/tailwindcss-v4-skillandagent-skills.md/skills/bout3fiddy/agents/tailwindcss. Linking them as references is fine, but agents reading the skill might fetch them as authoritative sources during composition work. Consider whether the team wants agents pulling third-party skill content into a build pipeline (or move these to a "## Inspired by" footnote without URLs that look fetch-friendly).
The CI sweep-along (apt-get install ffmpeg → FedericoCarboni/setup-ffmpeg@v3 action, SHA-pinned) is a clean unrelated-but-correct change.
— Review by Rames Jusso
|
@jrusso1020 thanks, agreed on the maintenance nits. I pushed d8843fb as a tiny docs-only cleanup:
On the |
Merge activity
|
3 participants
Problem
Users who want Tailwind utilities in a plain HyperFrames composition currently have to know which Tailwind browser script to add and where to place it. The first pass added
--tailwind, but review caught three production-facing gaps: the CDN version was major-only, the insertion helper could silently no-op on compact HTML, and the render pipeline did not explicitly wait for Tailwind's async browser compilation before capturing frame 0.There is also a version-specific agent risk: HyperFrames
init --tailwinduses Tailwind v4.2 through@tailwindcss/browser@4.2.4, whilepackages/studiostill uses Tailwind v3. Without a dedicated skill, agents can easily mix v3tailwind.config.js/@tailwindpatterns into v4 browser-runtime composition HTML.What this fixes
hyperframes init --tailwind.@tailwindcss/browser@4.2.4/dist/index.global.jswith SRI andcrossorigin="anonymous".window.__tailwindReadypromise next to the browser runtime.window.__tailwindReadyin both screenshot and BeginFrame capture modes before capturing frame 0.</head>case-insensitively, including single-line/minified heads, and falls back to prepending when there is no head tag..git,dist, andnode_modules.init_templatetelemetry event./tailwindskill for Tailwind v4.2 browser-runtime HyperFrames composition work.Root cause
scaffoldProject()copied the selected example and patched media placeholders, then immediately wrote project metadata andpackage.json. There was no optional post-copy step for framework-specific HTML support. The initial Tailwind post-copy step also treated the browser runtime like a static script, but Tailwind compiles utilities asynchronously after scanning the DOM, so the capture engine needed an explicit readiness contract.On the agent side, the repo exposed HyperFrames, CLI, GSAP, registry, and runtime adapter skills, but had no Tailwind-specific instruction to separate the v4 browser-runtime composition path from Studio's v3 internal setup.
Verification
Local checks
bunx vitest run packages/cli/src/commands/init.test.tsbun run --filter @hyperframes/cli test src/commands/init.test.tsbun run --filter @hyperframes/cli typecheckbun run --filter @hyperframes/engine typecheckbun run lint:skillsbun run lintnpx skills add . --listshowed 12 local skills, includingtailwind.bunx oxfmt --check packages/cli/src/commands/init.ts packages/cli/src/commands/init.test.ts packages/cli/src/telemetry/events.ts packages/engine/src/services/frameCapture.ts docs/packages/cli.mdxbunx oxfmt --check README.md docs/quickstart.mdx docs/packages/cli.mdx CLAUDE.md packages/cli/src/templates/_shared/CLAUDE.md packages/cli/src/templates/_shared/AGENTS.md skills/hyperframes-cli/SKILL.md skills/tailwind/SKILL.md .codex-plugin/plugin.json .cursor-plugin/plugin.jsonbunx oxlint packages/cli/src/commands/init.ts packages/cli/src/commands/init.test.ts packages/cli/src/telemetry/events.ts packages/engine/src/services/frameCapture.tsgit diff --checkGenerated-project render proof at
/tmp/hf-tailwind-render-proof:bun packages/cli/src/cli.ts init /tmp/hf-tailwind-render-proof --example blank --tailwind --non-interactive --skip-skillsflex,h-full,w-full,items-center,justify-center,bg-slate-950,rounded-3xl,bg-white,px-20,py-12,text-8xl,font-black,text-black, andshadow-2xl.bun packages/cli/src/cli.ts lint /tmp/hf-tailwind-render-proof→ 0 errors, 0 warnings.bun packages/cli/src/cli.ts validate /tmp/hf-tailwind-render-proof→ 0 errors, 0 regular warnings; the temp proof still reports validator contrast warnings even though the rendered/browser pixels show black text on white background.bun packages/cli/src/cli.ts render /tmp/hf-tailwind-render-proof --workers 1 --fps 24 --quality draft --output /tmp/hf-tailwind-render-proof-artifacts/output.mp4https://cdn.jsdelivr.net/npm/@tailwindcss/browser@4.2.4/dist/index.global.js.ffprobe -v error -select_streams v:0 -show_entries stream=codec_name,width,height,r_frame_rate,duration -of default=noprint_wrappers=1 /tmp/hf-tailwind-render-proof-artifacts/output.mp4→ H.264, 1920x1080, 24fps, 10s./tmp/hf-tailwind-render-proof-artifacts/frame-000.png.Browser verification
/tmp/hf-tailwind-render-proof.agent-browserto openhttp://localhost:5194./tmp/hf-tailwind-render-proof-artifacts/browser/tailwind-preview.png./tmp/hf-tailwind-render-proof-artifacts/browser/tailwind-preview.webm.agent-browserto open the new Tailwind skill proof page.@tailwindcss/browser@4.2.4./Users/miguel07code/.codex/worktrees/pr-577-tailwind-comments/tmp/agent-browser-proof/tailwind-skill.png./Users/miguel07code/.codex/worktrees/pr-577-tailwind-comments/tmp/agent-browser-proof/tailwind-skill.webm.Notes
hyperframes init --tailwindsmall and compatible with the current no-install generated project workflow./tailwindskill cites official Tailwind v4 docs plus community skill references, but its instructions are HyperFrames-specific and tuned for the pinned v4.2 browser runtime./tmp/hf-tailwind-render-proof-artifacts/andtmp/agent-browser-proof/and intentionally not committed.