Skip to content

fix(core): scope GSAP selector arrays to composition root - #4035

Merged
miguel-heygen merged 1 commit into
mainfrom
bugfix/1787766784-array-selector-scope
Sep 18, 2026
Merged

miguel-heygen merged 1 commit into
mainfrom
bugfix/1787766784-array-selector-scope

Conversation

@heygengenesis

@heygengenesis heygengenesis Bot commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

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:

$ bun run --cwd packages/core vitest run src/compiler/compositionScoping.test.ts -t "scopes each selector in a GSAP target array to the composition root"
AssertionError: expected [ 'scene', 'other', 'scene', 'other' ] to deeply equal [ 'scene', 'scene' ]
Test Files  1 failed (1)
Tests  1 failed | 56 skipped (57)
error: "vitest" exited with code 1

Full RED output: /home/ubuntu/workspace/.bug-fix/4034-array-selector-red.log

GREEN after the resolver change:

$ bun run --cwd packages/core vitest run src/compiler/compositionScoping.test.ts -t "scopes each selector in a GSAP target array to the composition root"
Test Files  1 passed (1)
Tests  1 passed | 56 skipped (57)

Full GREEN output: /home/ubuntu/workspace/.bug-fix/4034-array-selector-green.log

Temporarily 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 passed
  • bun run --cwd packages/core typecheck — passed
  • bunx oxfmt --check packages/core/src/compiler/compositionScoping.ts packages/core/src/compiler/compositionScoping.test.ts — passed
  • bunx oxlint packages/core/src/compiler/compositionScoping.ts packages/core/src/compiler/compositionScoping.test.ts — 0 warnings, 0 errors
  • bunx fallow audit --base origin/main — no issues in 2 changed files

Simplify

No reductions found. The repository has no dedicated simplify entry 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

  • Offending main resolver:
    var __hfResolveGsapTarget = function(target) {
    if (typeof target !== "string") return target;
    return __hfQueryAll(target);
  • Resolver change:
    var __hfResolveGsapTarget = function(target) {
    if (typeof target === "string") return __hfQueryAll(target);
    if (!Array.isArray(target)) return target;
    return target.reduce(function(resolved, item) {
    if (typeof item === "string") {
    return resolved.concat(Array.prototype.slice.call(__hfQueryAll(item)));
    }
    resolved.push(item);
    return resolved;
    }, []);
  • Regression fixture:
    it("scopes each selector in a GSAP target array to the composition root", () => {
    const { document } = parseHTML(`
    <div data-composition-id="scene">
    <h1 class="title">Scene title</h1>
    <p class="subtitle">Scene subtitle</p>
    </div>
    <div data-composition-id="other">
    <h1 class="title">Other title</h1>
    <p class="subtitle">Other subtitle</p>
    </div>
    `);
    const targetCompositions: Array<string | null> = [];
    const fakeWindow = {
    document,
    __timelines: {},
    gsap: {
    to(targets: Array<string | Element>) {
    const resolvedTargets = targets.flatMap((target) =>
    typeof target === "string" ? Array.from(document.querySelectorAll(target)) : [target],
    );
    targetCompositions.push(
    ...resolvedTargets.map((target) =>
    target.closest("[data-composition-id]")?.getAttribute("data-composition-id"),
    ),
    );
    },
    },
    };
    const wrapped = wrapScopedCompositionScript(
    `gsap.to(['.title', '.subtitle'], { opacity: 1 });`,
    "scene",
    );
    new Function("window", "gsap", wrapped)(fakeWindow, fakeWindow.gsap);
    expect(targetCompositions).toEqual(["scene", "scene"]);
    });

Co-authored-by: miguel.sierra <229591595+miguel-heygen@users.noreply.github.com>

@miga-heygen miga-heygen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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:

  1. Array-likes that are not arrays. Array.isArray is false for arguments, a NodeList, or any {0:..., length:n}, so those pass through unscoped. A NodeList is already resolved elements so it is harmless, but an array-like carrying selector strings still escapes to document scope. GSAP accepts these.
  2. Nested arrays. GSAP flattens [['.a','.b'], '.c']. Here the inner array is not a string, so it takes the push branch 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

@miguel-heygen
miguel-heygen merged commit 2730365 into main Sep 18, 2026
84 of 85 checks passed
@miguel-heygen
miguel-heygen deleted the bugfix/1787766784-array-selector-scope branch September 18, 2026 15:12
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.

2 participants