From e482605869a1bfff2c61f26e29739832dad6167f Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Sun, 13 Sep 2026 19:01:50 +0300 Subject: [PATCH 1/2] fix(plugin-page-builder,builder): a class id is a key, not a property name The class manager kept which rename of each class is live on a plain object keyed by class id. For the id __proto__ the read returned Object.prototype and the write stored nothing, so a refused rename never released its pending name and a retry of the same name was dropped as no change. The panel read pending names the same way, so a name typed away and back was sent as a rename. The counters are now a Map, and pending names are read as the record's own entries on both sides of the prop. --- .../a-class-id-is-a-key-not-a-property.md | 39 ++++++++++++++++++ .../builder/src/class-manager-panel.test.tsx | 17 ++++++++ packages/builder/src/class-manager-panel.tsx | 20 ++++++++- .../admin/BlocksField.classesPanel.test.tsx | 41 +++++++++++++++++++ .../src/admin/BlocksField.tsx | 19 ++++++--- 5 files changed, 129 insertions(+), 7 deletions(-) create mode 100644 .changeset/a-class-id-is-a-key-not-a-property.md diff --git a/.changeset/a-class-id-is-a-key-not-a-property.md b/.changeset/a-class-id-is-a-key-not-a-property.md new file mode 100644 index 0000000000..f78f0b5c9f --- /dev/null +++ b/.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. diff --git a/packages/builder/src/class-manager-panel.test.tsx b/packages/builder/src/class-manager-panel.test.tsx index f625927cae..c66d2cf21e 100644 --- a/packages/builder/src/class-manager-panel.test.tsx +++ b/packages/builder/src/class-manager-panel.test.tsx @@ -266,6 +266,23 @@ 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("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 diff --git a/packages/builder/src/class-manager-panel.tsx b/packages/builder/src/class-manager-panel.tsx index 256042ced6..3883219938 100644 --- a/packages/builder/src/class-manager-panel.tsx +++ b/packages/builder/src/class-manager-panel.tsx @@ -704,6 +704,24 @@ 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. + */ +function pendingSlugFor( + pendingSlugs: Readonly> | undefined, + classId: string +): string | undefined { + if (pendingSlugs === undefined || !Object.hasOwn(pendingSlugs, classId)) { + return undefined; + } + return pendingSlugs[classId]; +} + function ClassList({ rows, searching, @@ -793,7 +811,7 @@ function ClassList({
  • { 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 diff --git a/packages/plugin-page-builder/src/admin/BlocksField.tsx b/packages/plugin-page-builder/src/admin/BlocksField.tsx index e95a7764da..aaa85f0bc5 100644 --- a/packages/plugin-page-builder/src/admin/BlocksField.tsx +++ b/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>({}); + const renameAttempts = useRef(new Map()); 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 { From e2d0e299a2c34f509b24f1855d9bf2fc19c609d8 Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Sun, 13 Sep 2026 19:21:38 +0300 Subject: [PATCH 2/2] fix(builder): draw the class list when pending names arrive as null The pending-name lookup now asks for an own entry, and Object.hasOwn throws on null where the optional index it replaced answered nothing. The prop's type excludes null, but a host written in JavaScript can pass it, and the throw took the whole class manager down while its rows were drawn. null is now read as no record, as an omitted one is. --- packages/builder/src/class-manager-panel.test.tsx | 11 +++++++++++ packages/builder/src/class-manager-panel.tsx | 14 +++++++++----- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/packages/builder/src/class-manager-panel.test.tsx b/packages/builder/src/class-manager-panel.test.tsx index c66d2cf21e..892a5423c7 100644 --- a/packages/builder/src/class-manager-panel.test.tsx +++ b/packages/builder/src/class-manager-panel.test.tsx @@ -283,6 +283,17 @@ describe("renaming in place", () => { 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, + }); + 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 diff --git a/packages/builder/src/class-manager-panel.tsx b/packages/builder/src/class-manager-panel.tsx index 3883219938..e1842e769c 100644 --- a/packages/builder/src/class-manager-panel.tsx +++ b/packages/builder/src/class-manager-panel.tsx @@ -711,15 +711,19 @@ function usablePageSize(pageSize: number | undefined): number { * 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> | undefined, + pendingSlugs: Readonly> | null | undefined, classId: string ): string | undefined { - if (pendingSlugs === undefined || !Object.hasOwn(pendingSlugs, classId)) { - return undefined; - } - return pendingSlugs[classId]; + if (pendingSlugs === undefined || pendingSlugs === null) return undefined; + return Object.hasOwn(pendingSlugs, classId) + ? pendingSlugs[classId] + : undefined; } function ClassList({