Refactor: decouple CodeMirror and React #9190

franknoirot · 2025-12-05T09:35:21Z

WIP towards #8880, built off of #9064 which was a proof-of-concept. This still has just a bit more polish to be done but I think I've proven that this is very doable. Most of the cleanup remaining comes from needing to translate our overwrought scaffolding around CodeMirror and the EditorState into neater little effects. I think by the end of this the execution loop might be a bare CodeMirror plugin, which I am fucking thrilled about.

vercel · 2025-12-05T09:35:26Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Review	Updated (UTC)
modeling-app	Ready	Preview, Comment	Dec 17, 2025 2:35am

andrewvarga · 2025-12-05T14:24:02Z

src/lang/KclManager.ts

+  // setEditorView(editorView: EditorView) {
+  //   this.overrideTreeHighlighterUpdateForPerformanceTracking()
+  // }


Ohh I just saw this! There is an issue I've created recently: #9184

Basically we called overrideTreeHighlighterUpdateForPerformanceTracking each time an edit was made wrapping the update function multiple times.
A simple fix is to keep track of the plugins already patched, or only call this once at init time (or if plugins change).

My fix would very likely conflict with this PR so I will postpone fixing it. Let me know if you prefer to fix it in your PR, or otherwise I will wait for this to merge and fix afterwards.

Oh yeah I wanted to put a pin in that while I worked out the other unknowns in this PR, but I think I can bring this back or solve what it's trying to do much more simply now that we won't re-render all over the place and obliterate the EditorView.

franknoirot

I'm really excited for this one.

franknoirot · 2025-12-05T15:33:54Z

src/components/CommandBar/CommandBarKclInput.tsx

 import type { Node } from '@rust/kcl-lib/bindings/Node'

 import { CustomIcon } from '@src/components/CustomIcon'
-import { useCodeMirror } from '@src/components/layout/areas/CodeEditor'


All the diff in this PR is to get rid of this import so I could delete that file. What I've got is the start of a "microEditor" CodeMirror setup defined below, which we can package up and use as our common KCL input in a follow-up PR. Most of the annoyance here is now just that the component still re-renders like mad because off all my careless useMemos and so on, but that's how it actually is in prod.

franknoirot · 2025-12-05T15:35:17Z

src/components/layout/areas/KclEditorPane.tsx

+  const editorParent = useRef<HTMLDivElement>(null)
  useEffect(() => {
-    return () => {
-      kclEditorActor.send({ type: 'setKclEditorMounted', data: false })
-      kclEditorActor.send({ type: 'setLastSelectionEvent', data: undefined })
-      kclManager.diagnostics = []
-    }
+    editorParent.current?.appendChild(kclManager.editorView.dom)


BYE REACT DON'T NEED YA

franknoirot · 2025-12-05T15:38:31Z

src/components/LspProvider.tsx

+      // New code to just update the CodeMirror extensions directly.
+      kclManager.editorView.dispatch({
+        effects: kclLspCompartment.reconfigure(Prec.highest(lsp)),
+      })


I left basically everything the same with the LspProvider but it can go soon. I'm pretty confident we can make it instantiate when a project is opened just from KclManager and never touch React, especially once we get a proper openProject method in there and store the opened project and executing file; those are the only things that the useEffects in this component are here for.

franknoirot · 2025-12-05T15:39:53Z

src/editor/plugins/lsp/copilot/index.ts

 } from '@kittycad/codemirror-lsp-client'
 import { reportRejection } from '@src/lib/trap'
-import { deferExecution } from '@src/lib/utils'
+import { deferredCallback } from '@src/lib/utils'


Sure it's kinda noisy, but when I saw deferExecution I thought for sure this was some kind of engine execution wrapper, and it's really just a nice little deferring utility (perhaps with overlapping usefulness with our debounce function but that's for another day)

franknoirot · 2025-12-05T15:40:50Z

src/editor/plugins/lsp/kcl/index.ts

-
-/** A view plugin that requests completions from the server after a delay */
-export class KclPlugin implements PluginValue {
-  private viewUpdate: ViewUpdate | null = null


This class would hold onto viewUpdate, a very risky thing to hold onto since they're so ephemeral.

franknoirot · 2025-12-05T16:10:48Z

src/lang/KclManager.ts

  set code(code: string) {
+    this.editorView.dispatch({
+      changes: {
+        from: 0,
+        to: this.editorView.state.doc.length,
+        insert: code,
+      },
+    })
    this._code.value = code


Now setting the code value will dispatch, instead of bounce around.

franknoirot · 2025-12-05T16:13:34Z

src/lang/KclManager.ts

-            .writeFile(this._currentFilePath, this.code ?? '')
+            .writeFile(this._currentFilePath, newCode)


Same as with executeCode above, this.code is a bad pattern and not guaranteed to be up-to-date. Now we pass in new code from the ViewUpdate listener extension, which is.

franknoirot · 2025-12-05T16:14:00Z

src/lib/codeEditor.ts

Moved to @src/editor/plugins/theme/index.ts

franknoirot · 2025-12-05T16:14:55Z

src/machines/settingsMachine.ts

Now that I'm comfortable working with CodeMirror I loathe the boilerplate and type runaround of how I've used XState here.

franknoirot · 2025-12-05T16:15:13Z

src/components/layout/areas/CodeEditor.tsx

BYE REACT BYE

Baby step towards #8885 broken out from a larger effort in #9190. Just meant to start categorizing and commenting on the many class properties in this class, as we step towards consolidating them.

Towards #8885, stacked on #9236. Adding a for-now unused CodeMirror extension that registers the AST as a StateField on the EditorView. Right now we trash the EditorView constantly so that value will not be trustworthy, but it will be once we remove all our React scaffolding around CodeMirror, which is a work-in-progress in #9190. Then this will be available to become the canonical derived AST state, automatically updated whenever the code is updated by the user.

franknoirot · 2025-12-09T00:45:20Z

Taking just a bit of a detour here so that I can break out some refactor PRs and PRs that add CodeMirror extensions for other derived state. I'll return to this after a few of the PRs I've broken out from it land.

Towards #8885, broken out from #9190. Refactors the KCL LSP extension to not store any EditorView or other state. In addition, splits out concerns of highlighting engine-side entities and executing code. Users should experience no degradations from this refactor, although it is possible that this fixes some niche case bugs.

* Organize class properties on KclManager Baby step towards #8885 broken out from a larger effort in #9190. Just meant to start categorizing and commenting on the many class properties in this class, as we step towards consolidating them. * Update snapshots * Update snapshots * Revert AST setter This contained material changes that break the app currently, relying on other future changes. * Update snapshots * Update snapshots * Update snapshots * Revert changes to `code` setter These rely on other changes to come to work properly. * Update snapshots --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

Now we gotta fix the world in this PR hold onto your butts!

…chine

And a risky one at that!

It's such a bummer that we don't receive this as a signal it's crazy

vercel bot deployed to Preview December 5, 2025 09:46 View deployment

vercel bot deployed to Preview December 5, 2025 10:44 View deployment

franknoirot force-pushed the franknoirot/8880/kill-react-codemirror branch from 3de593d to c294f66 Compare December 5, 2025 10:45

vercel bot deployed to Preview December 5, 2025 11:13 View deployment

vercel bot deployed to Preview December 5, 2025 12:12 View deployment

andrewvarga reviewed Dec 5, 2025

View reviewed changes

andrewvarga mentioned this pull request Dec 5, 2025

performanceTrackingUpdate recursion #9184

Open

vercel bot deployed to Preview December 5, 2025 15:17 View deployment

franknoirot force-pushed the franknoirot/8880/kill-react-codemirror branch from 6c81094 to b946a6d Compare December 5, 2025 15:26

vercel bot deployed to Preview December 5, 2025 16:05 View deployment

franknoirot commented Dec 5, 2025

View reviewed changes

franknoirot force-pushed the franknoirot/8880/kill-react-codemirror branch from b6dc567 to f7fe680 Compare December 5, 2025 16:19

vercel bot deployed to Preview December 5, 2025 17:26 View deployment

franknoirot mentioned this pull request Dec 8, 2025

Organize class properties on KclManager #9236

Merged

franknoirot mentioned this pull request Dec 8, 2025

Create CodeMirror extension to register AST as StateField #9237

Merged

franknoirot mentioned this pull request Dec 9, 2025

Add CodeMirror extension for Operations #9245

Draft

franknoirot mentioned this pull request Dec 9, 2025

Remove test that has always failed half of the time #9248

Merged

franknoirot force-pushed the franknoirot/8880/kill-react-codemirror branch from 423fe17 to d1d4b0f Compare December 9, 2025 21:54

vercel bot deployed to Preview December 9, 2025 22:33 View deployment

vercel bot deployed to Preview December 9, 2025 23:48 View deployment

franknoirot mentioned this pull request Dec 10, 2025

Refactor KclPlugin to not keep any state #9272

Draft

franknoirot force-pushed the franknoirot/8880/kill-react-codemirror branch from ab7d796 to df5305e Compare December 10, 2025 16:11

franknoirot added 23 commits December 16, 2025 15:29

Remove the string return from initialiseWasm to clean up type

46f1d28

Now we gotta fix the world in this PR hold onto your butts!

Dep inject initPromise into settingsUtils, kclManager into settingsMa…

adfbe23

…chine

Remove exports of initPromise and initPromiseNode

48eba12

Oops pulled on the big thread

6b26b69

Pare down CodeMirror EditorView to live outside of React

7de9fef

Get blinking cursor setting working without react too

4a75001

Wire up autocompletions again

1e6f324

Get registered hotkeys working again, delete unused code

eef9083

Convert CommandBarKclInput to not use useCodeMirror

dd43e76

Don't let a circular dependency in now ya hear?

4878a38

Don't count hotkey registrations as relevant updates

f17e2f0

Rename deferExecution to deferredCallback, it's way too confusing

7a3aad7

KILL KCL VIEW PLUGIN LFG

a88d1b9

Clean up these extensions, give them proper names

5c516dc

Bring back this line of code, I think that was a miss

ed3f8ed

And a risky one at that!

Post-rebase fixup

95a41fb

fix highlighting extension

0dfc8e6

Better names for effect extensions

73ab7a7

Zoom-to-fit on initial load via annotation and updateListener

631ff89

tsc

1dd2014

WIP hacky timeout to wait for execution to complete

4c373ed

It's such a bummer that we don't receive this as a signal it's crazy

remove annotation approach it's not doing anything and makes circulars

6e9e452

Rebase fixups, classic

d8b7e66

franknoirot force-pushed the franknoirot/8880/kill-react-codemirror branch from ebcd92b to d8b7e66 Compare December 16, 2025 20:38

Update snapshots

895a8e6

vercel bot deployed to Preview December 16, 2025 21:04 View deployment

franknoirot and others added 2 commits December 16, 2025 21:11

More fixes

50f2931

Update snapshots

4f29226

vercel bot deployed to Preview December 17, 2025 02:35 View deployment

franknoirot mentioned this pull request Dec 22, 2025

Decouple React and CodeMirror (v2) #9448

Draft

		.writeFile(this._currentFilePath, this.code ?? '')
		.writeFile(this._currentFilePath, newCode)

Refactor: decouple CodeMirror and React #9190

Are you sure you want to change the base?

Refactor: decouple CodeMirror and React #9190

Uh oh!

Conversation

franknoirot commented Dec 5, 2025

Uh oh!

vercel bot commented Dec 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

franknoirot left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

franknoirot commented Dec 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

vercel bot commented Dec 5, 2025 •

edited

Loading

franknoirot commented Dec 9, 2025 •

edited

Loading