Refactor CodeEditor to CodeMirror#35
Refactor CodeEditor to CodeMirror#35warunalakshitha merged 5 commits intoballerina-platform:mainfrom
Conversation
📝 WalkthroughWalkthroughReplaces the Shiki+textarea overlay with a CodeMirror-based editor via a new Shiki→CodeMirror bridge, adds CodeMirror and Vim support, introduces persistent editorMode (standard|vim) in the store, updates hotkey wiring and language typings, and reworks editor mounting and reconfiguration. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant EditorPane
participant CodeEditor
participant ShikiEditor as ShikiEditor (CodeMirror)
participant EditorStore
User->>EditorPane: open/select file
EditorPane->>EditorStore: read editorMode
EditorPane->>CodeEditor: mount(content, language, hotkeys)
CodeEditor->>ShikiEditor: initialize(lang, theme, extensions)
ShikiEditor->>ShikiEditor: create EditorView + highlight plugin
ShikiEditor-->>CodeEditor: ready
User->>ShikiEditor: type/edit
ShikiEditor->>CodeEditor: onUpdate
CodeEditor->>EditorPane: onChange(content)
EditorPane->>EditorStore: update activeFile
User->>EditorPane: toggle Vim (Mod-Alt-v)
EditorPane->>EditorStore: toggleEditorMode()
EditorStore-->>CodeEditor: editorMode changed
CodeEditor->>ShikiEditor: reconfigure Vim compartment
ShikiEditor->>ShikiEditor: enable/disable Vim keymap
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/components/code-editor.tsx`:
- Around line 126-130: The component CodeEditor only applies the incoming value
prop on initial mount (the initial doc created around the doc seeding at the
existing doc creation code) and never pushes subsequent value changes into the
mounted ShikiEditor; fix this by obtaining the ShikiEditor instance (via the
ref/callback you already pass to ShikiEditor) and, inside a useEffect that
watches the value prop, call the exposed setValue(...) on the ShikiEditor
instance when value differs from editor.getValue() so updates are applied
without remounting (use the ShikiEditor getValue()/setValue() methods to compare
and update).
- Around line 58-70: baseExtensions currently unconditionally includes
ballerinaMode causing non-Ballerina files to use Ballerina syntax rules; update
baseExtensions to only push/include ballerinaMode when the component's language
prop equals 'ballerina' (check the language variable in the component scope) and
do the same for the other extension list that also unconditionally includes
ballerinaMode (the other extensions builder used for the read-only/editor
variant). Locate baseExtensions, ballerinaMode, and the other extensions builder
and wrap inclusion of ballerinaMode in a conditional (e.g., if (language ===
'ballerina') { extensions.push(ballerinaMode) }) so only Ballerina files get
that mode.
In `@apps/web/src/components/shiki-editor.ts`:
- Around line 667-705: The highlight() implementation recreates cmClasses each
call which causes new class names via newStyleModuleName() and repeated
mountStyles() calls; persist a mapping (e.g., this.cmClasses) on the component
instance and a set/map of mounted style modules (e.g., this.mountedStyles) so
that highlight() reuses existing class names instead of generating new ones and
only calls mountStyles() for new/unmounted styles; update references to
cmClasses in highlight() to use this.cmClasses and gate mountStyles(this.view,
...) behind a check against this.mountedStyles to avoid remounting/duplicating
rules, and ensure any necessary cleanup when a style is truly removed.
In `@apps/web/src/stores/editor-store.ts`:
- Around line 33-37: The initial state captures readStoredEditorMode() at module
load which causes reset() to restore the stale snapshot; change reset() (and any
code that references initial.editorMode) to call readStoredEditorMode() instead
of using initial.editorMode, or replace the frozen initial constant with a
getInitialState() factory that calls readStoredEditorMode() each time; update
references in reset(), setEditorMode(), and toggleEditorMode() to use the fresh
value so the store reflects the latest localStorage preference.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 623f0823-68f1-4af4-bb39-1b8d971183ae
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
apps/web/package.jsonapps/web/src/components/code-editor.tsxapps/web/src/components/editor.tsxapps/web/src/components/shiki-editor.tsapps/web/src/stores/editor-store.ts
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/web/src/components/code-editor.tsx (1)
150-189:⚠️ Potential issue | 🟠 Major
valueandlanguageare still mount-only inputs.Line 150 says the editor is recreated on language changes and
valueis synced separately, but the effect still has[]and the only post-mount reconfigure path is for Vim. If this component is reused for another file, it can keep both the old buffer and the old syntax mode/highlighter.♻️ Minimal fix
- }, []); + }, [language]); + + React.useEffect(() => { + const editor = editorRef.current; + if (!editor) return; + + const nextValue = value ?? ""; + if (editor.getValue() !== nextValue) { + editor.setValue(nextValue); + } + }, [value]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/code-editor.tsx` around lines 150 - 189, The editor is only mounted once so changing language or value can leave the old buffer and highlighter; update the effects so language changes reconfigure or recreate the editor and value changes sync into the editor: include language in the editor mounting effect deps (or add a small effect that calls editorRef.current?.reconfigure(languageCompartment.current, language === "ballerina" ? ballerinaMode : [])) and add a separate React.useEffect that watches value and, when editorRef.current exists and its current content differs from value, calls the editor instance to update the buffer (e.g., editorRef.current?.setValue(value) or the editor's equivalent); keep the existing vim reconfigure effect for vimCompartment and vimEnabled.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/components/code-editor.tsx`:
- Around line 146-149: Vim.defineEx is being called on every render — move the
registration into a useEffect so the command is registered once and uses a
stable callback via a ref; create or reuse saveFileRef similar to
onChangeRef/hotkeysRef that holds the latest saveFile from useFileTreeActions,
then inside useEffect call Vim.defineEx("write","w", () =>
saveFileRef.current()) and run this effect once (empty deps) or with only the
ref, and if Vim.defineEx provides an unregister/cleanup, return that cleanup
from the effect.
---
Duplicate comments:
In `@apps/web/src/components/code-editor.tsx`:
- Around line 150-189: The editor is only mounted once so changing language or
value can leave the old buffer and highlighter; update the effects so language
changes reconfigure or recreate the editor and value changes sync into the
editor: include language in the editor mounting effect deps (or add a small
effect that calls editorRef.current?.reconfigure(languageCompartment.current,
language === "ballerina" ? ballerinaMode : [])) and add a separate
React.useEffect that watches value and, when editorRef.current exists and its
current content differs from value, calls the editor instance to update the
buffer (e.g., editorRef.current?.setValue(value) or the editor's equivalent);
keep the existing vim reconfigure effect for vimCompartment and vimEnabled.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f2bd2a80-2e69-462c-a3cb-0b33cd896e4b
📒 Files selected for processing (2)
apps/web/src/components/code-editor.tsxapps/web/src/stores/editor-store.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/src/stores/editor-store.ts
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/web/src/components/code-editor.tsx (1)
152-184:⚠️ Potential issue | 🟠 Major
valueprop updates are not synced after mount.Line 158 seeds
doconce, but latervaluechanges never reach the mounted editor instance. This breaks controlled usage (e.g., external file/content switch).♻️ Proposed fix
React.useEffect(() => { const editor = editorRef.current; if (!editor) return; editor.reconfigure(vimCompartment.current, vimEnabled ? vim() : []); }, [vimEnabled]); + + React.useEffect(() => { + const editor = editorRef.current; + if (!editor) return; + const nextValue = value ?? ""; + if (editor.getValue() !== nextValue) { + editor.setValue(nextValue); + } + }, [value]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/code-editor.tsx` around lines 152 - 184, The effect that instantiates ShikiEditor seeds its initial doc from the value prop but never syncs subsequent value changes; add a new React.useEffect that watches value and, when editorRef.current exists and value differs from the editor's current content (compare against editorRef.current.state.doc.toString() or editorRef.current.getValue()), update the editor's document (call the editor instance method such as setDoc/update to replace the content) so external changes propagate; ensure you guard against echoing local edits by only applying the update when the texts differ.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/components/code-editor.tsx`:
- Around line 151-184: The editor is only created once (useEffect with []), so
when props change the language and vim mode never update; add a separate
React.useEffect that depends on language and vimEnabled which updates the
editor's compartments: call
languageCompartment.current.reconfigure(languageCompartment.current.of(language
=== "ballerina" ? ballerinaMode : [])) and
vimCompartment.current.reconfigure(vimCompartment.current.of(vimEnabled ? vim()
: [])) (or call the equivalent reconfigure API on the ShikiEditor/CodeMirror
instance held in editorRef.current if your wrapper exposes it) so the mode and
vim extension update at runtime without recreating the editor.
---
Duplicate comments:
In `@apps/web/src/components/code-editor.tsx`:
- Around line 152-184: The effect that instantiates ShikiEditor seeds its
initial doc from the value prop but never syncs subsequent value changes; add a
new React.useEffect that watches value and, when editorRef.current exists and
value differs from the editor's current content (compare against
editorRef.current.state.doc.toString() or editorRef.current.getValue()), update
the editor's document (call the editor instance method such as setDoc/update to
replace the content) so external changes propagate; ensure you guard against
echoing local edits by only applying the update when the texts differ.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7ae779ff-edff-4970-b6b0-7e37f00a4a7b
📒 Files selected for processing (1)
apps/web/src/components/code-editor.tsx
0e16f81 to
62bd9d7
Compare
|
Did we remove all unused code and dependencies of earlier code editor? |
62bd9d7 to
c163e67
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/web/src/components/shiki-editor.ts (1)
633-706:⚠️ Potential issue | 🟠 MajorCSS class names regenerated on every highlight pass.
cmClassesis created fresh at line 633, new class names generated vianewStyleModuleName()at line 667, and styles mounted each time at lines 700-705. This accumulates CSS rules without reuse or cleanup.Consider caching style-to-class mappings on the instance to avoid regenerating identical styles.
♻️ Proposed fix: Cache token style classes
class ShikiHighlighter extends Base { view!: EditorView; private grammarStateCache = new Map<number, unknown>(); + private tokenStyleClasses = new Map<string, string>(); private internal: Highlighter; private isCoreUpdating = false; @@ - const cmClasses: Record<string, string> = {}; + const cmClasses: Record<string, string> = {}; @@ - const cls = cmClasses[style] || newStyleModuleName(); - cmClasses[style] = cls; + let cls = this.tokenStyleClasses.get(style); + if (!cls) { + cls = newStyleModuleName(); + this.tokenStyleClasses.set(style, cls); + if (this.isCmStyle) { + mountStyles(this.view, { + [`& .cm-line .${cls}`]: toStyleObject(style, false) || {}, + }); + } + } + cmClasses[style] = cls; @@ - if (this.isCmStyle) { - for (const [k, v] of Object.entries(cmClasses)) { - mountStyles(this.view, { - [`& .cm-line .${v}`]: toStyleObject(k, false) || {}, - }); - } - }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/shiki-editor.ts` around lines 633 - 706, The code recreates cmClasses each highlight pass causing new class names via newStyleModuleName() and repeated mountStyles calls; change cmClasses to an instance-level cache (e.g., this.cmStyleClassMap) used by the highlight loop (replace local cmClasses usages in the loop with this.cmStyleClassMap), only call newStyleModuleName() when a style string is missing from the instance cache, and update the mountStyles loop to iterate over this.cmStyleClassMap so styles are reused across passes (optionally add a cleanup method to unmount/remove entries if the editor is destroyed).
🧹 Nitpick comments (4)
apps/web/src/components/editor.tsx (1)
168-180: Redundant conditional checks inside guarded block.Lines 171 and 178 have redundant null checks for
activeFilesince this block is already guarded byactiveFile &&at line 168.♻️ Simplify redundant conditionals
{activeFile && ( <CodeEditor key={activeFile.path} - value={activeFile?.content} + value={activeFile.content} onChange={handleChange} hotkeys={{ "Mod-Enter": onRun, "Mod-Alt-v": toggleEditorMode, "Mod-r": () => window.location.reload(), }} - language={activeFile ? getLanguage(activeFile.path) : "text"} + language={getLanguage(activeFile.path)} /> )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/editor.tsx` around lines 168 - 180, Remove the redundant null-safe checks inside the block already guarded by "activeFile &&": inside the CodeEditor props use activeFile.content for value and call getLanguage(activeFile.path) for language (instead of activeFile? or the ternary). Update the CodeEditor instantiation (the component named CodeEditor and its props value, language) to reference activeFile directly; keep existing handlers (handleChange, onRun, toggleEditorMode) and hotkeys unchanged.apps/web/src/components/code-editor.tsx (2)
151-152: Misleading biome-ignore comment.The comment says "value is synced separately" but there's no
setValuesync effect. The actual mechanism iskey={activeFile.path}ineditor.tsx(line 170) causing full remounts. If external code updates the same file's content without changing the path, the editor won't reflect it.Consider either:
- Updating the comment to accurately describe the key-based remounting strategy, or
- Adding a sync effect using
setValue()if programmatic content updates are needed📝 Option 1: Fix the comment
- // biome-ignore lint/correctness/useExhaustiveDependencies: editor is recreated only on lang change; value is synced separately + // biome-ignore lint/correctness/useExhaustiveDependencies: editor remounts via React key on file change; no runtime value sync♻️ Option 2: Add value sync effect
React.useEffect(() => { const editor = editorRef.current; if (!editor) return; const current = editor.getValue(); if (value !== undefined && current !== value) { editor.setValue(value); } }, [value]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/code-editor.tsx` around lines 151 - 152, The biome-ignore comment above the React.useEffect is misleading because the editor value is not actually synced via setValue; remounting is currently driven by key={activeFile.path} in the component, so external updates to the same path won't update the editor. Fix by either updating the comment to state that the editor is remounted via key={activeFile.path} (not value syncing), or add a value-sync effect that uses editorRef.current.getValue() and editorRef.current.setValue(value) inside a React.useEffect dependent on value so programmatic content updates are applied; reference editorRef, React.useEffect, setValue and key={activeFile.path} when making the change.
201-203: Vim command registration is not reversible.
Vim.defineExregisters a global command with no public API to unregister it. While re-registration safely overrides the previous handler (preventing accumulation), this pattern could cause issues if the codebase evolves to support multiple editor instances—each would overwrite the "write" command. For a single editor, this is acceptable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/code-editor.tsx` around lines 201 - 203, Vim.defineEx("write", "w", () => saveFileRef.current?.()); registers a global command with no unregister API, so change the effect to avoid multiple registrations by adding a module-scoped guard (e.g., isVimWriteRegistered) and only call Vim.defineEx if that flag is false, then set it to true; keep using saveFileRef.current for the handler and keep the React.useEffect with empty deps so the command is defined once across editor instances.apps/web/src/components/shiki-editor.ts (1)
318-318: Module-level compartments limit multi-instance support.
themeCompartment(line 318) andshikiComp(line 1162) are module-level singletons. This works with the current single-editor architecture but would cause state collisions if multipleShikiEditorinstances were needed in the future.Consider moving compartments to instance properties if multi-editor support becomes a requirement.
Also applies to: 1162-1162
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/shiki-editor.ts` at line 318, themeCompartment and shikiComp are module-level singletons which will cause state collisions if multiple ShikiEditor instances are created; convert both to instance properties on the ShikiEditor class by declaring e.g. this.themeCompartment = new Compartment() and this.shikiComp = new Compartment() in the ShikiEditor constructor, update all references from themeCompartment and shikiComp to this.themeCompartment and this.shikiComp (including any places that call EditorState.reconfigure or extensions that use those compartments), and remove the module-level declarations so each editor instance maintains its own compartments.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/components/shiki-editor.ts`:
- Around line 1187-1196: The async init uses this.view after awaiting
createShikiToCodeMirror, which can race with destroy(); add a destroyed guard:
introduce a boolean flag (e.g., this._destroyed) set to true in the destroy()
method, and when resolving the promise from createShikiToCodeMirror (the block
returning getTheme and calling this.view.dispatch with shikiComp.reconfigure),
check if this._destroyed or !this.view before calling this.view.dispatch and
before returning getTheme; if destroyed, skip the dispatch and return a no-op or
null theme. Reference: createShikiToCodeMirror, initPlaygroundHighlighter,
this.getTheme, this.view, destroy().
---
Duplicate comments:
In `@apps/web/src/components/shiki-editor.ts`:
- Around line 633-706: The code recreates cmClasses each highlight pass causing
new class names via newStyleModuleName() and repeated mountStyles calls; change
cmClasses to an instance-level cache (e.g., this.cmStyleClassMap) used by the
highlight loop (replace local cmClasses usages in the loop with
this.cmStyleClassMap), only call newStyleModuleName() when a style string is
missing from the instance cache, and update the mountStyles loop to iterate over
this.cmStyleClassMap so styles are reused across passes (optionally add a
cleanup method to unmount/remove entries if the editor is destroyed).
---
Nitpick comments:
In `@apps/web/src/components/code-editor.tsx`:
- Around line 151-152: The biome-ignore comment above the React.useEffect is
misleading because the editor value is not actually synced via setValue;
remounting is currently driven by key={activeFile.path} in the component, so
external updates to the same path won't update the editor. Fix by either
updating the comment to state that the editor is remounted via
key={activeFile.path} (not value syncing), or add a value-sync effect that uses
editorRef.current.getValue() and editorRef.current.setValue(value) inside a
React.useEffect dependent on value so programmatic content updates are applied;
reference editorRef, React.useEffect, setValue and key={activeFile.path} when
making the change.
- Around line 201-203: Vim.defineEx("write", "w", () =>
saveFileRef.current?.()); registers a global command with no unregister API, so
change the effect to avoid multiple registrations by adding a module-scoped
guard (e.g., isVimWriteRegistered) and only call Vim.defineEx if that flag is
false, then set it to true; keep using saveFileRef.current for the handler and
keep the React.useEffect with empty deps so the command is defined once across
editor instances.
In `@apps/web/src/components/editor.tsx`:
- Around line 168-180: Remove the redundant null-safe checks inside the block
already guarded by "activeFile &&": inside the CodeEditor props use
activeFile.content for value and call getLanguage(activeFile.path) for language
(instead of activeFile? or the ternary). Update the CodeEditor instantiation
(the component named CodeEditor and its props value, language) to reference
activeFile directly; keep existing handlers (handleChange, onRun,
toggleEditorMode) and hotkeys unchanged.
In `@apps/web/src/components/shiki-editor.ts`:
- Line 318: themeCompartment and shikiComp are module-level singletons which
will cause state collisions if multiple ShikiEditor instances are created;
convert both to instance properties on the ShikiEditor class by declaring e.g.
this.themeCompartment = new Compartment() and this.shikiComp = new Compartment()
in the ShikiEditor constructor, update all references from themeCompartment and
shikiComp to this.themeCompartment and this.shikiComp (including any places that
call EditorState.reconfigure or extensions that use those compartments), and
remove the module-level declarations so each editor instance maintains its own
compartments.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cd0fb0a4-1c59-4d74-8b2b-dbca8ed4c58e
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
apps/web/package.jsonapps/web/src/components/code-editor.tsxapps/web/src/components/editor.tsxapps/web/src/components/shiki-editor.tsapps/web/src/stores/editor-store.tspackage.json
✅ Files skipped from review due to trivial changes (1)
- package.json
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/web/package.json
- apps/web/src/stores/editor-store.ts
Purpose
Resolves #29
Summary
Refactors the web app’s editor to use CodeMirror instead of the previous Shiki+textarea overlay. Introduces a Shiki-to-CodeMirror bridge that applies Shiki tokenization and theming inside a CodeMirror EditorView, consolidates editor rendering into a single container, and replaces custom overlay synchronization with CodeMirror-driven editing and extensions.
Key changes
Editor framework migration
Features and UX
Types and API
Dependencies and tooling
Bug fixes
Intent / Outcome
Improve maintainability and extensibility of the in-browser editor by centralizing highlighting and editor behavior in CodeMirror, enabling richer keybindings and modes, simplifying the rendering model, and persisting the user’s editor-mode preference.