fix(plugin-page-builder,builder): a class id is a key, not a property name - #1874
Conversation
… 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.
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Warning Review limit reachedNext included review available in 2 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e482605869
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| pendingSlugs: Readonly<Record<string, string>> | undefined, | ||
| classId: string | ||
| ): string | undefined { | ||
| if (pendingSlugs === undefined || !Object.hasOwn(pendingSlugs, classId)) { |
There was a problem hiding this comment.
When a JavaScript or loosely typed host passes pendingSlugs={null}, the previous optional-chain lookup treated it like an omitted map, but this condition proceeds to Object.hasOwn(null, classId) and throws as soon as a class row renders, taking down the manager. Include null in the absence guard (or normalize the prop before this call) so the new own-property check preserves the former runtime behavior.
AGENTS.md reference: AGENTS.md:L303-L307
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Reproduced and fixed in e2d0e29. Passing pendingSlugs={null} made Object.hasOwn(null, classId) throw while the rows were drawn, where the optional index it replaced answered nothing. pendingSlugFor now reads null as no record, as an omitted one is; the prop's type is unchanged. New test: draws the list when a host passes null for the pending names. With the null guard removed it fails (1 failed | 75 passed), and with it restored the file passes 76/76. Builder suite 3,218 passed, plugin-page-builder 1,089, check-types 44/44, lint, comment convention and fallow all pass.
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.
|
@codex review |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
@nextlyhq/adapter-drizzle
@nextlyhq/adapter-mysql
@nextlyhq/adapter-postgres
@nextlyhq/adapter-sqlite
@nextlyhq/admin
@nextlyhq/admin-css
@nextlyhq/blocks-engine
@nextlyhq/blocks-react
@nextlyhq/builder
create-nextly-app
@nextlyhq/eslint-plugin
nextly
@nextlyhq/plugin-form-builder
@nextlyhq/plugin-mcp
@nextlyhq/plugin-page-builder
@nextlyhq/plugin-sdk
@nextlyhq/plugin-seo
@nextlyhq/storage-s3
@nextlyhq/storage-uploadthing
@nextlyhq/storage-vercel-blob
@nextlyhq/ui
commit: |
github-actions
Bot
added
type: docs
1 participant
Why
Codex flagged on #1857 that the rename-attempt counters are a plain object keyed by class id.
isUsableNamedClassaccepts any id, and for__proto__the read returnsObject.prototypewhile the write goes through the prototype setter and stores nothing.endRenamethen never matches, the pending name never clears, and after a refused save retrying the same name is dropped as "no change".The pending-name record beside it has the same shape one layer out. The class manager panel read
pendingSlugs?.[row.id], which for__proto__is an object, so a name typed away and back was sent as a rename.What changed
useClassSurface(plugin-page-builder) keeps the counters in aMap, and removes a pending name only when the record really holds one (Object.hasOwn).ClassManagerPanel(builder) reads a pending name as the record's own entry, the sameObject.hasOwnpatternclass-library.ts,instance-inspector.tsandstyle-values.tsalready use. The prop type is unchanged.Evidence
Reproduced first, as two tests that fail on the old code:
class-manager-panel.test.tsx: treats a name typed away and back as no rename, whatever the class idBlocksField.classesPanel.test.tsx: lets a class whose id is proto retry a rename its save refusedBreak-verified by writing the old implementation back (mutation confirmed applied, file restored byte-for-byte afterwards):
pendingSlugs?.[row.id])An own-key check on the pending name inside
beginRenamewas tried and dropped: it changes no outcome (a string name never equals the inherited object), so no test could hold it.pnpm check-typesorigin/main)