fix: nested GSAP sub-composition lint and render handling#405
fix: nested GSAP sub-composition lint and render handling#405miguel-heygen merged 4 commits intomainfrom
Conversation
miguel-heygen
changed the title
jrusso1020
left a comment
jrusso1020
left a comment
There was a problem hiding this comment.
Both fixes are at the right layer and the root-cause analysis is correct. Walking through each:
Fix 1 — missing_gsap_script lint rule for sub-compositions. canInheritGsapFromHost = options.isSubComposition || rawSource.trimStart().toLowerCase().startsWith("<template") is a good two-path shape. The explicit caller-supplied flag is the bulletproof path (programmatic callers who know they're linting a sub-composition pass isSubComposition: true); the <template> heuristic is the backstop for when the file is being linted in isolation from an IDE or CLI. Solid.
One small robustness note: the heuristic triggers on exact <template prefix after trimStart(). A composition file that starts with a DOCTYPE (<!DOCTYPE html>\n<template…), an HTML comment, or a BOM would fail the heuristic. Probably not real-world given the documented nested-composition pattern always starts with <template>, but if a user sees a false-positive after wrapping their template in a comment or adding a stray blank line, the explanation is there. Worth either a one-line comment on the heuristic noting the assumption, or switching to a lightweight scan ("first non-comment, non-doctype token is <template") later if users hit it.
Fix 2 — compiler-inlined GSAP payloads no longer trigger screenshot mode. isCompilerInlinedGsapScript matches the comment marker the compiler writes (/* inlined: https://… gsap.min.js */) and the detector skips those scripts entirely. Correct fix at the right site, test coverage on both the isolated detector and the end-to-end compileForRender path is appropriate.
Two observations worth calling out for followup:
-
Tight coupling between the compiler's comment format and the detector's regex. If the inliner ever changes its comment format —
/* inlined from: …,// inlined: …, different field ordering — the detector silently stops skipping and the false-positive comes back. This coupling is hard to test against because it lives in two files. A more durable design: mark compiler-inlined scripts with a DOM attribute (e.g.<script data-hf-compiler-inlined="https://cdn…/gsap.min.js">) and havedetectRenderModeHintscheck fordata-hf-compiler-inlinedattr-presence rather than regex-parsing the comment. Same intent, less fragile seam. Not blocking for this PR. -
GSAP core only. The regex
\bgsap(?:\.min)?\.js\bmatchesgsap.js/gsap.min.js. GSAP plugin bundles (scrolltrigger.min.js,flip.min.js, etc.) inlined from the CDN would still trigger the false-positive. Probably rare today — the issue reporter was using GSAP core — but worth broadening the pattern to match the filename convention the compiler uses for all GSAP-family inlines. If the comment format already includes a path like/* inlined: https://…/gsap/3.14.2/plugins/scrolltrigger.min.js */, a more permissive\bgsap\b.*\.js\bwould cover the whole family.
Fix 3 — sub-composition bootstrap renamed so requestAnimationFrame( substring no longer matches the detector. This works but it's a workaround, not a structural fix. By renaming __tryRun → __hfCompilerRunWhenMounted and indirecting through window.requestAnimationFrame.bind(window) stored in __hfCompilerSchedule, the detector's /requestAnimationFrame\s*\(/ check no longer fires on the literal call site. It solves the issue cleanly but future edits to the bootstrap have to preserve the invariant "do not write requestAnimationFrame( literally in this function" — or the false-positive regresses silently.
Suggestion for a followup (same spirit as #1 above): wrap the inserted bootstrap in a <script data-hf-compiler-generated="sub-composition-bootstrap"> wrapper and have detectRenderModeHints skip any <script> with a data-hf-compiler-generated attribute. Then the indirection trick becomes unnecessary, the bootstrap can call requestAnimationFrame(callback) directly, and there's one clear "compiler-generated, not user code" boundary the detector respects. For now the rename is fine; just add a one-line comment above the bootstrap noting "do not use the literal requestAnimationFrame( — it trips detectRenderModeHints."
Test coverage. Lint rule covers both trigger paths (explicit flag and template-file heuristic). Render-mode detector has an isolated unit test for the inlined-GSAP skip plus a full compileForRender integration test that exercises the end-to-end nested-composition path with a mocked CDN fetch. The integration test asserts both the render-mode decision (recommendScreenshot: false, empty reasons) and that the bootstrap's rAF call survives compilation — good belt-and-braces.
Backwards compatibility. options.isSubComposition is additive. The heuristic and the inlined-GSAP skip are strictly new behaviors that reduce false positives. No existing call site changes semantics.
Approved.
— Rames Jusso
miguel-heygen
changed the title
miguel-heygen
changed the title
miguel-heygen
changed the title
Merge activity
|
2 participants
Summary
missing_gsap_scriptrequestAnimationFrame()usageRoot Cause
detectRenderModeHints()runs before CDN scripts are inlined, so nested GSAP exports were never failing because of the GSAP payload itselfrequestAnimationFrame()before running the hoisted inline scriptWhat Changed
requestAnimationFrame()requestAnimationFrame()still opts into screenshot modeValidation
bun test packages/core/src/lint/rules/gsap.test.tsbun test packages/producer/src/services/htmlCompiler.test.tsbunx oxfmt packages/producer/src/services/htmlCompiler.ts packages/producer/src/services/htmlCompiler.test.tsbunx oxlint packages/producer/src/services/htmlCompiler.ts packages/producer/src/services/htmlCompiler.test.tsbun run --filter @hyperframes/producer test --sequential chat style-11-prodstyle-11-prodpassed locallychatstill shows local-only visual drift on this macOS/ARM workstation, but the render metadata now reportsrenderModeHints.recommendScreenshot=false, which is the concrete acceptance condition for#402Closes #392
Closes #402