[red-knot] Playground improvements (#17109)

## Summary

A few smaller editor improvements that felt worth pulling out of my
other feature PRs:

* Load the `Editor` lazily: This allows splitting the entire monaco
javascript into a separate async bundle, drastically reducing the size
of the `index.js`
* Fix the name of `to_range` and `text_range` to the more idiomatic js
names `toRange` and `textRange`
* Use one indexed values for `Position::line` and `Position::column`,
which is the same as monaco (reduces the need for `+1` and `-1`
operations spread all over the place)
* Preserve the editor state when navigating between tabs. This ensures
that selections are preserved even when switching between tabs.
* Stop the default handling of the `Enter` key press event when renaming
a file because it resulted in adding a newline in the editor

Author: MichaReiser

crates/red_knot_wasm/src/lib.rs

@@ -247,14 +247,14 @@ impl Diagnostic {
         Severity::from(self.inner.severity())
     }
 
-    #[wasm_bindgen]
+    #[wasm_bindgen(js_name = "textRange")]
     pub fn text_range(&self) -> Option<TextRange> {
         self.inner
             .span()
             .and_then(|span| Some(TextRange::from(span.range()?)))
     }
 
-    #[wasm_bindgen]
+    #[wasm_bindgen(js_name = "toRange")]
     pub fn to_range(&self, workspace: &Workspace) -> Option<Range> {
         self.inner.span().and_then(|span| {
             let line_index = line_index(workspace.db.upcast(), span.file());
@@ -287,22 +287,25 @@ pub struct Range {
     pub end: Position,
 }
 
+#[wasm_bindgen]
+#[derive(Copy, Clone, Eq, PartialEq, Debug)]
+pub struct Position {
+    /// One indexed line number
+    pub line: usize,
+
+    /// One indexed column number (the nth character on the line)
+    pub column: usize,
+}
+
 impl From<SourceLocation> for Position {
     fn from(location: SourceLocation) -> Self {
         Self {
-            line: location.row.to_zero_indexed(),
-            character: location.column.to_zero_indexed(),
+            line: location.row.get(),
+            column: location.column.get(),
         }
     }
 }
 
-#[wasm_bindgen]
-#[derive(Copy, Clone, Eq, PartialEq, Debug)]
-pub struct Position {
-    pub line: usize,
-    pub character: usize,
-}
-
 #[wasm_bindgen]
 #[derive(Copy, Clone, Hash, PartialEq, Eq)]
 pub enum Severity {

crates/red_knot_wasm/tests/api.rs

@@ -21,10 +21,7 @@ fn check() {
     assert_eq!(diagnostic.id(), "lint:unresolved-import");
     assert_eq!(
         diagnostic.to_range(&workspace).unwrap().start,
-        Position {
-            line: 0,
-            character: 7
-        }
+        Position { line: 1, column: 8 }
     );
     assert_eq!(diagnostic.message(), "Cannot resolve import `random22`");
 }

playground/knot/src/Editor/Chrome.tsx

@@ -1,4 +1,5 @@
 import {
+  lazy,
   use,
   useCallback,
   useDeferredValue,
@@ -16,15 +17,16 @@ import type { Diagnostic, Workspace } from "red_knot_wasm";
 import { Panel, PanelGroup } from "react-resizable-panels";
 import { Files } from "./Files";
 import SecondarySideBar from "./SecondarySideBar";
-import Editor from "./Editor";
 import SecondaryPanel, {
   SecondaryPanelResult,
   SecondaryTool,
 } from "./SecondaryPanel";
 import Diagnostics from "./Diagnostics";
-import { editor } from "monaco-editor";
-import IStandaloneCodeEditor = editor.IStandaloneCodeEditor;
 import { FileId, ReadonlyFiles } from "../Playground";
+import type { editor } from "monaco-editor";
+import type { Monaco } from "@monaco-editor/react";
+
+const Editor = lazy(() => import("./Editor"));
 
 interface CheckResult {
   diagnostics: Diagnostic[];
@@ -66,11 +68,14 @@ export default function Chrome({
     null,
   );
 
-  const editorRef = useRef<IStandaloneCodeEditor | null>(null);
+  const editorRef = useRef<{
+    editor: editor.IStandaloneCodeEditor;
+    monaco: Monaco;
+  } | null>(null);
 
   const handleFileRenamed = (file: FileId, newName: string) => {
     onFileRenamed(workspace, file, newName);
-    editorRef.current?.focus();
+    editorRef.current?.editor.focus();
   };
 
   const handleSecondaryToolSelected = useCallback(
@@ -86,12 +91,15 @@ export default function Chrome({
     [],
   );
 
-  const handleEditorMount = useCallback((editor: IStandaloneCodeEditor) => {
-    editorRef.current = editor;
-  }, []);
+  const handleEditorMount = useCallback(
+    (editor: editor.IStandaloneCodeEditor, monaco: Monaco) => {
+      editorRef.current = { editor, monaco };
+    },
+    [],
+  );
 
   const handleGoTo = useCallback((line: number, column: number) => {
-    const editor = editorRef.current;
+    const editor = editorRef.current?.editor;
 
     if (editor == null) {
       return;
@@ -107,6 +115,25 @@ export default function Chrome({
     editor.setSelection(range);
   }, []);
 
+  const handleRemoved = useCallback(
+    async (id: FileId) => {
+      const name = files.index.find((file) => file.id === id)?.name;
+
+      if (name != null && editorRef.current != null) {
+        // Remove the file from the monaco state to avoid that monaco "restores" the old content.
+        // An alternative is to use a `key` on the `Editor` but that means we lose focus and selection
+        // range when changing between tabs.
+        const monaco = await import("monaco-editor");
+        editorRef.current.monaco.editor
+          .getModel(monaco.Uri.file(name))
+          ?.dispose();
+      }
+
+      onFileRemoved(workspace, id);
+    },
+    [workspace, files.index, onFileRemoved],
+  );
+
   const checkResult = useCheckResult(files, workspace, secondaryTool);
 
   return (
@@ -120,7 +147,7 @@ export default function Chrome({
             onAdd={(name) => onFileAdded(workspace, name)}
             onRename={handleFileRenamed}
             onSelected={onFileSelected}
-            onRemove={(id) => onFileRemoved(workspace, id)}
+            onRemove={handleRemoved}
           />
           <PanelGroup direction="horizontal" autoSaveId="main">
             <Panel
@@ -132,7 +159,6 @@ export default function Chrome({
               <PanelGroup id="vertical" direction="vertical">
                 <Panel minSize={10} className="my-2" order={0}>
                   <Editor
-                    key={selectedFileName}
                     theme={theme}
                     visible={true}
                     fileName={selectedFileName}

playground/knot/src/Editor/Diagnostics.tsx

@@ -20,7 +20,7 @@ export default function Diagnostics({
   const diagnostics = useMemo(() => {
     const sorted = [...unsorted];
     sorted.sort((a, b) => {
-      return (a.text_range()?.start ?? 0) - (b.text_range()?.start ?? 0);
+      return (a.textRange()?.start ?? 0) - (b.textRange()?.start ?? 0);
     });
 
     return sorted;
@@ -73,15 +73,15 @@ function Items({
   return (
     <ul className="space-y-0.5 grow overflow-y-scroll">
       {diagnostics.map((diagnostic, index) => {
-        const position = diagnostic.to_range(workspace);
+        const position = diagnostic.toRange(workspace);
         const start = position?.start;
         const id = diagnostic.id();
 
-        const startLine = (start?.line ?? 0) + 1;
-        const startColumn = (start?.character ?? 0) + 1;
+        const startLine = start?.line ?? 1;
+        const startColumn = start?.column ?? 1;
 
         return (
-          <li key={`${diagnostic.text_range()?.start ?? 0}-${id ?? index}`}>
+          <li key={`${startLine}:${startColumn}-${id ?? index}`}>
             <button
               onClick={() => onGoTo(startLine, startColumn)}
               className="w-full text-start cursor-pointer select-text"

playground/knot/src/Editor/Editor.tsx

@@ -18,7 +18,7 @@ type Props = {
   theme: Theme;
   workspace: Workspace;
   onChange(content: string): void;
-  onMount(editor: IStandaloneCodeEditor): void;
+  onMount(editor: IStandaloneCodeEditor, monaco: Monaco): void;
 };
 
 type MonacoEditorState = {
@@ -63,7 +63,7 @@ export default function Editor({
         monaco: instance,
       };
 
-      onMount(editor);
+      onMount(editor, instance);
     },
 
     [onMount, workspace, diagnostics],
@@ -120,14 +120,14 @@ function updateMarkers(
         }
       };
 
-      const range = diagnostic.to_range(workspace);
+      const range = diagnostic.toRange(workspace);
 
       return {
         code: diagnostic.id(),
-        startLineNumber: (range?.start?.line ?? 0) + 1,
-        startColumn: (range?.start?.character ?? 0) + 1,
-        endLineNumber: (range?.end?.line ?? 0) + 1,
-        endColumn: (range?.end?.character ?? 0) + 1,
+        startLineNumber: range?.start?.line ?? 0,
+        startColumn: range?.start?.column ?? 0,
+        endLineNumber: range?.end?.line ?? 0,
+        endColumn: range?.end?.column ?? 0,
         message: diagnostic.message(),
         severity: mapSeverity(diagnostic.severity()),
         tags: [],

playground/knot/src/Editor/Files.tsx

@@ -182,6 +182,7 @@ function FileEntry({ name, onClicked, onRenamed, selected }: FileEntryProps) {
             switch (event.key) {
               case "Enter":
                 event.currentTarget.blur();
+                event.preventDefault();
                 return;
               case "Escape":
                 setNewName(null);