Add Raw JSON Form Field Input #9078
Conversation
Devessier
force-pushed
the
add-uuid-form-field-input
branch
2 times, most recently
from
480eb2e to
34e959b
Compare
…ngine will do on the backend Users have to put double quotes where they want to create strings.
Devessier
force-pushed
the
add-uuid-form-field-input
branch
from
34e959b to
90e7dc2
Compare
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
Implemented a new FormRawJsonFieldInput component for handling JSON validation and variable tag insertion in form fields, with support for variable tags in JSON content.
- Added
FormRawJsonFieldInput.tsxwith JSON validation that replaces variable tags with '0' for validation and converts empty strings to null - Added comprehensive Storybook tests in
FormRawJsonFieldInput.stories.tsxcovering JSON validation, variable insertion, and edge cases - Added
renderTextmethod tovariableTag.tsto return raw variable attributes (e.g., '{{test}}') - Exported
CAPTURE_VARIABLE_TAG_REGEXandCAPTURE_ALL_VARIABLE_TAG_INNER_REGEXconstants for reuse in variable tag parsing - Modified
FormFieldInput.tsxto support the new RawJson field type
6 file(s) reviewed, 7 comment(s)
Edit PR Review Bot Settings | Greptile
| ) : isFieldRawJson(field) ? ( | ||
| <FormRawJsonFieldInput | ||
| label={field.label} | ||
| defaultValue={defaultValue as string | undefined} |
There was a problem hiding this comment.
logic: Type casting to string | undefined may be insufficient for raw JSON - consider using JsonValue type instead to properly handle nested objects and arrays
| await Promise.all([ | ||
| userEvent.type(editor, '{{ "a": {{ "b" : "d" } }'), | ||
|
|
||
| waitFor(() => { | ||
| expect(args.onPersist).toHaveBeenCalledWith('{ "a": { "b" : "d" } }'); | ||
| }), |
There was a problem hiding this comment.
logic: Using Promise.all here is unnecessary since there's only one waitFor condition. This could lead to race conditions if more promises are added later.
| await Promise.all([ | ||
| userEvent.type(editor, `{Backspace>${defaultValueStringLength}}`), | ||
|
|
||
| waitFor(() => { | ||
| expect(args.onPersist).toHaveBeenCalledWith(null); | ||
| }), |
There was a problem hiding this comment.
logic: Using Promise.all with a single waitFor condition is unnecessary and could mask timing issues. Consider removing Promise.all wrapper.
| let isValidJson; | ||
| try { | ||
| JSON.parse(textWithReplacedVariables); | ||
|
|
||
| isValidJson = true; | ||
| } catch { | ||
| isValidJson = false; | ||
| } |
There was a problem hiding this comment.
logic: Empty catch block silently fails JSON validation. Consider providing feedback to user about invalid JSON format.
| const textWithReplacedVariables = text.replaceAll( | ||
| new RegExp(CAPTURE_VARIABLE_TAG_REGEX, 'g'), | ||
| '0', | ||
| ); |
There was a problem hiding this comment.
style: Replacing variables with '0' could validate successfully but produce invalid JSON when actual variable values are longer/shorter. Consider using a placeholder that matches expected value length.
| const handleVariableTagInsert = (variableName: string) => { | ||
| if (!isDefined(editor)) { | ||
| throw new Error( | ||
| 'Expected the editor to be defined when a variable is selected', | ||
| ); | ||
| } | ||
|
|
||
| editor.commands.insertVariableTag(variableName); | ||
| }; |
There was a problem hiding this comment.
logic: handleVariableTagInsert throws if editor undefined, but component renders null in that case. This error should never occur - consider removing the check or logging a warning instead.
| }: FormRawJsonFieldInputProps) => { | ||
| const inputId = useId(); | ||
|
|
||
| const editor = useTextVariableEditor({ |
There was a problem hiding this comment.
style: onUpdate callback is recreated on every render. Consider memoizing with useCallback to prevent unnecessary re-renders.
renderTextmethod forVariableTag. This method returns thevariableattribute of the node, i.e.{{test}}.editor.getText()function to simply get the textual representation of the field without relying oneditor.getJSON()andparseEditorContent.TextVariableEditorcomponent.RawJsonFieldInputfield, especially the JSON validation.Closes https://github.com/twentyhq/private-issues/issues/180