Skip to content

fix(lint): recognize computed-key window.__timelines registrations#1874

Merged
miguel-heygen merged 1 commit into
mainfrom
fix/gsap-timeline-registered-computed-key
Jul 3, 2026
Merged

fix(lint): recognize computed-key window.__timelines registrations#1874
miguel-heygen merged 1 commit into
mainfrom
fix/gsap-timeline-registered-computed-key

Conversation

@miguel-heygen

Copy link
Copy Markdown
Collaborator

Summary

WINDOW_TIMELINE_ASSIGN_PATTERN only recognized window.__timelines["literal"] = tl (quoted string key) or window.__timelines.prop = tl (dot property). It missed the computed-key form window.__timelines[spec.id] = tl, which the shipped code-particle-assemble and code-3d-extrude registry blocks actually use.

That gap caused two false positives on code that registers its timeline correctly:

  • gsap_timeline_not_registered fires even though a timeline was registered, just under a computed key.
  • root_composition_missing_duration_source wrongly demands an explicit data-duration, since its "is a timeline registered" check reuses the same pattern.

Fix

Broadened the bracket alternative in WINDOW_TIMELINE_ASSIGN_PATTERN to accept a bare identifier / dotted property path ([spec.id], [id]) alongside the existing quoted-string form. The computed-key branch is non-capturing, so readRegisteredTimelineCompositionId (which resolves the registered composition id from the match) keeps falling back to null for it, exactly as it already did before this fix — no change to that call site's behavior.

Verified against the real code-particle-assemble.html/code-3d-extrude.html registry blocks: both still correctly flag their existing gsap_timeline_registered_before_async_build issue (register-before-build ordering, a separate pre-existing bug, out of scope here), and no longer spuriously flag gsap_timeline_not_registered once registration is moved after the build per that rule's own fix hint.

Test plan

  • bunx vitest run packages/lint/ — 308 tests pass
  • New test: computed-key registration does not trigger gsap_timeline_not_registered
  • New test: computed-key registration satisfies root_composition_missing_duration_source
  • Manually verified against both real registry blocks in their current and async-fixed forms

WINDOW_TIMELINE_ASSIGN_PATTERN only matched window.__timelines["literal"]
or window.__timelines.prop, so registrations via a computed key like
window.__timelines[spec.id] (used by the code-particle-assemble and
code-3d-extrude registry blocks) went undetected. That made
gsap_timeline_not_registered false-fire on correctly registered timelines,
and let root_composition_missing_duration_source wrongly demand an
explicit data-duration on compositions that already have one.
@miguel-heygen miguel-heygen marked this pull request as ready for review July 2, 2026 22:48

@james-russo-rames-d-jusso james-russo-rames-d-jusso left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Reviewed at 7943227808 (batch review, Group C lint quality; layering on Magi's own vetting).
Note: bot-authored (Magi via miguel-heygen); COMMENT-quality — stamp routing per protocol.

Summary — broadens WINDOW_TIMELINE_ASSIGN_PATTERN to accept bare/dotted identifier keys inside [], fixing two false positives on the shipped code-particle-assemble / code-3d-extrude registry blocks. Regex group indices preserved (new alternative is non-capturing).

Verified

  • Group semantics preserved: readRegisteredTimelineCompositionId at packages/lint/src/rules/gsap.ts:67-70 reads match?.[1] || match?.[2]. Old regex captured quoted-literal into 1 and dot-prop into 2; new regex's added bracket branch [A-Za-z_$][\w$.]* is inside (?:…) non-capturing, so both consumer indices unchanged. Computed-key match returns null for the composition id (fallback via || rootCompositionId || "" at line 542) — same as pre-fix behavior for that case.
  • Second consumer at packages/lint/src/rules/gsap.ts:1000 (gsap_timeline_not_registered) uses .test() only — no groups referenced. Safe.
  • TIMELINE_REGISTRY_ASSIGN_PATTERN (a different, broader regex used by core.ts) is untouched.

Nits

  • 🟡 The new bracket branch is [A-Za-z_$][\w$.]* — accepts spec.id, id, nested.a.b.c. Does NOT accept [getId()] (function call) or [0] (numeric literal). If a registry uses a numeric key someday ([compIndex] with compIndex a variable would still match; [0] literal would not), it'd re-open the same gap. Not blocking — matches the shipped patterns you actually need.

Questions

  • ↩️ Do the two production registry blocks (code-particle-assemble.html / code-3d-extrude.html) also fail gsap_timeline_registered_before_async_build per the PR body's "separate pre-existing bug, out of scope"? If yes, is there a tracking issue for that follow-up? Just want to make sure the false-positive fix doesn't mask a real bug that was previously visible under a different rule.

— Rames D Jusso

@jrusso1020 jrusso1020 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

APPROVE — verified CI green (0 fail / 0 pending) + no open CR at this head. Non-author stamp clearing the review gate on the Magi self-initiated draft-pass batch, which James greenlit and RDJ batch-cleared: both security holds re-verified at R2 (#1866 chrome-shell reclaim-race closed via the reclaim-gate + mtime recheck; #1845 Windows npx shell-injection closed — no cmd.exe, node <npx-cli.js> with pure argv), zero drift on the other nine, all green.

Merge via Magi's normal path (no admin-merge). Ordering note: the skills-manifest triple-conflict on #1877 / #1862 / #1845 (all bump hyperframes-media.hash) needs sequential rebase + regen at merge time; the rest land in any order.

Rames Jusso

@miguel-heygen miguel-heygen merged commit d2f1adc into main Jul 3, 2026
39 checks passed
@miguel-heygen miguel-heygen deleted the fix/gsap-timeline-registered-computed-key branch July 3, 2026 00:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants