Repository navigation
fix(core): scope GSAP selector arrays to composition root - #4035
Conversation
Co-authored-by: miguel.sierra <229591595+miguel-heygen@users.noreply.github.com>
miga-heygen
left a comment
There was a problem hiding this comment.
Approved at 89bb4ca0289f5f26c70f47e749816b1cfb731743. 52 checks green, 8 skipped, none failing.
Right insertion point
__hfResolveGsapTarget is the single chokepoint every target path already runs through: the gsap proxy's to/from/fromTo/set, __hfScopeTimeline's four timeline methods, and utils.toArray. Fixing the array case there fixes it everywhere at once, with no second owner of the rule. The alternative, patching each call site, is the version of this change that would have rotted.
The test is non-vacuous, and for the right reason
The fake gsap.to resolves strings itself via a document-wide querySelectorAll. That is what makes it a real test rather than a tautology: without the fix the targets arrive as strings, the fake resolves them across both roots, and the assertion sees ["scene","other","scene","other"] instead of ["scene","scene"]. It fails for the exact reason production failed.
Order is also preserved, which matters: reduce walks the array in order and concatenates each selector's matches in document order, so a stagger over ['.title','.subtitle'] keeps the sequence GSAP would have produced unscoped.
Two holes this does not close
Both are narrower than the bug being fixed and neither should hold the PR. Naming them so the next person does not assume array targets are now fully covered:
- Array-likes that are not arrays.
Array.isArrayis false forarguments, aNodeList, or any{0:..., length:n}, so those pass through unscoped. ANodeListis already resolved elements so it is harmless, but an array-like carrying selector strings still escapes to document scope. GSAP accepts these. - Nested arrays. GSAP flattens
[['.a','.b'], '.c']. Here the inner array is not a string, so it takes thepushbranch and its selectors are never scoped.
If you want both, the loop becomes a recursive flatten over anything iterable rather than an Array.isArray check. That is a bigger behavioural surface than #4034 asks for, so a follow-up issue is the right home for it, not this PR.
Nit, non-blocking
The reduce mixes concat (allocates a new accumulator) with push (mutates and returns the same one). Both branches return the accumulator correctly so it is not a bug, but the two halves read as if they disagree about who owns the array. push.apply on both branches, or concat on both, would say one thing.
Scope
Read the diff, the full resolver and its callers in compositionScoping.ts at this head, and the check rollup. I did not run the suite locally. Merge is not mine to press.
— Miga
Summary
Scopes string entries in GSAP target arrays through the composition root and flattens the resolved element lists. Non-string targets remain unchanged.
Related to #4034.
Evidence
RED before the resolver change:
Full RED output:
/home/ubuntu/workspace/.bug-fix/4034-array-selector-red.logGREEN after the resolver change:
Full GREEN output:
/home/ubuntu/workspace/.bug-fix/4034-array-selector-green.logTemporarily reverting only the resolver makes the same regression fail again with
['scene', 'other', 'scene', 'other']; full output:/home/ubuntu/workspace/.bug-fix/4034-array-selector-revert.log.Additional validation:
bun run --cwd packages/core vitest run src/compiler/compositionScoping.test.ts— 57 passedbun run --cwd packages/core typecheck— passedbunx oxfmt --check packages/core/src/compiler/compositionScoping.ts packages/core/src/compiler/compositionScoping.test.ts— passedbunx oxlint packages/core/src/compiler/compositionScoping.ts packages/core/src/compiler/compositionScoping.test.ts— 0 warnings, 0 errorsbunx fallow audit --base origin/main— no issues in 2 changed filesSimplify
No reductions found. The repository has no dedicated
simplifyentry point; its CI diff analyzer,bunx fallow audit --base origin/main, was run over this diff and found no dead code, complexity, or duplication issue.Permalinks
hyperframes/packages/core/src/compiler/compositionScoping.ts
Lines 526 to 528 in a8a9fdb
hyperframes/packages/core/src/compiler/compositionScoping.ts
Lines 526 to 535 in 89bb4ca
hyperframes/packages/core/src/compiler/compositionScoping.test.ts
Lines 387 to 423 in 89bb4ca