Skip to content

Commit 23563d4

Browse files
vanceingallsclaude
andcommitted
fix(sdk): 8 code-review correctness fixes
- setGsapScript: remove element when newScript="" (fixes undo/redo duplicate-script bug) - parseDeclarations: track quotes so ; inside CSS values (data URIs) doesn't split - handleRemoveGsapKeyframe: guard against duplicate-percentage ambiguity (return EMPTY) - resolveKeyframe: return kfs so callers can check uniqueness - handleSetClassStyle: emit op:"add" (not "replace") when no prior <style> element - FsAdapter listVersions: Number(f.split("_")[0]) — was NaN due to underscore in key - FsAdapter doWrite: split try/catch so appendVersion failure doesn't fire error handlers - FileAdapter playground: add content:"" field to satisfy PersistVersionEntry contract Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 6539cb3 commit 23563d4

5 files changed

Lines changed: 37 additions & 8 deletions

File tree

packages/sdk-playground/src/fileAdapter.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ class FileAdapter implements PersistAdapter {
3232
const res = await fetch("/api/composition/versions");
3333
if (!res.ok) return [];
3434
const rows = (await res.json()) as Array<{ key: string; timestamp?: number }>;
35-
return rows.map((r) => ({ key: r.key, timestamp: r.timestamp }));
35+
return rows.map((r) => ({ key: r.key, content: "", timestamp: r.timestamp }));
3636
}
3737

3838
async loadFrom(_path: string, versionKey: string): Promise<string | undefined> {

packages/sdk/src/adapters/fs.ts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,13 +44,19 @@ class FsAdapter implements PersistAdapter {
4444
}
4545

4646
private async doWrite(path: string, content: string): Promise<void> {
47+
const abs = this.abs(path);
4748
try {
48-
const abs = this.abs(path);
4949
await mkdir(dirname(abs), { recursive: true });
5050
await writeFile(abs, content, "utf8");
51-
await this.appendVersion(path, content);
5251
} catch (err) {
5352
for (const h of this.errorHandlers) h({ error: { message: String(err), cause: err } });
53+
return;
54+
}
55+
// Version archival is best-effort — failure here does not affect the primary write.
56+
try {
57+
await this.appendVersion(path, content);
58+
} catch {
59+
// version history unavailable; primary write succeeded
5460
}
5561
}
5662

@@ -70,7 +76,7 @@ class FsAdapter implements PersistAdapter {
7076
sorted.map(async (f) => ({
7177
key: f.replace(/\.html$/, ""),
7278
content: await readFile(join(dir, f), "utf8"),
73-
timestamp: Number(f.replace(/\.html$/, "")),
79+
timestamp: Number(f.split("_")[0]),
7480
})),
7581
);
7682
} catch {

packages/sdk/src/engine/cssWriter.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,22 @@ function parseCssRules(css: string): CssRule[] {
6161
function parseDeclarations(body: string): Record<string, string> {
6262
const decls: Record<string, string> = {};
6363
let depth = 0;
64+
let quote: string | null = null;
6465
let start = 0;
6566
for (let i = 0; i <= body.length; i++) {
6667
const ch = i < body.length ? body[i]! : ";"; // sentinel flush
68+
if (quote) {
69+
if (ch === "\\") {
70+
i++;
71+
continue;
72+
} // skip escaped char
73+
if (ch === quote) quote = null;
74+
continue;
75+
}
76+
if (ch === '"' || ch === "'") {
77+
quote = ch;
78+
continue;
79+
}
6780
if (ch === "(") depth++;
6881
else if (ch === ")") depth--;
6982
else if (ch === ";" && depth === 0) {

packages/sdk/src/engine/model.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,12 @@ export function getGsapScript(document: Document): string | null {
177177
}
178178

179179
export function setGsapScript(document: Document, newScript: string): void {
180-
let el = findGsapScriptElement(document);
180+
const existing = findGsapScriptElement(document);
181+
if (!newScript) {
182+
existing?.remove();
183+
return;
184+
}
185+
let el = existing;
181186
if (!el) {
182187
el = document.createElement("script") as unknown as Element;
183188
const head =

packages/sdk/src/engine/mutate.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -500,7 +500,9 @@ function handleSetClassStyle(
500500
setStyleSheet(parsed.document, newCss);
501501
const path = styleSheetPath();
502502
return {
503-
forward: [{ op: "replace", path, value: newCss }],
503+
forward: [
504+
oldCss === "" ? { op: "add", path, value: newCss } : { op: "replace", path, value: newCss },
505+
],
504506
inverse: [oldCss === "" ? { op: "remove", path } : { op: "replace", path, value: oldCss }],
505507
};
506508
}
@@ -602,7 +604,7 @@ function resolveKeyframe(parsed: ParsedDocument, animationId: string, keyframeIn
602604
const located = parsedForWrite?.located.find((l) => l.id === animationId);
603605
const kfs = located?.animation.keyframes?.keyframes;
604606
if (!kfs || keyframeIndex < 0 || keyframeIndex >= kfs.length) return null;
605-
return { script, kf: kfs[keyframeIndex]! };
607+
return { script, kf: kfs[keyframeIndex]!, kfs };
606608
}
607609

608610
// fallow-ignore-next-line complexity
@@ -663,8 +665,11 @@ function handleRemoveGsapKeyframe(
663665
): MutationResult {
664666
const resolved = resolveKeyframe(parsed, animationId, keyframeIndex);
665667
if (!resolved) return EMPTY;
666-
const { script, kf } = resolved;
668+
const { script, kf, kfs } = resolved;
667669
const pct = kf.percentage;
670+
// removeKeyframeFromScript matches by percentage; bail if two keyframes share
671+
// the same percentage to avoid removing the wrong one.
672+
if (kfs.filter((k) => k.percentage === pct).length > 1) return EMPTY;
668673
const newScript = removeKeyframeFromScript(script, animationId, pct);
669674
if (newScript === script) return EMPTY;
670675
setGsapScript(parsed.document, newScript);

0 commit comments

Comments
 (0)