fix(lint): flag digit-leading ids used by toArray and ScrollTrigger selectors

[miguel-heygen]

What

id_requires_css_escape now also treats a digit-leading id as used (and so an error) when it appears in gsap.utils.toArray("#1-x") or in a ScrollTrigger trigger, endTrigger or pin value, including the scrollTrigger: "#1-x" shorthand.

Why

Since #3980 the rule errors only when a selector in the composition targets the id, and it reads selectors from CSS rules and from query and tween calls. gsap.utils.toArray and ScrollTrigger configs also run the string through querySelectorAll, so #1-x throws there too and aborts the rest of the script, but the rule rated those ids a warning.

Related work

Refs #3980. Carries over the toArray and ScrollTrigger cases from #3713, which #3980 superseded.

How

One change to the existing SELECTOR_CALL_PATTERN: toArray( joins the call list, and a second branch matches a trigger / endTrigger / pin / scrollTrigger / scroller / pinnedContainer / pinSpacer key followed by a string. Both feed the same selector check as before, so escaped selectors (#\31 23-frame) stay warnings. A non-selector key holding the same string stays a warning.

Not covered, by design of a one-regex rule: optional-call toArray?.(...), bracket access gsap.utils["toArray"], computed keys { ["trigger"]: ... }, a destructured toArray, and selector arrays (already missed for .to([...])). A key match has no context, so an unrelated object with a trigger: string also counts; that only raises an already-flagged id from warning to error.

Test plan

• Unit tests added: 13 rows in the existing rates a digit-leading id by selector use table, ported from fix(lint): block executed invalid raw selectors #3713 plus pin, endTrigger, scroller, pinnedContainer, pinSpacer, a quoted key, the shorthand, and negative rows for label, data-pin, $pin and spin.
• core.test.ts 115/115 pass. Dropping any one key, the optional quote, or $ or \w from the lookbehind fails exactly one row.
• No file in registry/, skills/ or packages/ uses these forms with a digit-leading id, so no existing composition changes severity.
• Manual testing performed (lint-only change)
• Documentation updated (not needed: the rule's documented behaviour is unchanged)

Size

Small on purpose: one regex branch and the test rows that pin it. It closes the gap #3980 left from #3713 without reviving #3713's parser rewrite.

[github-actions]

Edit accuracy: accurate 1216 (base branch 1216), smooth 977 of those

The gate passes.
Smoothness is reported in the artifact, not gated. A case fails only if it fails 2 of 3 runs.

Quarantined, measured but not gated (1)

• seqnudge-none-pct-r0-nested-z100 (fixed by fix(studio): playhead keyframe edits keep their ease, undo names its edit and never waits on videos #4889): pass, passes

[somanshreddy]

First pass at 399fcff6. This is a comment, not an approval, and it has no blockers. Two independent passes went into it, mine and Codex's, and I checked every point below at this head by linting a composition containing <div id="123-frame">.

What holds up

• The six new error rows work. I ran core.test.ts: 109 passed, including the hyphenated-key row added in the last commit. Escaped selectors (#\31 23-frame) stay warnings. Single quotes, template literals, a newline between key and value, and gsap.timeline({ scrollTrigger: { trigger } }) all rate error.
• No backtracking risk. The two string alternatives can't overlap, so an unterminated quote scans linearly. A 227 KB script of ScrollTrigger calls takes 3 ms against the new pattern, versus 1 ms against the old one.
• The compatibility claim holds. Codex and I each grepped registry/, skills/ and packages/ separately. The new forms appear with a #<digit> id only in the new test rows, so no existing composition changes severity.
• Mutations of the toArray branch and each key are caught.
  • Dropping toArray fails 1 row.
  • Dropping the whole key branch fails 5.
  • Dropping trigger fails 2.
  • Dropping endTrigger, pin or scrollTrigger fails 1 each.
  • Removing - from the lookbehind fails the data-pin row.

Worth a follow-up

1. Three more ScrollTrigger options take selector text but aren't matched. scroller, pinnedContainer and pinSpacer are documented as Element | String and go through the same toArray lookup. For example, ScrollTrigger.create({ trigger: ".a", scroller: "#123-frame" }) still rates warning. Adding them to the key list costs one line plus one row each. (Codex found pinSpacer; we both found the other two.)

2. Three parts of the new pattern aren't pinned by any test. Each of these mutations leaves all 109 tests passing:

   • Drop the optional closing quote ["']?. Then ScrollTrigger.create({ "trigger": "#123-frame" }) falls back to warning, and no error row uses a quoted key.
   • Drop $ from the lookbehind. Then { $pin: "#123-frame" } becomes an error, even though the last commit's subject says $-prefixed keys are ignored.
   • Drop \w from the lookbehind. Then { spin: "#123-frame" } becomes an error.

   A quoted-key error row plus $pin and spin warning rows would pin all three.

Lower priority: the key branch matches the key name with no context

• These all rate error at this head:
  • const meta = { trigger: "#123-frame" };
  • { "@pin": "#123-frame" } (the lookbehind excludes - but not @, : or .);
  • a string holding JSON, '{"trigger":"#123-frame"}';
  • _.toArray("#123-frame").
• This is the same kind of match the rule already makes: new Map().set("#123-frame", 1) already rates error, because the old pattern's .set( and .to( branches match any receiver.
• The cost is small. A false match only turns a warning into an error for an id that's already flagged, and the fix hint (rename the id) is the same either way. Render only blocks on lint errors under --strict, but hyperframes lint exits 1.
• Codex rated this should-fix and suggested AST-based sink detection. For a one-regex PR I'd leave that out of scope, unless false errors show up in practice.

Nits: a few valid forms still rate warning. This is fine to leave, but a short comment on the pattern listing what it doesn't cover would help the next reader:

• gsap.utils.toArray?.("#123-frame");
• gsap.utils["toArray"](...);
• { ["trigger"]: ... };
• a destructured toArray("#123-frame");
• arrays such as toArray(["#123-frame"]). Arrays were already missed for .to([...]).

CI: green at this head (77 passed, 4 skipped).

— Somu

[somanshreddy]

Re-review of the 399fcff6 → 6d756792 delta (one commit: one regex line and six test rows). I re-checked each earlier finding by number but did not review the full PR again. This is a comment, not an approval. My first-round items are fixed, and the delta has no blockers. Two independent passes went into this, mine and a static Codex pass on the delta in a separate checkout. I re-ran the earlier witnesses against lintHyperframeHtml with a <div id="123-frame"> composition.

Earlier findings, by number

1. Missed ScrollTrigger options: fixed. scroller, pinnedContainer and pinSpacer now rate error, in both double- and single-quoted forms. An escaped selector (scroller: "#\31 23-frame") still rates warning. Listing pin before pinSpacer/pinnedContainer is fine: the alternation retries the longer key when pin isn't followed by :.

2. Three untested parts of the pattern: fixed. The new rows pin all three, and each of these mutations now fails exactly one row:

   • dropping the optional closing quote (the quoted-key row);
   • dropping $ from the lookbehind (the $pin row);
   • dropping \w from the lookbehind (the spin row).

   Dropping scroller, pinnedContainer or pinSpacer from the key list also fails exactly one row each, so all 6 mutations are caught.

3. The key branch matches the key name with no context: unchanged, as expected. const meta = { trigger: "#123-frame" }, { "@pin": … } and _.toArray("#123-frame") still rate error. Two notes now that the list is longer:

   • scroller is a fairly generic property name. A plain { scroller: "#123-frame" } in app code now rates error.
   • The lookbehind doesn't exclude ., so a quoted dotted key also matches: { "layout.scroller": "#123-frame" } is an error at this head. The same thing already happened for "layout.pin" at 399fcff6, so it's the existing behaviour extended to the new keys, not something new. (Codex found the dotted form.)

   Same cost as before: a warning on an id that's already flagged becomes an error. I'd still leave it unless false errors show up.

4. Nits: unchanged. toArray?.(, ["toArray"](, toArray([...]) and other non-literal forms still rate warning.

New, low priority (Codex found these; I confirmed each by lint run)

• toggleClass: { targets: "#123-frame", className: "active" } rates warning, although ScrollTrigger resolves targets as selector text. One more key would cover it, but targets is even more generic than scroller, so given item 3 I'd leave it.
• ScrollTrigger.batch("#123-frame", …) and gsap.utils.selector("#123-frame") also stay warning, because .batch( and .selector( aren't in the call list. That's a follow-up only if those calls appear in real compositions.

Performance is unchanged. I timed the selector regex alone over 311 KB of ScrollTrigger calls that use all four keys:

• old pattern 6.9 ms, new pattern 3.8 ms;
• with a 20,000-character unterminated string appended, new pattern 6.1 ms;
• on 195 KB of pin: pin: …, 1.1 ms for both.

Tests and CI: core.test.ts passes 115 of 115 (109 plus the 6 new rows). CI is green at this head: 94 checks pass.

— Somu

[jrusso1020]

Approve at 6d756792.

Somansh's points from 399fcff6 are addressed:

1. scroller, pinnedContainer and pinSpacer are now in the key list and rate an error, and each one has its own test row.
2. The three untested parts of the pattern (quoted keys, and the $ and \w lookbehind) now have rows: "trigger", $pin and spin. There's also a data-pin row for the - case.
3. The lower-priority point still stands, as his re-review notes: the key match ignores context, so a plain {scroller: "#1-x"} rates an error. That's the same trade-off the existing .set( sink makes, and it's fine for a lint rule.

Checked: core.test.ts passes 115/115 locally. I applied 12 one-at-a-time mutations to SELECTOR_CALL_PATTERN and every one failed at least one test:

• dropping the optional key quote
• dropping each of the three lookbehind characters
• dropping each of toArray, trigger, endTrigger, pin, scrollTrigger, scroller, pinnedContainer and pinSpacer

— Rames