NotificationsYou must be signed in to change notification settings
Fork715
Star17.5k

fix: changed code-mirror to monaco-editor#8047

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.

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

Already on GitHub?Sign in to your account

Jump to bottom

Merged

omkarK06 merged 24 commits intomainfromfix/revert-code-mirror-changes

Sep 12, 2025

Merged

fix: changed code-mirror to monaco-editor#8047

omkarK06 merged 24 commits intomainfromfix/revert-code-mirror-changes

Sep 12, 2025

Conversation

Copy link

Contributor

omkarK06 commentedAug 19, 2025•
edited by github-actionsbot
Loading

PR Type

Enhancement, Bug fix

Description

Replace CodeMirror with Monaco editor
Add Monaco autocomplete and markers
Update build to bundle Monaco
Remove re-render key from logs UI

Diagram Walkthrough

flowchart LR  CM["CodeMirror-based editor"] -- Replaced by --> ME["Monaco editor integration"]  ME -- Autocomplete/Markers --> UX["Improved editor UX"]  Build["Vite config"] -- Add monaco plugin/chunks --> Bundle["Optimized bundles"]  UI["Logs and SearchBar"] -- Minor fixes/cleanup --> Stability["Reduced re-renders"]

File Walkthrough

Relevant files

Enhancement

3 files

CodeQueryEditor.vue `Replace CodeMirror with Monaco, add features`	+369/-980
Index.vue `Cleanups, visualization flow, formatting tweaks`	+105/-78
SearchBar.vue `Editor placeholder, menus, toggle guards`	+148/-82

Dependencies

3 files

CodeQueryEditorDarkTheme.ts `Remove legacy CodeMirror dark theme`	+0/-230
CodeQueryEditorLightTheme.ts `Remove legacy CodeMirror light theme`	+0/-246
package.json `Remove CodeMirror deps; add Monaco and plugin`	+2/-16

Configuration changes

1 files

vite.config.ts `Integrate Monaco plugin and chunking`	+25/-15

github-actionsbot added the ☢️ BugSomething isn't working label

Aug 19, 2025

bjp232004 approved these changes

Aug 19, 2025

View reviewed changes

greptile-appsbot reviewed

Aug 19, 2025

View reviewed changes

Copy link

Contributor

greptile-appsbot left a comment

Choose a reason for hiding this comment

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

Greptile Summary

This pull request implements a complete migration from CodeMirror to Monaco Editor for the web application's code editing functionality. The changes encompass several key areas:

Dependency Management: Thepackage.json removes 16 CodeMirror-related packages (including@codemirror/autocomplete,@codemirror/commands, language packages, themes, and core codemirror) and replaces them withmonaco-editor (v0.52.2) andvite-plugin-monaco-editor (v1.1.0).

Build Configuration: Thevite.config.ts is updated to integrate Monaco Editor with a custom plugin configuration that specifies a distribution path for Monaco's web workers, and updates chunk configurations to handle Monaco Editor bundles instead of CodeMirror.

Component Rewrite: TheCodeQueryEditor.vue component undergoes a complete rewrite, replacing CodeMirror'sEditorView API with Monaco's editor instance. The component maintains the same external interface but changes internal implementation including editor initialization, content handling, and theme management.

Theme Removal: BothCodeQueryEditorLightTheme.ts andCodeQueryEditorDarkTheme.ts files are removed entirely, as Monaco Editor provides built-in theming (vs andvs-dark) that replaces the custom CodeMirror theme configurations.

This migration aligns the codebase with Monaco Editor's more robust feature set, including better TypeScript support, enhanced IntelliSense, and the familiar VS Code editing experience. The change affects code editing functionality throughout the application, particularly in query editors and log analysis interfaces.

Confidence score: 2/5

This PR has significant implementation issues that could cause runtime errors and functionality problems
Score lowered due to missing null checks, removed debouncing logic, potential memory leaks, and incomplete error handling in the main editor component
Pay close attention toweb/src/components/CodeQueryEditor.vue which contains several critical issues including unsafe editor object access and missing cleanup logic

_{5 files reviewed, 2 comments}

_{Edit Code Review Bot Settings |Greptile}

web/src/components/CodeQueryEditor.vueShow resolvedHide resolved

web/src/components/CodeQueryEditor.vue OutdatedShow resolvedHide resolved

github-actionsbot added the Review effort 4/5 label

Aug 19, 2025

Copy link

Contributor

github-actionsbot commentedAug 19, 2025

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review Possible Issue Emitted update events are no longer debounced; every keystroke triggers "update-query" and "update:query", which could cause excessive reactivity and performance regressions compared to the prior debounced behavior. editorObj.onDidChangeModelContent( // debounce((e: any) => { (e: any) => { console.log("e", editorObj.getValue()?.trim(), props.editorId); emit("update-query", editorObj.getValue()?.trim()); emit("update:query", editorObj.getValue()?.trim()); },); Memory/Leak Risk Monaco completion provider and editor instance disposal is partial; provider is stored in a ref but editor instance is never disposed on unmount, and multiple registerAutoCompleteProvider calls could stack if not carefully disposed. onMounted(async () => { provider.value?.dispose(); if (props.language === "vrl") { monaco.languages.register({ id: "vrl" }); // Register a tokens provider for the language monaco.languages.setMonarchTokensProvider( "vrl", vrlLanguageDefinition as any, ); } if (props.language === "sql") { await import( "monaco-editor/esm/vs/basic-languages/sql/sql.contribution.js" ); } if (props.language === "json") { await import( "monaco-editor/esm/vs/language/json/monaco.contribution.js" ); } if (props.language === "html") { await import( "monaco-editor/esm/vs/language/html/monaco.contribution.js" ); } if (props.language === "markdown") { await import( "monaco-editor/esm/vs/basic-languages/markdown/markdown.contribution.js" ); } if (props.language === "python") { await import( "monaco-editor/esm/vs/basic-languages/python/python.contribution.js" ); } if (props.language === "javascript") { await import( "monaco-editor/esm/vs/basic-languages/javascript/javascript.contribution.js" ); } setupEditor();});onActivated(async () => { if (!editorObj) { setupEditor(); editorObj?.layout(); } else { provider.value?.dispose(); registerAutoCompleteProvider(); }});onDeactivated(() => { provider.value?.dispose();});onUnmounted(() => { provider.value?.dispose(); console.log("onUnmounted", props.editorId);}); Build Config Monaco plugin import uses monacoEditorPlugin.default(...); depending on ESM interop, this may fail in some environments—ensure correct usage (default vs named) and that customDistPath is served correctly in production. }),enterpriseResolverPlugin,vueJsx(),monacoEditorPlugin.default({customDistPath:()=>path.resolve(__dirname,"dist/monacoeditorwork"),}),].filter(Boolean),

omkarK06 force-pushed thefix/revert-code-mirror-changes branch from663111d toa3bc0d0Compare

August 19, 2025 11:00

Copy link

Contributor

github-actionsbot commentedAug 19, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Fix off-by-one cursor index Returning index minus one will underflow to -1 at document start and is off-by-one elsewhere. Remove the subtraction and safely handle missing model/position to prevent runtime errors. web/src/components/CodeQueryEditor.vue [456-460] const getCursorIndex = () => {- const currentPosition = editorObj.getPosition();- const cursorIndex =- editorObj?.getModel().getOffsetAt(currentPosition) - 1;- return cursorIndex \|\| null;+ const model = editorObj?.getModel?.();+ const pos = editorObj?.getPosition?.();+ if (!model \|\| !pos) return null;+ return model.getOffsetAt(pos); }; Suggestion importance[1-10]: 8 __ Why: The current code subtracts 1 from the offset, yielding -1 at the start and off-by-one elsewhere. The fix is precise, guards nulls, and prevents incorrect indices and potential errors, improving correctness.	Medium
Possible issue	Stop trimming and debounce changes Avoid trimming on every keystroke; it alters user input and cursor position, and emits excessively. Emit the raw value on change, and perform trimming on blur or before submit. Also debounce the change handler to reduce churn. web/src/components/CodeQueryEditor.vue [199-206] -editorObj.onDidChangeModelContent(- // debounce((e: any) => {- (e: any) => {- console.log("e", editorObj.getValue()?.trim(), props.editorId);- emit("update-query", editorObj.getValue()?.trim());- emit("update:query", editorObj.getValue()?.trim());- },-);+const emitContentChange = debounce(() => {+ const val = editorObj.getValue();+ emit("update-query", val);+ emit("update:query", val);+}, props.debounceTime);+editorObj.onDidChangeModelContent(() => {+ emitContentChange();+});+ Suggestion importance[1-10]: 7 __ Why: Trimming on each keystroke can disrupt cursor position and user intent; debouncing change emission reduces churn and matches the existing trim-on-blur behavior. The improved code aligns with PR context and is a solid usability/performance fix.	Medium
General	Guard model when setting markers `setModelMarkers` expects a model object, not null; calling it without checking the current model can throw. Fetch the current model once, guard null, and clear markers by setting an empty array in the same guarded call. web/src/components/CodeQueryEditor.vue [489-541] -const addErrorDiagnostics = (ranges: any) => {- // const markers = [- // {- // resource: {- // $mid: 1,- // external: "inmemory://model/4",- // path: "/4",- // scheme: "inmemory",- // authority: "model",- // },- // owner: "owner",- // code: "MY_ERROR_CODE",- // severity: monaco.MarkerSeverity.Error,- // message: "Error: Something went wrong",- // startLineNumber: 2,- // startColumn: 1,- // endLineNumber: 7,- // endColumn: 1,- // },- // ...- // ];-- // Set markers to the model- // monaco.editor.setModelMarkers(getModel(), "owner", markers);- const markers = ranges.map((range: any) => ({- severity: monaco.MarkerSeverity.Error, // Mark as error+const addErrorDiagnostics = (ranges: any[]) => {+ const model = editorObj?.getModel?.();+ if (!model) return;+ const markers = (ranges \|\| []).map((range: any) => ({+ severity: monaco.MarkerSeverity.Error, startLineNumber: range.startLine,- startColumn: 1, // Start of the line+ startColumn: 1, endLineNumber: range.endLine,- endColumn: 1, // End of the line- message: range.error, // The error message- code: "", // Optional error code+ endColumn: 1,+ message: range.error \|\| "",+ code: "" }));-- monaco.editor.setModelMarkers(getModel(), "owner", []);- monaco.editor.setModelMarkers(getModel(), "owner", markers);-}+ monaco.editor.setModelMarkers(model, "owner", markers);+}; Suggestion importance[1-10]: 6 __ Why: Calling setModelMarkers with a null model can throw; guarding and computing markers safely is correct and improves robustness. Impact is moderate since it's defensive handling around diagnostics.	Low