Conversation
Contributor
📝 WalkthroughWalkthroughThe PR removes Monaco Editor dependencies and replaces them with CodeMirror editor components across multiple frontend views and components. Adds CodeMirror language extensions for JSON, Python, and XML, plus a merge/diff UI library. Also introduces dynamic schema option population from task data in ReactFormBuilder. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ❌ 3❌ Failed checks (1 warning, 2 inconclusive)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
spiffworkflow-frontend/src/views/ReactFormEditor.tsx (1)
265-269: Use strict equality (===) for string comparisons.Lines 265 and 267 use loose equality (
==) instead of strict equality (===) for comparing strings.Proposed fix
const extensions = []; - if (editorDefaultLanguage == 'json') { + if (editorDefaultLanguage === 'json') { extensions.push(json()); - } else if (editorDefaultLanguage == 'xml') { + } else if (editorDefaultLanguage === 'xml') { extensions.push(xml()); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spiffworkflow-frontend/src/views/ReactFormEditor.tsx` around lines 265 - 269, The comparisons against editorDefaultLanguage use loose equality (==); update both conditional checks to use strict equality (===) so string comparisons are exact: replace occurrences of "editorDefaultLanguage == 'json'" and "editorDefaultLanguage == 'xml'" with strict checks "editorDefaultLanguage === 'json'" and "editorDefaultLanguage === 'xml'" in the ReactFormEditor component where extensions.push(json()) and extensions.push(xml()) are invoked.spiffworkflow-frontend/src/components/ReactFormBuilder/ReactFormBuilder.tsx (1)
354-380: UseforEachinstead ofmapfor side-effect-only iterations, and consider deep cloning.
Object.keys().map()on line 357 is used purely for side effects (modifyingitem.anyOf). UseforEachinstead, asmapis intended for transformations that return values.The shallow copy on line 355 (
{ ...schema }) doesn't deep-cloneschema.properties, so mutations toitem.anyOfwill affect the original schema object. This could cause unexpected behavior ifpostJsonSchemais used elsewhere.Line 364 directly replaces the prefix without first verifying the string starts with
'options_from_task_data_var:'.Proposed fix
function addOptionsToSchema(schema: any, data: any) { - const newSchema = { ...schema }; + const newSchema = JSON.parse(JSON.stringify(schema)); if (typeof schema.properties === 'object') { - Object.keys(schema.properties).map((key) => { + Object.keys(newSchema.properties).forEach((key) => { const item = newSchema.properties[key]; if ( Array.isArray(item.anyOf) && item.anyOf.length > 0 && - typeof item.anyOf[0] === 'string' + typeof item.anyOf[0] === 'string' && + item.anyOf[0].startsWith('options_from_task_data_var:') ) { const name = item.anyOf[0].replace('options_from_task_data_var:', ''); if (Array.isArray(data[name])) { - item.anyOf = data[name].map((opt) => { + item.anyOf = data[name].map((opt: { value: string; label: string }) => { return { type: 'string', enum: [opt.value], title: opt.label, }; }); } else { item.anyOf = []; } } }); } return newSchema; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spiffworkflow-frontend/src/components/ReactFormBuilder/ReactFormBuilder.tsx` around lines 354 - 380, In addOptionsToSchema, replace the misuse of Object.keys(...).map (used only for side effects) with forEach, deep-clone the incoming schema (not just with {...schema}) so mutations to schema.properties and item.anyOf don't affect the caller, and guard the string replacement by checking that item.anyOf[0] is a string that startsWith('options_from_task_data_var:') before calling replace; specifically, deep-clone schema (including schema.properties), iterate Object.keys(newSchema.properties).forEach to locate each item, check Array.isArray(item.anyOf) && typeof item.anyOf[0] === 'string' && item.anyOf[0].startsWith('options_from_task_data_var:'), compute name by slicing or replace, and then set item.anyOf from data[name] (or [] if missing) so original schema remains untouched.spiffworkflow-frontend/package.json (1)
17-17: Remove unused Monaco dependencies to reduce bundle size.The PR adds CodeMirror packages, and Monaco dependencies (
@monaco-editor/reacton line 17 andmonaco-editoron line 49) are no longer imported anywhere in the codebase. These can be safely removed.Also applies to: 49-49
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spiffworkflow-frontend/package.json` at line 17, Remove the unused Monaco packages from package.json by deleting the dependency entries for "@monaco-editor/react" and "monaco-editor"; before committing, search the codebase for any remaining imports/usages of "@monaco-editor/react" or "monaco-editor" (e.g., import statements, dynamic requires) to ensure nothing breaks, and update lockfile (run npm/yarn install) to regenerate package-lock.json or yarn.lock so the unused packages are actually removed from the lockfile.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@spiffworkflow-frontend/src/resources/json_schema_examples/dropdown-schema.json`:
- Line 3: Typo in the JSON "description" value: change the word "form" to "from"
in the dropdown schema's description field (the "description" property in
dropdown-schema.json) so it reads "A dropdown list with options pulled from
existing Task Data. Each item should have label and value attributes."; update
only that string value to correct the spelling.
In `@spiffworkflow-frontend/src/views/ProcessModelEditDiagram.tsx`:
- Around line 758-764: The CodeMirror component's height prop has a typo
('500ps') causing invalid styling; update the height prop on the CodeMirror
instance (the JSX where CodeMirror is rendered with value={inputJson} and
onChange={handleEditorScriptTestUnitInputChange}) to use the correct units
'500px' instead of '500ps' so the editor displays at the intended height.
In `@spiffworkflow-frontend/src/views/ReactFormEditor.tsx`:
- Around line 271-278: The CodeMirror component is passing a numeric height
(600) but expects a CSS string; update the JSX where CodeMirror is rendered (the
CodeMirror element using props height, value bound to processModelFileContents
and onChange calling setProcessModelFileContents) to pass a string with units
like "600px" (or another appropriate unit) instead of the number so the height
prop is correctly interpreted.
---
Nitpick comments:
In `@spiffworkflow-frontend/package.json`:
- Line 17: Remove the unused Monaco packages from package.json by deleting the
dependency entries for "@monaco-editor/react" and "monaco-editor"; before
committing, search the codebase for any remaining imports/usages of
"@monaco-editor/react" or "monaco-editor" (e.g., import statements, dynamic
requires) to ensure nothing breaks, and update lockfile (run npm/yarn install)
to regenerate package-lock.json or yarn.lock so the unused packages are actually
removed from the lockfile.
In `@spiffworkflow-frontend/src/components/ReactFormBuilder/ReactFormBuilder.tsx`:
- Around line 354-380: In addOptionsToSchema, replace the misuse of
Object.keys(...).map (used only for side effects) with forEach, deep-clone the
incoming schema (not just with {...schema}) so mutations to schema.properties
and item.anyOf don't affect the caller, and guard the string replacement by
checking that item.anyOf[0] is a string that
startsWith('options_from_task_data_var:') before calling replace; specifically,
deep-clone schema (including schema.properties), iterate
Object.keys(newSchema.properties).forEach to locate each item, check
Array.isArray(item.anyOf) && typeof item.anyOf[0] === 'string' &&
item.anyOf[0].startsWith('options_from_task_data_var:'), compute name by slicing
or replace, and then set item.anyOf from data[name] (or [] if missing) so
original schema remains untouched.
In `@spiffworkflow-frontend/src/views/ReactFormEditor.tsx`:
- Around line 265-269: The comparisons against editorDefaultLanguage use loose
equality (==); update both conditional checks to use strict equality (===) so
string comparisons are exact: replace occurrences of "editorDefaultLanguage ==
'json'" and "editorDefaultLanguage == 'xml'" with strict checks
"editorDefaultLanguage === 'json'" and "editorDefaultLanguage === 'xml'" in the
ReactFormEditor component where extensions.push(json()) and
extensions.push(xml()) are invoked.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ff2bedd2-bdcd-4383-8eda-ab6128f1b004
⛔ Files ignored due to path filters (1)
spiffworkflow-frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
spiffworkflow-frontend/package.jsonspiffworkflow-frontend/src/components/AuthenticationConfiguration.tsxspiffworkflow-frontend/src/components/ReactFormBuilder/ReactFormBuilder.tsxspiffworkflow-frontend/src/resources/json_schema_examples/dropdown-schema.jsonspiffworkflow-frontend/src/views/Extension.tsxspiffworkflow-frontend/src/views/ProcessInstanceShow.tsxspiffworkflow-frontend/src/views/ProcessModelEditDiagram.tsxspiffworkflow-frontend/src/views/ReactFormEditor.tsx
| { | ||
| "title": "Dropdown list", | ||
| "description": "A dropdown list with options pulled form existing Task Data. IMPORTANT - Add 'fruits' to Task Data before using this component!!!", | ||
| "description": "A dropdown list with options pulled form existing Task Data. Each item should have label and value attributes.", |
Contributor
There was a problem hiding this comment.
Typo in description: "form" should be "from".
The description contains a typo: "pulled form existing Task Data" should be "pulled from existing Task Data".
Proposed fix
- "description": "A dropdown list with options pulled form existing Task Data. Each item should have label and value attributes.",
+ "description": "A dropdown list with options pulled from existing Task Data. Each item should have label and value attributes.",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "description": "A dropdown list with options pulled form existing Task Data. Each item should have label and value attributes.", | |
| "description": "A dropdown list with options pulled from existing Task Data. Each item should have label and value attributes.", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@spiffworkflow-frontend/src/resources/json_schema_examples/dropdown-schema.json`
at line 3, Typo in the JSON "description" value: change the word "form" to
"from" in the dropdown schema's description field (the "description" property in
dropdown-schema.json) so it reads "A dropdown list with options pulled from
existing Task Data. Each item should have label and value attributes."; update
only that string value to correct the spelling.
Comment on lines
+758
to
764
| <CodeMirror | ||
| height={'500ps'} | ||
| width="auto" | ||
| defaultLanguage="json" | ||
| options={Object.assign(jsonEditorOptions(), {})} | ||
| defaultValue={inputJson} | ||
| value={inputJson} | ||
| extensions={[json()]} | ||
| onChange={handleEditorScriptTestUnitInputChange} | ||
| /> |
Contributor
There was a problem hiding this comment.
Typo in height prop: '500ps' should be '500px'.
Line 759 has a typo in the height value.
Proposed fix
<CodeMirror
- height={'500ps'}
+ height={'500px'}
width="auto"
value={inputJson}
extensions={[json()]}
onChange={handleEditorScriptTestUnitInputChange}
/>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <CodeMirror | |
| height={'500ps'} | |
| width="auto" | |
| defaultLanguage="json" | |
| options={Object.assign(jsonEditorOptions(), {})} | |
| defaultValue={inputJson} | |
| value={inputJson} | |
| extensions={[json()]} | |
| onChange={handleEditorScriptTestUnitInputChange} | |
| /> | |
| <CodeMirror | |
| height={'500px'} | |
| width="auto" | |
| value={inputJson} | |
| extensions={[json()]} | |
| onChange={handleEditorScriptTestUnitInputChange} | |
| /> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@spiffworkflow-frontend/src/views/ProcessModelEditDiagram.tsx` around lines
758 - 764, The CodeMirror component's height prop has a typo ('500ps') causing
invalid styling; update the height prop on the CodeMirror instance (the JSX
where CodeMirror is rendered with value={inputJson} and
onChange={handleEditorScriptTestUnitInputChange}) to use the correct units
'500px' instead of '500ps' so the editor displays at the intended height.
Comment on lines
+271
to
278
| <CodeMirror | ||
| height={600} | ||
| width="auto" | ||
| defaultLanguage={editorDefaultLanguage} | ||
| defaultValue={processModelFileContents || ''} | ||
| value={processModelFileContents || ''} | ||
| extensions={extensions} | ||
| onChange={(value) => setProcessModelFileContents(value || '')} | ||
| /> | ||
| ); |
Contributor
There was a problem hiding this comment.
CodeMirror height prop should be a string with units.
The height prop is set to the numeric value 600, but CodeMirror expects a string with CSS units (e.g., '600px'). Other editors in this PR use string values like '600px' or '30rem'.
Proposed fix
return (
<CodeMirror
- height={600}
+ height="600px"
width="auto"
value={processModelFileContents || ''}
extensions={extensions}
onChange={(value) => setProcessModelFileContents(value || '')}
/>
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <CodeMirror | |
| height={600} | |
| width="auto" | |
| defaultLanguage={editorDefaultLanguage} | |
| defaultValue={processModelFileContents || ''} | |
| value={processModelFileContents || ''} | |
| extensions={extensions} | |
| onChange={(value) => setProcessModelFileContents(value || '')} | |
| /> | |
| ); | |
| <CodeMirror | |
| height="600px" | |
| width="auto" | |
| value={processModelFileContents || ''} | |
| extensions={extensions} | |
| onChange={(value) => setProcessModelFileContents(value || '')} | |
| /> | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@spiffworkflow-frontend/src/views/ReactFormEditor.tsx` around lines 271 - 278,
The CodeMirror component is passing a numeric height (600) but expects a CSS
string; update the JSX where CodeMirror is rendered (the CodeMirror element
using props height, value bound to processModelFileContents and onChange calling
setProcessModelFileContents) to pass a string with units like "600px" (or
another appropriate unit) instead of the number so the height prop is correctly
interpreted.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.