upgrade monaco editor (remove monaco-editor/loader and monaco-editor/react) #2743
Conversation
sawka
commented
Jan 3, 2026
sawka
commented
- Updates monaco-editor to new version.
- removes weird AMD stuff, to move to ESM via vite
- remove monaco-editor/loader
- remove monaco-editor/react
- implement the more native monaco wrappers ourselves
WalkthroughAdds a centralized Monaco environment (frontend/app/monaco/monaco-env.ts), two React components for Monaco editors (MonacoCodeEditor and MonacoDiffViewer), and a Monaco schema registry (frontend/app/monaco/schemaendpoints.ts). Replaces older per-component Monaco wiring across multiple view components to use the new modules, removes a legacy schemaendpoints module from frontend/app/view/codeeditor, and updates type imports to use monaco-editor. Updates package.json to remove Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
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}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
frontend/app/view/codeeditor/diffviewer.tsx (1)
44-51: Consider optimizing the UUID initialization.The current pattern
useRef(crypto.randomUUID()).currentworks correctly but evaluatescrypto.randomUUID()on every render even though only the first value is retained. Consider usinguseMemofor cleaner lazy initialization.🔎 Suggested optimization
-const uuidRef = useRef(crypto.randomUUID()).current; +const uuid = useMemo(() => crypto.randomUUID(), []); let editorPath: string; if (fileName) { const separator = fileName.startsWith("/") ? "" : "/"; editorPath = blockId + separator + fileName; } else { - editorPath = uuidRef; + editorPath = uuid; }frontend/app/monaco/monaco-react.tsx (1)
54-60: Clarify theme comment.The comment on line 54 is unclear. Since Monaco themes are global (as confirmed by line 60), the
themeprop in the editor options doesn't actually set the theme—monaco.editor.setTheme(theme)does. Consider clarifying this comment to explain that the theme option is included for consistency but the global theme setter on line 60 is what actually applies it.🔎 Suggested clarification
const editor = monaco.editor.create(el, { ...options, readOnly: readonly, - theme, // note: editor.create uses global theme via setTheme, but keeping your mental model + theme, // Monaco themes are global; actual theme is set below via setTheme model, }); editorRef.current = editor; - // Monaco theme is global; set it here + // Set global Monaco theme monaco.editor.setTheme(theme);frontend/app/view/codeeditor/codeeditor.tsx (2)
74-82: Consider simplifying or clarifying the return value.The
handleEditorOnMountwrapper returnsnullexplicitly. While this works as a cleanup function, it would be more idiomatic to either:
- Return
undefined(or omit the return statement)- Return the actual cleanup function from
onMountAdditionally, verify that returning
nullinstead of the cleanup function doesn't break cleanup logic inMonacoCodeEditor.🔎 Suggested simplification
function handleEditorOnMount( editor: MonacoTypes.editor.IStandaloneCodeEditor, monaco: typeof MonacoModule ): () => void { if (onMount) { unmountRef.current = onMount(editor, monaco); } - return null; + return undefined; }Or more directly:
-function handleEditorOnMount( - editor: MonacoTypes.editor.IStandaloneCodeEditor, - monaco: typeof MonacoModule -): () => void { - if (onMount) { - unmountRef.current = onMount(editor, monaco); - } - return null; -} +function handleEditorOnMount( + editor: MonacoTypes.editor.IStandaloneCodeEditor, + monaco: typeof MonacoModule +) { + if (onMount) { + return onMount(editor, monaco); + } +}Then update usage if storing in unmountRef is needed elsewhere.
84-92: Consider removingreadonlyfrom the dependency array.The
readonlyprop is included in theuseMemodependency array (line 92) but is not used within the computation (lines 84-91). SinceMonacoCodeEditorhandles thereadonlyprop independently through its own effects, including it here causes unnecessary recomputation ofeditorOptswithout any benefit.🔎 Proposed fix
const editorOpts = useMemo(() => { const opts = defaultEditorOptions(); opts.minimap.enabled = minimapEnabled; opts.stickyScroll.enabled = stickyScrollEnabled; opts.wordWrap = wordWrap ? "on" : "off"; opts.fontSize = fontSize; opts.copyWithSyntaxHighlighting = false; return opts; - }, [minimapEnabled, stickyScrollEnabled, wordWrap, fontSize, readonly]); + }, [minimapEnabled, stickyScrollEnabled, wordWrap, fontSize]);
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (14)
electron.vite.config.tsfrontend/app/monaco/monaco-env.tsfrontend/app/monaco/monaco-react.tsxfrontend/app/monaco/schemaendpoints.tsfrontend/app/monaco/yamlworker.jsfrontend/app/view/codeeditor/codeeditor.tsxfrontend/app/view/codeeditor/diffviewer.tsxfrontend/app/view/codeeditor/schemaendpoints.tsfrontend/app/view/preview/preview-edit.tsxfrontend/app/view/preview/preview-model.tsxfrontend/app/view/waveconfig/waveconfig-model.tsfrontend/app/view/waveconfig/waveconfig.tsxfrontend/wave.tspackage.json
💤 Files with no reviewable changes (1)
- frontend/app/view/codeeditor/schemaendpoints.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-01T00:57:23.025Z
Learnt from: sawka
Repo: wavetermdev/waveterm PR: 2504
File: frontend/app/aipanel/aipanel-contextmenu.ts:15-16
Timestamp: 2025-11-01T00:57:23.025Z
Learning: In the waveterm codebase, types defined in custom.d.ts are globally available and do not require explicit imports. Backend types defined in gotypes.d.ts are also globally available.
Applied to files:
frontend/app/view/waveconfig/waveconfig-model.tsfrontend/app/view/waveconfig/waveconfig.tsx
🧬 Code graph analysis (3)
frontend/app/view/codeeditor/diffviewer.tsx (1)
frontend/app/monaco/monaco-react.tsx (1)
MonacoDiffViewer(127-183)
frontend/app/monaco/monaco-react.tsx (1)
frontend/app/monaco/monaco-env.ts (1)
loadMonaco(42-81)
frontend/app/view/codeeditor/codeeditor.tsx (1)
frontend/app/monaco/monaco-react.tsx (1)
MonacoCodeEditor(27-116)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
- GitHub Check: Build for TestDriver.ai
🔇 Additional comments (22)
frontend/app/view/waveconfig/waveconfig-model.ts (1)
15-15: LGTM! Type import path updated correctly.The import path has been correctly updated from the ESM-specific path to the base monaco-editor package, aligning with the PR's goal to simplify Monaco integration.
frontend/app/view/preview/preview-model.tsx (1)
20-20: LGTM! Consistent type import update.This change is consistent with the updated import pattern across the codebase and correctly references the base monaco-editor package for type definitions.
electron.vite.config.ts (1)
154-164: Good additions to watch ignore patterns.Adding
**/*.txtand**/*.logto the ignore list is sensible to prevent unnecessary file watching overhead.package.json (2)
77-147: Monaco dependency updates align with PR objectives.The removal of
@monaco-editor/loaderand@monaco-editor/react, combined with the upgrade tomonaco-editor^0.55.1, correctly implements the PR's goal to move to custom Monaco wrappers and ESM via Vite.
108-109: Monaco-yaml is compatible with the upgraded monaco-editor version.monaco-yaml 5.4.0 specifies "monaco-editor": ">=0.36" as a peer dependency, and monaco-editor 0.55.1 satisfies this requirement. No breaking changes were introduced in the 0.55.1 release that would affect YAML language support.
frontend/wave.ts (1)
5-5: LGTM! Centralized Monaco loading.The import path change to
@/app/monaco/monaco-envcorrectly centralizes Monaco initialization logic, aligning with the PR's architecture improvements. The usage ofloadMonaco()remains unchanged at lines 199 and 270.frontend/app/view/waveconfig/waveconfig.tsx (1)
12-12: LGTM! Type import correctly updated.The Monaco type import now uses the base
monaco-editorpackage instead of the internal ESM path, aligning with the new centralized Monaco integration introduced in this PR.frontend/app/view/preview/preview-edit.tsx (2)
10-11: LGTM! Monaco imports correctly updated.The imports now use the base
monaco-editorpackage, consistent with the new integration approach. The separation of the runtime import (line 10) and type import (line 11) follows best practices.
76-76: LGTM! Parameter rename and type update are correct.The parameter rename from
monacotomonacoApiavoids naming conflicts with the importedmonaconamespace, and the typetypeof monacocorrectly represents the Monaco API object.frontend/app/monaco/schemaendpoints.ts (1)
1-50: LGTM! Schema registry is well-structured.This module provides a centralized schema registry for Monaco JSON validation. The consistent pattern of mapping
wave://URIs toWAVECONFIGPATHfile patterns with corresponding schema objects is clear and maintainable.frontend/app/view/codeeditor/diffviewer.tsx (2)
4-8: LGTM! Imports correctly updated for new Monaco integration.The imports now use
MonacoDiffViewerfrom the new centralized Monaco wrapper and updated Monaco types from the base package.
63-76: LGTM! Diff viewer migration completed correctly.The replacement of the legacy diff editor with
MonacoDiffVieweris implemented correctly, passing through all necessary props including the computedpathfor editor identification.frontend/app/monaco/monaco-env.ts (5)
21-40: LGTM! Worker routing is comprehensive and well-structured.The
MonacoEnvironment.getWorkerimplementation correctly routes all major language labels to their appropriate workers with a sensible fallback to the generic editor worker.
42-46: LGTM! Idempotent guard prevents double configuration.The
monacoConfiguredflag correctly ensures Monaco is only configured once, preventing issues from multiple initialization attempts.
47-66: LGTM! Theme definitions are appropriate.The dark theme uses transparent backgrounds for proper integration with the Wave terminal UI, while the light theme uses a solid background. The transparent
focusBorderremoves unwanted focus rings.
67-70: Verify that empty YAML schemas is intentional.The YAML configuration enables validation but provides an empty schemas array. This enables basic YAML syntax validation without schema-based validation. Confirm this is the intended behavior for YAML files in Wave.
72-80: LGTM! Validation configuration is appropriate.Disabling TypeScript semantic validation reduces noise for configuration files, while enabling JSON validation with the centralized schemas from
MonacoSchemasprovides helpful validation for Wave config files. TheenableSchemaRequest: trueallows Monaco to fetch schemas as needed.frontend/app/monaco/monaco-react.tsx (3)
84-96: LGTM!The text synchronization logic correctly uses
pushEditOperationsto maintain undo/redo history and employs theapplyingFromPropsguard to prevent infinite loops when programmatically updating the model.
99-104: LGTM!The options and theme synchronization correctly updates the editor when props change, maintaining consistency with the global theme approach established during mount.
127-183: LGTM!The
MonacoDiffViewerimplementation follows good practices:
- Proper lifecycle management with mount/unmount cleanup
- Model synchronization using
setValue(appropriate for diff viewers where undo/redo isn't typically needed)- Consistent theme and options handling
frontend/app/view/codeeditor/codeeditor.tsx (2)
4-9: LGTM!The import updates correctly reference the new
MonacoCodeEditorcomponent and Monaco types, aligning with the centralized Monaco integration approach.
97-106: LGTM!The integration with
MonacoCodeEditoris correct, with all necessary props properly passed and the computededitorPathappropriately utilized.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
frontend/app/monaco/monaco-env.ts (2)
67-70: YAML schemas array is empty.The
schemasarray is empty, so YAML validation is enabled but no schema constraints are applied. If this is a placeholder for future implementation, consider adding a TODO comment.
71-71: Consider dynamic theme selection based on app theme.The default theme is hardcoded to
wave-theme-dark. If the app supports light mode, you may want to detect and apply the appropriate theme at initialization or expose a function to switch themes.frontend/app/view/codeeditor/codeeditor.tsx (1)
83-91: Remove unusedreadonlyfrom useMemo dependencies.The
readonlyvariable is listed in the dependency array but is not used within the memo calculation. Either remove it from dependencies or add the intended usage.🔎 Proposed fix
const editorOpts = useMemo(() => { const opts = defaultEditorOptions(); opts.minimap.enabled = minimapEnabled; opts.stickyScroll.enabled = stickyScrollEnabled; opts.wordWrap = wordWrap ? "on" : "off"; opts.fontSize = fontSize; opts.copyWithSyntaxHighlighting = false; return opts; - }, [minimapEnabled, stickyScrollEnabled, wordWrap, fontSize, readonly]); + }, [minimapEnabled, stickyScrollEnabled, wordWrap, fontSize]);
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
frontend/app/monaco/monaco-env.tsfrontend/app/monaco/monaco-react.tsxfrontend/app/view/codeeditor/codeeditor.tsxfrontend/app/view/codeeditor/diffviewer.tsxpackage.json
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/app/view/codeeditor/diffviewer.tsx
- package.json
🧰 Additional context used
🧬 Code graph analysis (2)
frontend/app/monaco/monaco-react.tsx (1)
frontend/app/monaco/monaco-env.ts (1)
loadMonaco(42-82)
frontend/app/view/codeeditor/codeeditor.tsx (1)
frontend/app/monaco/monaco-react.tsx (1)
MonacoCodeEditor(24-107)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
- GitHub Check: Build for TestDriver.ai
🔇 Additional comments (8)
frontend/app/monaco/monaco-env.ts (2)
1-17: LGTM! Clean worker and contribution imports.The module correctly imports Monaco contributions for language support and configures workers via Vite's
?workersuffix for proper ESM bundling.
21-40: LGTM! Comprehensive worker routing.The worker selection correctly handles all Monaco-supported languages with appropriate workers and provides a sensible fallback.
frontend/app/monaco/monaco-react.tsx (4)
9-12: LGTM! Clean model creation helper.The URI encoding ensures safe paths for model identification.
63-73: LGTM! Correct cleanup and lifecycle management.The disposal order is correct: subscription → onUnmount callback → editor → model. The empty dependency array with eslint-disable is acceptable for mount/unmount-only behavior.
75-88: Good use ofpushEditOperationsfor text sync.Using
pushEditOperationsinstead ofsetValuepreserves the undo stack, which is the correct approach for prop-driven updates.
117-171: LGTM! Well-structured diff viewer component.The component correctly manages the diff editor lifecycle, model creation/disposal, and prop synchronization. Using
setValuefor diff models is appropriate since undo history isn't typically needed in diff viewers.frontend/app/view/codeeditor/codeeditor.tsx (2)
4-9: LGTM! Updated imports for new Monaco integration.Clean transition to the new
MonacoCodeEditorcomponent and proper type imports.
96-104: LGTM! Clean integration with MonacoCodeEditor.The component correctly wires up all required props to the new
MonacoCodeEditorcomponent.
| if (!el) return; | ||
|
|
||
| const model = createModel(text, path, language); | ||
| console.log("[monaco] CREATE MODEL", path, model); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove debug console.log statement.
This debug logging should be removed before merging.
🔎 Proposed fix
const model = createModel(text, path, language);
- console.log("[monaco] CREATE MODEL", path, model);
const editor = monaco.editor.create(el, {📝 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.
| console.log("[monaco] CREATE MODEL", path, model); | |
| const model = createModel(text, path, language); | |
| const editor = monaco.editor.create(el, { |
🤖 Prompt for AI Agents
In frontend/app/monaco/monaco-react.tsx around line 45, remove the debug
console.log("[monaco] CREATE MODEL", path, model) statement; simply delete that
line (or replace with a non-debug logger call if persistent logging is required)
so no debug output is left in production code.
| if (onUnmountRef.current) onUnmountRef.current(); | ||
| editor.dispose(); | ||
| model.dispose(); | ||
| console.log("[monaco] dispose model"); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove debug console.log statement.
This debug logging should be removed before merging.
🔎 Proposed fix
editor.dispose();
model.dispose();
- console.log("[monaco] dispose model");
editorRef.current = null;📝 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.
| console.log("[monaco] dispose model"); | |
| editor.dispose(); | |
| model.dispose(); | |
| editorRef.current = null; |
🤖 Prompt for AI Agents
In frontend/app/monaco/monaco-react.tsx around line 68, remove the debug
console.log("[monaco] dispose model"); statement so no stray debug logging
remains; if structured logging is required instead, replace it with the
project’s logger call, otherwise simply delete the line and ensure no other
console.* debug calls remain in this file.
| function handleEditorOnMount( | ||
| editor: MonacoTypes.editor.IStandaloneCodeEditor, | ||
| monaco: typeof MonacoModule | ||
| ): () => void { | ||
| if (onMount) { | ||
| unmountRef.current = onMount(editor, monaco); | ||
| } | ||
| return null; | ||
| } |
There was a problem hiding this comment.
Type mismatch: return null doesn't match () => void return type.
The handleEditorOnMount function declares return type () => void but returns null on line 80. This should return a no-op function instead.
🔎 Proposed fix
function handleEditorOnMount(
editor: MonacoTypes.editor.IStandaloneCodeEditor,
monaco: typeof MonacoModule
): () => void {
if (onMount) {
unmountRef.current = onMount(editor, monaco);
}
- return null;
+ return () => {};
}📝 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.
| function handleEditorOnMount( | |
| editor: MonacoTypes.editor.IStandaloneCodeEditor, | |
| monaco: typeof MonacoModule | |
| ): () => void { | |
| if (onMount) { | |
| unmountRef.current = onMount(editor, monaco); | |
| } | |
| return null; | |
| } | |
| function handleEditorOnMount( | |
| editor: MonacoTypes.editor.IStandaloneCodeEditor, | |
| monaco: typeof MonacoModule | |
| ): () => void { | |
| if (onMount) { | |
| unmountRef.current = onMount(editor, monaco); | |
| } | |
| return () => {}; | |
| } |
🤖 Prompt for AI Agents
In frontend/app/view/codeeditor/codeeditor.tsx around lines 73 to 81, the
function handleEditorOnMount is declared to return () => void but currently
returns null; change the function to return a no-op function instead (e.g.,
return () => {}), and ensure unmountRef.current is assigned only when onMount
returns a function or undefined (i.e., handle possible void return from onMount)
so types align without using null.
