Skip to content

poc: codeeditor shrink state persist #7797

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
shubh-bruno wants to merge 1 commit into
base: main
Choose a base branch
from
Open

poc: codeeditor shrink state persist #7797

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
141 changes: 136 additions & 5 deletions packages/bruno-app/src/components/CodeEditor/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,42 @@ window.JSHINT = JSHINT;

const TAB_SIZE = 2;

/*
* Per-tab Doc cache — why this exists:
*
* CodeMirror 5 splits an editor into two objects: the Editor (UI shell) and the
* Doc (content + cursor + selection + undo history + TextMarkers like folds).
* Previously, every tab switch called `editor.setValue(newContent)` which mutates
* the current Doc and destroys all of that state. Users lost folds, cursor,
* selection, and undo history on every switch.
*
* Instead, we keep one Doc per (item, mode, readOnly) combination in this cache.
* On tab switch we call `editor.swapDoc(cachedDoc)` — CM5's native multi-document
* API — which atomically swaps the entire Doc and preserves every piece of
* per-tab state for free.
*
* Constraint: a Doc can be attached to only one editor at a time (CM5 enforces
* this via `doc.cm`). See componentWillUnmount for how we release the Doc.
*/
const docCache = new Map();
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Apr 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Add an eviction path for cached docs.

docCache keeps every CodeMirror.Doc indefinitely, and unmount intentionally leaves content, markers, and undo history cached. Long sessions that open many requests or large responses can retain a lot of memory. Please add an explicit invalidation path tied to tab/collection close, or a bounded LRU/TTL policy.

Also applies to: 395-406

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@packages/bruno-app/src/components/CodeEditor/index.js` at line 44, docCache
currently holds CodeMirror.Doc objects forever (const docCache = new Map()),
leaking content, markers, and undo history; add an explicit
eviction/invalidation strategy: either implement a bounded LRU or TTL for
entries in docCache, and expose a removal call that your tab/collection
lifecycle can call when a tab is closed (e.g., add a removeDoc(key) or
clearDocsForCollection(collectionId) function and call it from the
tab-close/unmount handler). Ensure eviction cleans up the underlying
CodeMirror.Doc (call its .toTextArea() / .clearHistory() or appropriate dispose
API) before deleting the Map entry so markers/undo stacks are released.
Reference docCache and the tab/collection unmount/close handlers when wiring
this up.


const getOrCreateDoc = (key, content, mode) => {
let doc = docCache.get(key);
if (doc) {
// The cached Doc may have stale content if props.value changed while this
// tab was inactive (e.g. a new response arrived). Sync the content so the
// user sees the latest value. This does reset fold state on this Doc, but
// that's correct — fold positions for old content are meaningless.
if (doc.getValue() !== content) {
doc.setValue(content);
}
Comment on lines +46 to +55
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Apr 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

find . -type f -name "index.js" | grep -i "codeeditor"

Repository: usebruno/bruno

Length of output: 286

🏁 Script executed:

cat -n packages/bruno-app/src/components/CodeEditor/index.js | head -80

Repository: usebruno/bruno

Length of output: 3651

🏁 Script executed:

cat -n packages/bruno-app/src/components/CodeEditor/index.js | sed -n '40,60p'

Repository: usebruno/bruno

Length of output: 974

🏁 Script executed:

cat -n packages/bruno-app/src/components/CodeEditor/index.js | sed -n '230,260p'

Repository: usebruno/bruno

Length of output: 1650

🏁 Script executed:

cat -n packages/bruno-app/src/components/CodeEditor/index.js | sed -n '310,330p'

Repository: usebruno/bruno

Length of output: 1237

🏁 Script executed:

wc -l packages/bruno-app/src/components/CodeEditor/index.js

Repository: usebruno/bruno

Length of output: 115

Move the doc.cm attachment check into getOrCreateDoc before mutating cached content.

The function calls doc.setValue(content) before checking whether that Doc is still attached to another editor. If the cached Doc is in use elsewhere, this mutation fires change handlers on the wrong editor and corrupts its state. The defensive fallback check only happens after getOrCreateDoc returns — too late to prevent the damage.

Pass the current editor to getOrCreateDoc and check doc.cm before calling setValue. If the Doc is attached elsewhere, create a fresh Doc instead of mutating the cached one.

Proposed fix 
-const getOrCreateDoc = (key, content, mode) => {
+const getOrCreateDoc = (key, content, mode, editor) => {
   let doc = docCache.get(key);
   if (doc) {
+    if (doc.cm && doc.cm !== editor) {
+      doc = new CodeMirror.Doc(content, mode);
+      docCache.set(key, doc);
+      return doc;
+    }
+
     // The cached Doc may have stale content if props.value changed while this
     // tab was inactive (e.g. a new response arrived). Sync the content so the
     // user sees the latest value. This does reset fold state on this Doc, but
     // that's correct — fold positions for old content are meaningless.
     if (doc.getValue() !== content) {
@@
       let doc = getOrCreateDoc(
         docKey,
         this.props.value || '',
-        this.props.mode || 'application/ld+json'
+        this.props.mode || 'application/ld+json',
+        editor
       );
-      // Defensive fallback: a CM5 Doc can only be attached to one editor at a
-      // time. If the cached Doc is still attached to a previous (dead) editor —
-      // e.g. React StrictMode double-mounting, or an unmount that skipped our
-      // release logic — swapDoc would throw "document already in use". Replace
-      // the cache entry with a fresh Doc in that case.
-      if (doc.cm && doc.cm !== editor) {
-        doc = new CodeMirror.Doc(this.props.value || '', this.props.mode || 'application/ld+json');
-        docCache.set(docKey, doc);
-      }
@@
         let doc = getOrCreateDoc(
           newDocKey,
           this.props.value || '',
-          this.props.mode || 'application/ld+json'
+          this.props.mode || 'application/ld+json',
+          this.editor
         );
-        // Same defensive fallback as componentDidMount — see there for why.
-        if (doc.cm && doc.cm !== this.editor) {
-          doc = new CodeMirror.Doc(this.props.value || '', this.props.mode || 'application/ld+json');
-          docCache.set(newDocKey, doc);
-        }

Also applies to: 246-249, 317-320

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@packages/bruno-app/src/components/CodeEditor/index.js` around lines 46 - 55,
getOrCreateDoc mutates a cached Doc (doc.setValue(content)) before checking
whether that Doc is attached to another editor, which can trigger change
handlers on the wrong editor; change getOrCreateDoc signature to accept the
current editor instance, and inside getOrCreateDoc check doc.cm (or doc.cm !==
undefined) before calling doc.setValue — if doc.cm is set (attached elsewhere)
create a fresh Doc instead of mutating the cached one and update docCache
accordingly; update all call sites (including the other occurrences mentioned)
to pass the current editor so the defensive attachment check happens before any
mutation.

return doc;
}
doc = new CodeMirror.Doc(content, mode);
docCache.set(key, doc);
return doc;
};

export default class CodeEditor extends React.Component {
constructor(props) {
super(props);
Expand All @@ -48,6 +84,24 @@ export default class CodeEditor extends React.Component {
};
}

// Identifies which Doc in docCache belongs to this CodeEditor instance.
//
// Callers can pass an explicit `docKey` prop when the auto-derived key would
// collide — e.g. Pre-Request vs Post-Response script editors share the same
// item/mode/readOnly and need an extra disambiguator.
//
// Auto-derived parts:
// id — distinguishes different tabs (requests or collections)
// mode — distinguishes editors within the same tab (e.g. JSON body vs JS script)
// readOnly — distinguishes response viewer (ro) from body editor (rw) when modes match
_getDocKey() {
if (this.props.docKey) return this.props.docKey;
const id = this.props.item?.uid || this.props.collection?.uid || 'default';
const mode = this.props.mode || 'default';
const readOnly = this.props.readOnly ? 'ro' : 'rw';
return `${id}:${mode}:${readOnly}`;
}

componentDidMount() {
const variables = getAllVariables(this.props.collection, this.props.item);

Expand Down Expand Up @@ -175,6 +229,30 @@ export default class CodeEditor extends React.Component {
});

if (editor) {
// CodeMirror(node, { value }) created a throwaway initial Doc from props.value.
// Replace it with this tab's cached Doc so any previously preserved folds,
// cursor, selection, and undo history are restored.
const docKey = this._getDocKey();
let doc = getOrCreateDoc(
docKey,
this.props.value || '',
this.props.mode || 'application/ld+json'
);
// Defensive fallback: a CM5 Doc can only be attached to one editor at a
// time. If the cached Doc is still attached to a previous (dead) editor —
// e.g. React StrictMode double-mounting, or an unmount that skipped our
// release logic — swapDoc would throw "document already in use". Replace
// the cache entry with a fresh Doc in that case.
if (doc.cm && doc.cm !== editor) {
doc = new CodeMirror.Doc(this.props.value || '', this.props.mode || 'application/ld+json');
docCache.set(docKey, doc);
}
if (doc !== editor.getDoc()) {
editor.swapDoc(doc);
}
this._currentDocKey = docKey;
this.cachedValue = editor.getValue();

editor.setOption('lint', this.props.mode && editor.getValue().trim().length > 0 ? this.lintOptions : false);
editor.on('change', this._onEdit);
editor.scrollTo(null, this.props.initialScroll);
Expand Down Expand Up @@ -218,11 +296,48 @@ export default class CodeEditor extends React.Component {
this.editor.options.jump.schema = this.props.schema;
CodeMirror.signal(this.editor, 'change', this.editor);
}
if (this.props.value !== prevProps.value && this.props.value !== this.cachedValue && this.editor) {
const cursor = this.editor.getCursor();
this.cachedValue = String(this?.props?.value ?? '');
this.editor.setValue(String(this.props.value) || '');
this.editor.setCursor(cursor);
if (this.editor) {
// Two distinct update paths:
// 1. Doc key changed → tab switch → swapDoc (preserves all per-tab state)
// 2. Same doc, value changed → external content update → setValue (state resets)
const newDocKey = this._getDocKey();
const docKeyChanged = newDocKey !== this._currentDocKey;

if (docKeyChanged) {
// Path 1 — tab switch. Look up (or create) the incoming tab's Doc and
// swap it in. CM5 handles the rest: the outgoing Doc keeps its folds,
// cursor, selection, undo history, and scroll in docCache for later;
// the incoming Doc restores whatever state it had when last visited.
let doc = getOrCreateDoc(
newDocKey,
this.props.value || '',
this.props.mode || 'application/ld+json'
);
// Same defensive fallback as componentDidMount — see there for why.
if (doc.cm && doc.cm !== this.editor) {
doc = new CodeMirror.Doc(this.props.value || '', this.props.mode || 'application/ld+json');
docCache.set(newDocKey, doc);
}
this.editor.swapDoc(doc);
this._currentDocKey = newDocKey;
this.cachedValue = this.editor.getValue();
// swapDoc resets the editor's mode to whatever mode the incoming Doc
// was created with (raw, not the 'brunovariables' overlay). Re-apply
// the overlay and re-evaluate lint config for the new content.
this.addOverlay();
this.editor.setOption(
'lint',
this.props.mode && this.editor.getValue().trim().length > 0 ? this.lintOptions : false
);
} else if (this.props.value !== prevProps.value && this.props.value !== this.cachedValue) {
// Path 2 — same tab, new external value (e.g. a fresh response arrived
// while this tab was active). Update the current Doc via setValue. Fold
// state resets because line positions no longer correspond to anything.
const cursor = this.editor.getCursor();
this.cachedValue = String(this?.props?.value ?? '');
this.editor.setValue(String(this.props.value) || '');
Comment on lines +337 to +338
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Apr 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Avoid rendering nullish values as text.

Line 338 uses String(this.props.value) || '', so undefined becomes 'undefined' and null becomes 'null'. Coalesce before stringifying.

Proposed fix 
-        this.cachedValue = String(this?.props?.value ?? '');
-        this.editor.setValue(String(this.props.value) || '');
+        const nextValue = String(this.props.value ?? '');
+        this.cachedValue = nextValue;
+        this.editor.setValue(nextValue);
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@packages/bruno-app/src/components/CodeEditor/index.js` around lines 337 -
338, The code is converting null/undefined into the literal strings
"null"/"undefined" when setting this.cachedValue and calling
this.editor.setValue; change both uses to coalesce this.props.value to an empty
string before stringifying (i.e., use this.props.value ?? '' or
this?.props?.value ?? '' as the input to String) so cachedValue and
editor.setValue never receive the text "null" or "undefined" — update the
references to cachedValue and the call to this.editor.setValue accordingly.

this.editor.setCursor(cursor);
}
}

if (this.editor) {
Expand Down Expand Up @@ -277,6 +392,22 @@ export default class CodeEditor extends React.Component {
// Clean up lint error tooltip
this.cleanupLintErrorTooltip?.();

// Release the cached Doc before the editor goes away.
//
// A CM5 Doc can only be attached to one editor at a time (enforced via
// doc.cm). If we destroy this editor without swapping the cached Doc out,
// CM5 still considers the Doc attached — and the next CodeEditor to mount
// for this tab will throw "This document is already in use." on swapDoc.
//
// Swapping in a fresh throwaway Doc clears doc.cm on our cached Doc while
// leaving its content, folds, cursor, and undo history intact inside
// docCache, ready to be attached to the next editor instance.
try {
this.editor.swapDoc(new CodeMirror.Doc('', this.props.mode || 'application/ld+json'));
} catch (e) {
// noop — swapDoc can fail if the editor is already in a bad state
}

const wrapper = this.editor.getWrapperElement();
wrapper?.parentNode?.removeChild(wrapper);

Expand Down
2 changes: 2 additions & 0 deletions packages/bruno-app/src/components/CollectionSettings/Script/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ const Script = ({ collection }) => {
<CodeEditor
ref={preRequestEditorRef}
collection={collection}
docKey={`${collection.uid}:collection-script:pre-request`}
value={requestScript || ''}
theme={displayedTheme}
onEdit={onRequestScriptEdit}
Expand All @@ -118,6 +119,7 @@ const Script = ({ collection }) => {
<CodeEditor
ref={postResponseEditorRef}
collection={collection}
docKey={`${collection.uid}:collection-script:post-response`}
value={responseScript || ''}
theme={displayedTheme}
onEdit={onResponseScriptEdit}
Expand Down
2 changes: 2 additions & 0 deletions packages/bruno-app/src/components/FolderSettings/Script/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ const Script = ({ collection, folder }) => {
<CodeEditor
ref={preRequestEditorRef}
collection={collection}
docKey={`${folder.uid}:folder-script:pre-request`}
value={requestScript || ''}
theme={displayedTheme}
onEdit={onRequestScriptEdit}
Expand All @@ -121,6 +122,7 @@ const Script = ({ collection, folder }) => {
<CodeEditor
ref={postResponseEditorRef}
collection={collection}
docKey={`${folder.uid}:folder-script:post-response`}
value={responseScript || ''}
theme={displayedTheme}
onEdit={onResponseScriptEdit}
Expand Down
4 changes: 3 additions & 1 deletion packages/bruno-app/src/components/RequestPane/Script/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { useState, useEffect, useRef } from 'react';
import React, { useEffect, useRef } from 'react';
import get from 'lodash/get';
import find from 'lodash/find';
import { useDispatch, useSelector } from 'react-redux';
Expand Down Expand Up @@ -99,6 +99,7 @@ const Script = ({ item, collection }) => {
<CodeEditor
ref={preRequestEditorRef}
collection={collection}
docKey={`${item.uid}:script:pre-request`}
value={requestScript || ''}
theme={displayedTheme}
font={get(preferences, 'font.codeFont', 'default')}
Expand All @@ -115,6 +116,7 @@ const Script = ({ item, collection }) => {
<CodeEditor
ref={postResponseEditorRef}
collection={collection}
docKey={`${item.uid}:script:post-response`}
value={responseScript || ''}
theme={displayedTheme}
font={get(preferences, 'font.codeFont', 'default')}
Expand Down
Loading