Simplify pattern properties UI#43
Conversation
Deploying jsonjoy-builder with
|
| Latest commit: |
5ae5b30
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://02835608.jsonjoy-builder.pages.dev |
| Branch Preview URL: | https://pattern-properties.jsonjoy-builder.pages.dev |
There was a problem hiding this comment.
💡 Codex Review
jsonjoy-builder/src/components/SchemaEditor/SchemaPropertyEditor.tsx
Lines 90 to 93 in 5ae5b30
Pattern-property names are regexes, but rename submission forwards any non-empty string directly to onNameChange without validating it. In PatternSchemaPropertyEditor, this allows editing a valid pattern into an invalid one (for example (), which writes an invalid patternProperties key into the schema and can break downstream validator compilation (e.g., AJV). The add-flow already validates regex syntax, so the edit-flow should enforce the same check before applying the rename.
ℹ️ 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".
| newName, | ||
| propertySchemaObj, | ||
| ); | ||
| onChange(renameObjectProperty(normalizedSchema, oldName, newName)); |
There was a problem hiding this comment.
Preserve renamed field when target name already exists
This rename path now calls renameObjectProperty directly, which rebuilds the object by key iteration; if newName already exists, whichever key is encountered last wins. In nested object editing this means renaming a to b can silently drop the renamed field schema (or overwrite the existing one depending on order), producing inconsistent data-loss behavior from a single name edit. The previous flow updated the renamed key explicitly before removal, so this regression should be guarded by duplicate-name validation or a deterministic merge policy.
Useful? React with 👍 / 👎.
|
a toggle to move pattern property to properties and likewise would be helpful |
closes #33