feat: enhance BulkEditor with local state management for editor text #5374
Conversation
sanjaikumar-bruno
requested review from
helloanoop,
maintainer-bruno,
bijin-bruno,
lohit-bruno and
naman-bruno
as code owners
|
@sanjaikumar-bruno add jira also brief description, screen-recording of the issue |
| @@ -8,11 +8,23 @@ const BulkEditor = ({ params, onChange, onToggle, onSave, onRun }) => { | |||
| const preferences = useSelector((state) => state.app.preferences); | |||
| const { displayedTheme } = useTheme(); | |||
|
|
|||
| const parsedParams = useMemo(() => serializeBulkKeyValue(params), [params]); | |||
| const itemsText = useMemo(() => serializeBulkKeyValue(params), [params]); | |||
There was a problem hiding this comment.
@sanjaikumar-bruno i distinctly remember we introduced parsedParams useMemo inorder to avoid two sources of truth and the inconsistencies that get introduced with it. I remember the usage useState was causing inconsistencies within the editor behaviour, This change may cause a regression in editor behaviour. will you be able to test the behaviour throughly?
| // Editor local state | ||
| const [editorText, setEditorText] = useState(itemsText); | ||
|
|
||
| useEffect(() => { |
There was a problem hiding this comment.
you are changing a state that is also a dependency of the useEffect, which will cause infinite loops and can crash the application. This might not be ideal. @sanjaikumar-bruno
We introduced useMemo to avoid this behaviour. earlier!
| @@ -22,7 +34,7 @@ const BulkEditor = ({ params, onChange, onToggle, onSave, onRun }) => { | |||
| mode="text/plain" | |||
| theme={displayedTheme} | |||
| font={preferences.codeFont || 'default'} | |||
| value={parsedParams} | |||
| value={editorText} | |||
There was a problem hiding this comment.
I believe, we will haven't found root cause of this issue.
No description provided.