Skip to content

Commit b94d8bc

Browse files
vanceingallsclaude
andcommitted
fix(core,sdk): code-review findings — 5 correctness bugs + 2 cleanup
- gsapParserAcorn: top-level variable targets now resolved via program-scope null-key fallback in lookupBindingFromAncestors (const el = querySelector...) - gsapParserAcorn: fromTo guard requires args.length >= 3, preventing undefined args[2]/args[3] access when fewer args supplied - gsapWriterAcorn: remove fuzzing fallback in removeAnimationFromScript that silently deleted the wrong animation (from→to ID conversion) - gsapWriterAcorn: valueToCode guards NaN → "0" to avoid broken tween props; safeKey regex aligned to ASCII-only (matching gsapSerialize) - mutate: handleSetGsapTween now includes stagger in extras (was in addGsapTween but missing from setGsapTween) - apply-patches: script case now mirrors stylesheet — op=remove calls setGsapScript("") instead of silently ignoring the patch Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 173e826 commit b94d8bc

4 files changed

Lines changed: 36 additions & 34 deletions

File tree

packages/core/src/parsers/gsapParserAcorn.ts

Lines changed: 28 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,9 @@ function lookupBindingFromAncestors(
153153
const selector = bindings.get(scopeNode)?.get(name);
154154
if (selector !== undefined) return selector;
155155
}
156-
return null;
156+
// Program-scope bindings are stored under null (enclosingScopeNodeFromAncestors
157+
// returns null when no function wrapper exists — the common case in HF scripts).
158+
return bindings.get(null)?.get(name) ?? null;
157159
}
158160

159161
function isFunctionNode(node: any): boolean {
@@ -470,31 +472,31 @@ function findAllTweenCalls(
470472
) {
471473
const method = callee.property.name;
472474
const args = node.arguments;
473-
if (args.length >= 2) {
474-
const selectorValue =
475-
resolveTargetSelector(args[0], nodeAncestors, scope, targetBindings) ??
476-
"__unresolved__";
477-
478-
if (method === "fromTo") {
479-
results.push({
480-
node,
481-
ancestors: nodeAncestors,
482-
method: "fromTo",
483-
selector: selectorValue,
484-
fromArg: args[1],
485-
varsArg: args[2],
486-
positionArg: args[3],
487-
});
488-
} else {
489-
results.push({
490-
node,
491-
ancestors: nodeAncestors,
492-
method: method as GsapMethod,
493-
selector: selectorValue,
494-
varsArg: args[1],
495-
positionArg: args[2],
496-
});
497-
}
475+
const selectorValue =
476+
args.length >= 1
477+
? (resolveTargetSelector(args[0], nodeAncestors, scope, targetBindings) ??
478+
"__unresolved__")
479+
: "__unresolved__";
480+
481+
if (method === "fromTo" && args.length >= 3) {
482+
results.push({
483+
node,
484+
ancestors: nodeAncestors,
485+
method: "fromTo",
486+
selector: selectorValue,
487+
fromArg: args[1],
488+
varsArg: args[2],
489+
positionArg: args[3],
490+
});
491+
} else if (method !== "fromTo" && args.length >= 2) {
492+
results.push({
493+
node,
494+
ancestors: nodeAncestors,
495+
method: method as GsapMethod,
496+
selector: selectorValue,
497+
varsArg: args[1],
498+
positionArg: args[2],
499+
});
498500
}
499501
}
500502
}

packages/core/src/parsers/gsapWriterAcorn.ts

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,13 @@ import * as acornWalk from "acorn-walk";
2020
function valueToCode(value: unknown): string {
2121
if (typeof value === "string" && value.startsWith("__raw:")) return value.slice(6);
2222
if (typeof value === "string") return JSON.stringify(value);
23-
if (typeof value === "number" || typeof value === "boolean") return String(value);
23+
if (typeof value === "number") return Number.isNaN(value) ? "0" : String(value);
24+
if (typeof value === "boolean") return String(value);
2425
return JSON.stringify(value);
2526
}
2627

2728
function safeKey(key: string): string {
28-
return /^[A-Za-z_$][\w$]*$/.test(key) ? key : JSON.stringify(key);
29+
return /^[a-zA-Z_$][a-zA-Z0-9_$]*$/.test(key) ? key : JSON.stringify(key);
2930
}
3031

3132
// fallow-ignore-next-line complexity
@@ -241,11 +242,7 @@ export function addAnimationToScript(
241242
export function removeAnimationFromScript(script: string, animationId: string): string {
242243
const parsed = parseGsapScriptAcornForWrite(script);
243244
if (!parsed) return script;
244-
let target = parsed.located.find((l) => l.id === animationId);
245-
if (!target) {
246-
const convertedId = animationId.replace(/-from-|-fromTo-/, "-to-");
247-
target = parsed.located.find((l) => l.id === convertedId);
248-
}
245+
const target = parsed.located.find((l) => l.id === animationId);
249246
if (!target) return script;
250247

251248
const ms = new MagicString(script);

packages/sdk/src/engine/apply-patches.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,9 @@ function applyOne(parsed: ParsedDocument, patch: JsonPatchOp, p: ParsedPath): vo
207207
}
208208

209209
case "script": {
210-
if (patch.op !== "remove") {
210+
if (patch.op === "remove") {
211+
setGsapScript(parsed.document, "");
212+
} else {
211213
setGsapScript(parsed.document, String(patch.value ?? ""));
212214
}
213215
break;

packages/sdk/src/engine/mutate.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -546,6 +546,7 @@ function handleSetGsapTween(
546546
const extras: Record<string, unknown> = {};
547547
if (properties.repeat !== undefined) extras.repeat = properties.repeat;
548548
if (properties.yoyo !== undefined) extras.yoyo = properties.yoyo;
549+
if (properties.stagger !== undefined) extras.stagger = properties.stagger;
549550
if (Object.keys(extras).length > 0) updates.extras = extras;
550551

551552
const newScript = updateAnimationInScript(script, animationId, updates);

0 commit comments

Comments
 (0)