Skip to content

Commit 212da86

Browse files
authored
Merge pull request #762 from devtron-labs/fix/diff-viewer
fix: CodeEditor - reinitialize diff view on external value change to ensure consistent state
2 parents 3895156 + 4a9c31e commit 212da86

File tree

6 files changed

+81
-66
lines changed

6 files changed

+81
-66
lines changed

package-lock.json

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@devtron-labs/devtron-fe-common-lib",
3-
"version": "1.14.2-pre-2",
3+
"version": "1.14.2-pre-3",
44
"description": "Supporting common component library",
55
"type": "module",
66
"main": "dist/index.js",

src/Shared/Components/CICDHistory/DeploymentHistoryConfigDiff/DeploymentHistoryDiffView.tsx

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ const DeploymentHistoryDiffView = ({
3232
baseTemplateConfiguration,
3333
previousConfigAvailable,
3434
rootClassName,
35-
codeEditorKey,
3635
}: DeploymentTemplateHistoryType) => {
3736
const { historyComponent, historyComponentName } = useParams<DeploymentHistoryParamsType>()
3837

@@ -79,15 +78,7 @@ const DeploymentHistoryDiffView = ({
7978
collapseUnchangedDiffView
8079
/>
8180
) : (
82-
<CodeEditor
83-
key={codeEditorKey}
84-
value={editorValuesRHS}
85-
height="auto"
86-
disableSearch
87-
readOnly
88-
noParsing
89-
mode={MODES.YAML}
90-
/>
81+
<CodeEditor value={editorValuesRHS} height="auto" disableSearch readOnly noParsing mode={MODES.YAML} />
9182
)
9283

9384
const handleShowVariablesClick = () => {

src/Shared/Components/CICDHistory/types.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -550,8 +550,8 @@ export interface DeploymentTemplateHistoryType {
550550
baseTemplateConfiguration: DeploymentHistoryDetail
551551
previousConfigAvailable: boolean
552552
rootClassName?: string
553-
codeEditorKey?: string
554553
}
554+
555555
export interface DeploymentHistoryDetailRes extends ResponseType {
556556
result?: DeploymentHistoryDetail
557557
}

src/Shared/Components/CodeEditor/CodeEditorRenderer.tsx

Lines changed: 76 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ export const CodeEditorRenderer = ({
6161
// REFS
6262
const codeMirrorRef = useRef<ReactCodeMirrorRef>()
6363
const codeMirrorMergeParentRef = useRef<HTMLDivElement>()
64+
const codeMirrorMergeRef = useRef<MergeView>()
6465
const diffMinimapRef = useRef<MergeView>()
6566
const diffMinimapParentRef = useRef<HTMLDivElement>()
6667

@@ -183,78 +184,102 @@ export const CodeEditorRenderer = ({
183184
}
184185
})
185186

186-
useEffect(() => {
187-
// DIFF VIEW INITIALIZATION
188-
if (!loading && codeMirrorMergeParentRef.current) {
189-
const scanLimit = getScanLimit(lhsValue, value)
187+
/**
188+
* Initializes or reinitializes the CodeMirror merge view for diff comparison.
189+
*
190+
* This function:
191+
* 1. Destroys any existing merge view instances to prevent memory leaks
192+
* 2. Creates a new MergeView instance with the current values and configurations
193+
* 3. Initializes the diff minimap if enabled
194+
* 4. Updates the component state with the new instances
195+
*
196+
*/
197+
const initializeCodeMirrorMergeView = () => {
198+
// Destroy existing merge view instance if it exists
199+
codeMirrorMergeInstance?.destroy()
200+
codeMirrorMergeRef.current?.destroy()
201+
202+
const codeMirrorMergeView = new MergeView({
203+
a: {
204+
doc: lhsValue,
205+
extensions: [...originalViewExtensions, originalUpdateListener],
206+
},
207+
b: {
208+
doc: value,
209+
extensions: [...modifiedViewExtensions, modifiedUpdateListener],
210+
},
211+
...(!readOnly ? { revertControls: 'a-to-b', renderRevertControl: getRevertControlButton } : {}),
212+
diffConfig: { scanLimit: getScanLimit(lhsValue, value), timeout: 5000 },
213+
parent: codeMirrorMergeParentRef.current,
214+
collapseUnchanged: collapseUnchanged ? {} : undefined,
215+
})
190216

191-
codeMirrorMergeInstance?.destroy()
217+
codeMirrorMergeRef.current = codeMirrorMergeView
218+
setCodeMirrorMergeInstance(codeMirrorMergeView)
192219

193-
const codeMirrorMergeView = new MergeView({
220+
// MINIMAP INITIALIZATION
221+
if (!disableMinimap && codeMirrorMergeView && diffMinimapParentRef.current) {
222+
diffMinimapInstance?.destroy()
223+
diffMinimapRef.current?.destroy()
224+
225+
const diffMinimapMergeView = new MergeView({
194226
a: {
195227
doc: lhsValue,
196-
extensions: [...originalViewExtensions, originalUpdateListener],
228+
extensions: diffMinimapExtensions,
197229
},
198230
b: {
199231
doc: value,
200-
extensions: [...modifiedViewExtensions, modifiedUpdateListener],
232+
extensions: diffMinimapExtensions,
201233
},
202-
...(!readOnly ? { revertControls: 'a-to-b', renderRevertControl: getRevertControlButton } : {}),
203-
diffConfig: { scanLimit, timeout: 5000 },
204-
parent: codeMirrorMergeParentRef.current,
205-
collapseUnchanged: collapseUnchanged ? {} : undefined,
234+
gutter: false,
235+
diffConfig: { scanLimit: getScanLimit(lhsValue, value), timeout: 5000 },
236+
parent: diffMinimapParentRef.current,
206237
})
207-
setCodeMirrorMergeInstance(codeMirrorMergeView)
208-
209-
// MINIMAP INITIALIZATION
210-
if (!disableMinimap && codeMirrorMergeView && diffMinimapParentRef.current) {
211-
diffMinimapInstance?.destroy()
212-
diffMinimapRef.current?.destroy()
213-
214-
const diffMinimapMergeView = new MergeView({
215-
a: {
216-
doc: lhsValue,
217-
extensions: diffMinimapExtensions,
218-
},
219-
b: {
220-
doc: value,
221-
extensions: diffMinimapExtensions,
222-
},
223-
gutter: false,
224-
diffConfig: { scanLimit, timeout: 5000 },
225-
parent: diffMinimapParentRef.current,
226-
})
227-
228-
diffMinimapRef.current = diffMinimapMergeView
229-
setDiffMinimapInstance(diffMinimapMergeView)
230-
}
238+
239+
diffMinimapRef.current = diffMinimapMergeView
240+
setDiffMinimapInstance(diffMinimapMergeView)
241+
}
242+
}
243+
244+
useEffect(() => {
245+
// DIFF VIEW INITIALIZATION
246+
if (!loading && codeMirrorMergeParentRef.current) {
247+
initializeCodeMirrorMergeView()
231248
}
232249

233250
return () => {
234251
setCodeMirrorMergeInstance(null)
235252
setDiffMinimapInstance(null)
253+
codeMirrorMergeRef.current = null
236254
diffMinimapRef.current = null
237255
}
238256
}, [loading, codemirrorMergeKey, diffMode, collapseUnchanged, disableMinimap])
239257

240-
// Sync external changes of `lhsValue` and `value` state to the diff-editor state.
258+
/**
259+
* Synchronizes external changes of `lhsValue` and `value` with the diff-editor state.
260+
*
261+
* When the external state (lhsValue for left-hand side or value for right-hand side) changes,
262+
* we need to update the CodeMirror merge view to reflect these changes. This effect detects
263+
* discrepancies between the current editor content and the external state.
264+
*
265+
* Instead of trying to update the existing editors directly (which can be complex and error-prone),
266+
* we reinitialize the entire merge view when the external state differs from the editor content.
267+
* This ensures a clean, consistent state that properly reflects the external data.
268+
*
269+
*/
241270
useEffect(() => {
242-
if (codeMirrorMergeInstance) {
243-
const originalDoc = codeMirrorMergeInstance.a.state.doc.toString()
244-
if (originalDoc !== lhsValue) {
245-
codeMirrorMergeInstance.a.dispatch({
246-
changes: { from: 0, to: originalDoc.length, insert: lhsValue || '' },
247-
})
248-
}
249-
250-
const modifiedDoc = codeMirrorMergeInstance.b.state.doc.toString()
251-
if (modifiedDoc !== value) {
252-
codeMirrorMergeInstance.b.dispatch({
253-
changes: { from: 0, to: modifiedDoc.length, insert: value || '' },
254-
})
271+
if (codeMirrorMergeRef.current) {
272+
// Get the current content from both editors
273+
const originalDoc = codeMirrorMergeRef.current.a.state.doc.toString()
274+
const modifiedDoc = codeMirrorMergeRef.current.b.state.doc.toString()
275+
276+
// If either editor's content doesn't match the external state,
277+
// reinitialize the entire merge view with the current values
278+
if (originalDoc !== lhsValue || modifiedDoc !== value) {
279+
initializeCodeMirrorMergeView()
255280
}
256281
}
257-
}, [lhsValue, value, codeMirrorMergeInstance])
282+
}, [lhsValue, value])
258283

259284
// SCALING FACTOR UPDATER
260285
useEffect(() => {

src/Shared/Components/DeploymentConfigDiff/DeploymentConfigDiffMain.tsx

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,6 @@ export const DeploymentConfigDiffMain = ({
218218
</div>
219219
)}
220220
<DeploymentHistoryDiffView
221-
codeEditorKey={`${sortingConfig?.sortBy}-${sortingConfig?.sortOrder}-${scopeVariablesConfig?.convertVariables}`}
222221
baseTemplateConfiguration={secondaryList}
223222
currentConfiguration={primaryList}
224223
previousConfigAvailable

0 commit comments

Comments
 (0)