fix(plugin-page-builder,builder): a class id is a key, not a property name

.changeset/a-class-id-is-a-key-not-a-property.md

@@ -0,0 +1,39 @@
+---
+"@nextlyhq/adapter-drizzle": patch
+"@nextlyhq/adapter-mysql": patch
+"@nextlyhq/adapter-postgres": patch
+"@nextlyhq/adapter-sqlite": patch
+"@nextlyhq/admin": patch
+"@nextlyhq/admin-css": patch
+"@nextlyhq/blocks-engine": patch
+"@nextlyhq/blocks-react": patch
+"@nextlyhq/builder": patch
+"create-nextly-app": patch
+"@nextlyhq/eslint-config": patch
+"@nextlyhq/eslint-plugin": patch
+"@nextlyhq/module-specifiers": patch
+"nextly": patch
+"@nextlyhq/plugin-form-builder": patch
+"@nextlyhq/plugin-mcp": patch
+"@nextlyhq/plugin-page-builder": patch
+"@nextlyhq/plugin-sdk": patch
+"@nextlyhq/plugin-seo": patch
+"@nextlyhq/prettier-config": patch
+"@nextlyhq/storage-s3": patch
+"@nextlyhq/storage-uploadthing": patch
+"@nextlyhq/storage-vercel-blob": patch
+"@nextlyhq/telemetry": patch
+"@nextlyhq/tsconfig": patch
+"@nextlyhq/ui": patch
+---
+
+A class whose id was `__proto__` could not be renamed again after a refused
+save. The class manager records which rename of each class is the live one, and
+it kept that record on a plain object keyed by class id, where `__proto__` reads
+back an inherited object and a write to it stores nothing. The refused rename
+never released its pending name, so retrying the same name was taken as no
+change at all.
+
+The record is now a `Map`, and the pending names are read as the record's own
+entries in both the editor and the class manager panel, so a class behaves the
+same whatever id it carries.

packages/builder/src/class-manager-panel.test.tsx

@@ -266,6 +266,34 @@ describe("renaming in place", () => {
     expect(onRename).not.toHaveBeenCalled();
   });
 
+  it("treats a name typed away and back as no rename, whatever the class id", () => {
+    /*
+     * A class id is stored data, and a pending-name record indexed by it can
+     * answer an inherited property instead of an entry. For `__proto__` that
+     * answer is an object, so a name typed away and back was compared against
+     * it rather than against the class's own name, and sent as a rename.
+     */
+    const { onRename } = draw({
+      library: [cls("__proto__", "odd", 0)],
+      pendingSlugs: {},
+    });
+    fireEvent.change(nameField("odd"), { target: { value: "od" } });
+    fireEvent.change(nameField("odd"), { target: { value: "odd" } });
+    fireEvent.keyDown(nameField("odd"), { key: "Enter" });
+    expect(onRename).not.toHaveBeenCalled();
+  });
+
+  it("draws the list when a host passes null for the pending names", () => {
+    // The prop's type excludes null, but a host written in JavaScript can pass
+    // it, and an own-key check on null throws while the rows are being drawn.
+    const { onRename } = draw({
+      pendingSlugs: null as unknown as Record<string, string>,
+    });
+    fireEvent.change(nameField("hero"), { target: { value: "banner" } });
+    fireEvent.keyDown(nameField("hero"), { key: "Enter" });
+    expect(onRename).toHaveBeenCalledWith("id-hero", "banner");
+  });
+
   it("treats a name typed away and back as no rename at all", () => {
     /*
      * Two things at once, and both matter. Its own slug is not a COLLISION, so

packages/builder/src/class-manager-panel.tsx

@@ -704,6 +704,28 @@ function usablePageSize(pageSize: number | undefined): number {
     : DEFAULT_PAGE_SIZE;
 }
 
+/**
+ * The name a class is heading for, read as the record's OWN entry.
+ *
+ * A class id is stored data, so this record can be asked about any string. An
+ * index answers an inherited property for some of them, `__proto__` among them,
+ * and the row would then compare an edit against that object rather than a
+ * name, sending a name typed away and back as a rename.
+ *
+ * `null` is taken as no record, as an omitted one is. The prop's type excludes
+ * it, but a host written in JavaScript can still pass it, and `Object.hasOwn`
+ * throws on it where an optional index would have answered nothing.
+ */
+function pendingSlugFor(
+  pendingSlugs: Readonly<Record<string, string>> | null | undefined,
+  classId: string
+): string | undefined {
+  if (pendingSlugs === undefined || pendingSlugs === null) return undefined;
+  return Object.hasOwn(pendingSlugs, classId)
+    ? pendingSlugs[classId]
+    : undefined;
+}
+
 function ClassList({
   rows,
   searching,
@@ -793,7 +815,7 @@ function ClassList({
           <li key={row.id} className="nx-classman__row">
             <ClassRowView
               row={row}
-              pendingSlug={pendingSlugs?.[row.id]}
+              pendingSlug={pendingSlugFor(pendingSlugs, row.id)}
               library={library}
               styles={stylesById.get(row.id)}
               styleContext={styleContext}

packages/plugin-page-builder/src/admin/BlocksField.classesPanel.test.tsx

@@ -648,6 +648,47 @@ describe("the classes manager reaching an author", () => {
     expect(last).toContain('"card"');
   });
 
+  it("lets a class whose id is __proto__ retry a rename its save refused", async () => {
+    /*
+     * The rename identity is keyed by class id, and a class id is stored data.
+     * On a plain record `__proto__` read back an inherited object and wrote
+     * through the prototype setter, so the refused attempt never released its
+     * pending name, and the retry, judged against that name, was dropped as no
+     * change at all.
+     */
+    storedRead = {
+      data: {
+        classes: [{ id: "__proto__", slug: "card", orderIndex: 0, styles: {} }],
+      },
+      isPending: false,
+      error: null,
+    };
+    saveResult = new Error("The site style is locked.");
+    openEditor();
+
+    await act(async () => {
+      fireEvent.change(screen.getByLabelText("Name of card"), {
+        target: { value: "panel" },
+      });
+      fireEvent.blur(screen.getByLabelText("Name of card"));
+    });
+    expect(await screen.findByText("The site style is locked.")).toBeTruthy();
+    const refused = saved.length;
+    expect(refused).toBeGreaterThan(0);
+
+    saveResult = { success: true };
+    await act(async () => {
+      fireEvent.change(screen.getByLabelText("Name of card"), {
+        target: { value: "panel" },
+      });
+      fireEvent.blur(screen.getByLabelText("Name of card"));
+    });
+
+    // Written again, rather than swallowed as a no-op against a stuck name.
+    await vi.waitFor(() => expect(saved.length).toBeGreaterThan(refused));
+    expect(JSON.stringify(saved[saved.length - 1])).toContain('"panel"');
+  });
+
   it("says a failed read failed, rather than loading forever", () => {
     // A read that FAILED will not finish. A panel still saying "loading"
     // describes a state the site is not in, and the author waits for something

packages/plugin-page-builder/src/admin/BlocksField.tsx

@@ -1172,11 +1172,15 @@ function useClassSurface(
    * A ref rather than state because both readers need the value SYNCHRONOUSLY —
    * the panel reads it in the same event that started the attempt, and a state
    * update is not visible until the render after.
+   *
+   * A `Map` rather than a record, because a class id is stored data: every string
+   * a class can carry has to be a key here, and on a plain object some strings,
+   * `__proto__` among them, name an inherited accessor instead of an entry.
    */
-  const renameAttempts = useRef<Record<string, number>>({});
+  const renameAttempts = useRef(new Map<string, number>());
   const beginRename = useCallback((classId: string, slug: string): number => {
-    const mine = (renameAttempts.current[classId] ?? 0) + 1;
-    renameAttempts.current[classId] = mine;
+    const mine = (renameAttempts.current.get(classId) ?? 0) + 1;
+    renameAttempts.current.set(classId, mine);
     setPendingSlugs(current =>
       current[classId] === slug ? current : { ...current, [classId]: slug }
     );
@@ -1191,16 +1195,19 @@ function useClassSurface(
      * reads as a no-op while the queued rename goes on to persist a different
      * name.
      */
-    if (renameAttempts.current[classId] !== mine) return;
+    if (renameAttempts.current.get(classId) !== mine) return;
     setPendingSlugs(current => {
-      if (!(classId in current)) return current;
+      // The record's OWN entry, for the reason the counters are a `Map`: `in`
+      // also answers for inherited names, and removing one that is not there
+      // would rebuild the record for nothing.
+      if (!Object.hasOwn(current, classId)) return current;
       const { [classId]: _gone, ...rest } = current;
       return rest;
     });
   }, []);
   /** The live attempt for a class, for a caller deciding whether it is current. */
   const currentRenameAttempt = useCallback(
-    (classId: string): number => renameAttempts.current[classId] ?? 0,
+    (classId: string): number => renameAttempts.current.get(classId) ?? 0,
     []
   );
   return {