test(runner): the batched drop is one commit, and stays one (DEV-2500)

runner/apps/authoring/src/App.tsx

@@ -37,6 +37,7 @@ import { loadStarterExample, toPlaceholderEntry } from "./starter-catalog.js";
 import { DocsCascader, type CascaderLeaf } from "./DocsCascader.js";
 import { currentUser, login, logout, getToken, type User } from "./auth.js";
 import { AdminPanel } from "./Admin.js";
+import { applyDroppedFiles } from "./addFiles.js";
 import { AskAiButton, ChatPanel } from "./Chat.js";
 import { StyleButton, StylePanel } from "./StylePanel.js";
 import { ShareLinks } from "./ShareLinks.js";
@@ -1906,12 +1907,16 @@ function Authoring({
    *
    *  Not `addFile` in a loop: that would be one `setFiles` per file — and on a
    *  Tier-2 framework one dev-server rebuild per file, each invalidating the
-   *  last. One state commit, one dirty-set update, then stream the files. */
+   *  last. One state commit, one dirty-set update, then stream the files.
+   *
+   *  The map math lives in `addFiles.ts` under `pipeline/add-files.test.mjs`,
+   *  which also greps this function: one `setFiles`, one ref commit, through
+   *  `applyDroppedFiles`. Keep it that shape. */
   const addFiles = useCallback(
     (dropped: { path: string; contents: string }[]) => {
-      if (!dropped.length) return;
-      const next = { ...filesRef.current };
-      for (const { path, contents } of dropped) next[path] = contents;
+      const next = applyDroppedFiles(filesRef.current, dropped);
+      // Reference-equal means an empty drop: nothing to commit, render or push.
+      if (next === filesRef.current) return;
       filesRef.current = next;
       setFiles(next);
       // Variadic on purpose (see its definition): one call dots every dropped tab.

runner/apps/authoring/src/addFiles.ts

@@ -0,0 +1,49 @@
+// The state commit of a drag & drop (DEV-2500), as a pure map transform.
+//
+// Extracted from `App.tsx`'s `addFiles` so the batching requirement stops being
+// a code comment and becomes an importable contract: the ticket calls the
+// batched `onAddFiles` necessary, not cosmetic — N sequential `addFile` calls
+// would mean N `setFiles` renders and, on a Tier-2 framework, N container
+// pushes each invalidating the last. `pipeline/add-files.test.mjs` pins the
+// shape down; `App.tsx` keeps the side effects (the ref commit, the render,
+// the dirty dots, the runtime writes) and delegates the map math here.
+//
+// Dependency-free on purpose, like `demoOwners.ts` and `markdownActions.ts`:
+// a module the pipeline tests load cannot import a sibling `./x.js` under
+// `--experimental-strip-types`. The one import is type-only, which the
+// stripper erases.
+
+import type { FilesMap } from "@handsontable/demo-runtime";
+
+/** A dropped file, in the shape the FILES tree hands over (`onAddFiles`). */
+export interface DroppedWorkspaceFile {
+  path: string;
+  contents: string;
+}
+
+/**
+ * Fold one dropped batch into the workspace, in one step.
+ *
+ * This is DEV-2500's batching contract, and `pipeline/add-files.test.mjs`
+ * holds it to exactly this:
+ *
+ * - A non-empty drop returns ONE new map — never the input mutated — carrying
+ *   every existing file plus every dropped one. That single object is what
+ *   `App.tsx` commits (one `filesRef` assignment, one `setFiles`), so the
+ *   whole drop is one render and one rebuild.
+ * - A colliding path is overwritten in place. The FILES tree has already
+ *   asked ("Overwrite" vs "Keep both") before the batch gets here, and
+ *   "Keep both" renames before the call — a collision arriving here *means*
+ *   overwrite.
+ * - An empty drop returns `current` itself (reference-equal), so the caller
+ *   can compare and skip the commit entirely.
+ */
+export function applyDroppedFiles(
+  current: FilesMap,
+  dropped: DroppedWorkspaceFile[],
+): FilesMap {
+  if (!dropped.length) return current;
+  const next: FilesMap = { ...current };
+  for (const { path, contents } of dropped) next[path] = contents;
+  return next;
+}

runner/pipeline/add-files.test.mjs

@@ -0,0 +1,105 @@
+// The batched drop commit (DEV-2500). The ticket calls the batched `onAddFiles`
+// necessary, not cosmetic — N sequential `addFile` calls would be N `setFiles`
+// renders and N container pushes each invalidating the last — yet until now the
+// contract survived only as a code comment; a per-file loop would have passed
+// every existing e2e (they only check the rows appear). Two halves here:
+// `applyDroppedFiles` (the pure map transform `App.tsx` commits) imported and
+// exercised directly, and a source grep holding `App.tsx`'s `addFiles` to the
+// one-commit shape.
+//
+// Run: node --experimental-strip-types --test pipeline/*.test.mjs
+import test from "node:test";
+import assert from "node:assert/strict";
+import { readFileSync } from "node:fs";
+import { fileURLToPath } from "node:url";
+import { dirname, join } from "node:path";
+import { applyDroppedFiles } from "../apps/authoring/src/addFiles.ts";
+
+test("a drop lands as one new files map, not N", () => {
+  const current = { "/index.ts": "entry", "/styles.css": "body {}" };
+  const snapshot = structuredClone(current);
+  const next = applyDroppedFiles(current, [
+    { path: "/a.ts", contents: "A" },
+    { path: "/b.css", contents: "B" },
+    { path: "/data/rows.json", contents: "[]" },
+  ]);
+  // A NEW map: this one object is the whole commit — one setFiles, one rebuild.
+  assert.notEqual(next, current);
+  assert.deepEqual(next, {
+    "/index.ts": "entry",
+    "/styles.css": "body {}",
+    "/a.ts": "A",
+    "/b.css": "B",
+    "/data/rows.json": "[]",
+  });
+  // ...and the input was read, never written.
+  assert.deepEqual(current, snapshot);
+});
+
+test("a colliding path is overwritten in place", () => {
+  // By the time a batch reaches this helper the FILES tree has already asked
+  // ("Overwrite" vs "Keep both") and "Keep both" renamed before the call — see
+  // FileTree's commit(). A collision arriving here therefore means overwrite.
+  const current = { "/index.ts": "old", "/other.ts": "kept" };
+  const next = applyDroppedFiles(current, [{ path: "/index.ts", contents: "new" }]);
+  assert.deepEqual(next, { "/index.ts": "new", "/other.ts": "kept" });
+  // Overwritten in the new map; the input still holds the old contents.
+  assert.equal(current["/index.ts"], "old");
+});
+
+test("an empty drop returns the same map", () => {
+  // Reference-equal on purpose: App.tsx compares against filesRef.current and
+  // skips the whole commit when nothing was dropped.
+  const current = { "/index.ts": "entry" };
+  assert.equal(applyDroppedFiles(current, []), current);
+});
+
+// ---- the source guard: App.tsx's addFiles keeps the one-commit shape --------
+
+/**
+ * Slice `addFiles`'s whole `useCallback(...)` argument list out of App.tsx by
+ * balanced-paren scan. Honest fragility note: the scan counts every `(` and
+ * `)` including those inside comments and strings — today they all come in
+ * pairs, and the sanity assertions below (the slice ends, and contains the
+ * pieces it must) turn a formatting change that breaks the scan into a loud
+ * failure here rather than a silent pass.
+ */
+function addFilesSlice(source) {
+  const marker = "const addFiles = useCallback(";
+  const start = source.indexOf(marker);
+  assert.notEqual(start, -1, "App.tsx no longer declares `const addFiles = useCallback(`");
+  assert.equal(source.indexOf(marker, start + 1), -1, "expected exactly one addFiles declaration");
+  let depth = 0;
+  for (let i = start + marker.length - 1; i < source.length; i += 1) {
+    if (source[i] === "(") depth += 1;
+    else if (source[i] === ")") depth -= 1;
+    if (depth === 0) return source.slice(start, i + 1);
+  }
+  assert.fail("unbalanced parens scanning addFiles — fix the slice helper, not the assertion");
+}
+
+test("App.tsx's addFiles keeps the one-commit shape", () => {
+  const root = join(dirname(fileURLToPath(import.meta.url)), "..");
+  const app = readFileSync(join(root, "apps/authoring/src/App.tsx"), "utf8");
+  const body = addFilesSlice(app);
+
+  // Sanity: the slice really is the drop handler, whole.
+  assert.match(body, /markDirty\(/, "slice lost the dirty marking — the scan broke");
+  assert.match(body, /writeFile\(/, "slice lost the runtime writes — the scan broke");
+
+  // The contract: the map math goes through the tested helper...
+  assert.match(body, /applyDroppedFiles\(/, "addFiles must build its map via applyDroppedFiles");
+  // ...and lands as ONE state commit. Two `setFiles(` here means the batch
+  // regressed to a loop — exactly what DEV-2500 exists to prevent.
+  assert.equal(
+    (body.match(/setFiles\(/g) ?? []).length,
+    1,
+    "addFiles must call setFiles exactly once per drop",
+  );
+  // One ref commit too ([^=] keeps `next === filesRef.current` from matching).
+  assert.equal(
+    (body.match(/filesRef\.current\s*=[^=]/g) ?? []).length,
+    1,
+    "addFiles must assign filesRef.current exactly once per drop",
+  );
+});