Fix RangeError due to stale signature help tooltips

src/editor/codemirror/language-server/signatureHelp.ts

@@ -7,7 +7,7 @@
  *
  * SPDX-License-Identifier: MIT
  */
-import { StateEffect, StateField } from "@codemirror/state";
+import { EditorState, StateEffect, StateField } from "@codemirror/state";
 import {
   Command,
   EditorView,
@@ -16,7 +16,6 @@ import {
   logException,
   PluginValue,
   showTooltip,
-  Tooltip,
   ViewPlugin,
   ViewUpdate,
 } from "@codemirror/view";
@@ -38,17 +37,23 @@ import {
 import { nameFromSignature, removeFullyQualifiedName } from "./names";
 import { offsetToPosition } from "./positions";
 
-interface SignatureChangeEffect {
-  pos: number;
-  result: SignatureHelp | null;
-}
+export const setSignatureHelpRequestPosition = StateEffect.define<number>({});
 
-export const setSignatureHelpEffect = StateEffect.define<SignatureChangeEffect>(
+export const setSignatureHelpResult = StateEffect.define<SignatureHelp | null>(
   {}
 );
 
 interface SignatureHelpState {
-  tooltip: Tooltip | null;
+  /**
+   * -1 for no signature help requested.
+   */
+  pos: number;
+  /**
+   * The latest result we want to display.
+   *
+   * This may be out of date while we wait for async response from LSP
+   * but we display it as it's generally useful.
+   */
   result: SignatureHelp | null;
 }
 
@@ -63,32 +68,42 @@ const signatureHelpToolTipBaseTheme = EditorView.baseTheme({
   },
 });
 
-const triggerSignatureHelpRequest = async (view: EditorView): Promise<void> => {
-  const uri = view.state.facet(uriFacet)!;
-  const client = view.state.facet(clientFacet)!;
-  const pos = view.state.selection.main.from;
+const triggerSignatureHelpRequest = async (
+  view: EditorView,
+  state: EditorState
+): Promise<void> => {
+  const uri = state.facet(uriFacet)!;
+  const client = state.facet(clientFacet)!;
+  const pos = state.selection.main.from;
   const params: SignatureHelpParams = {
     textDocument: { uri },
-    position: offsetToPosition(view.state.doc, pos),
+    position: offsetToPosition(state.doc, pos),
   };
   try {
+    // Must happen before other event handling that might dispatch more
+    // changes thatt invalidate our position.
+    queueMicrotask(() => {
+      view.dispatch({
+        effects: [setSignatureHelpRequestPosition.of(pos)],
+      });
+    });
     const result = await client.connection.sendRequest(
       SignatureHelpRequest.type,
       params
     );
     view.dispatch({
-      effects: [setSignatureHelpEffect.of({ pos, result })],
+      effects: [setSignatureHelpResult.of(result)],
     });
   } catch (e) {
-    logException(view.state, e, "signature-help");
+    logException(state, e, "signature-help");
     view.dispatch({
-      effects: [setSignatureHelpEffect.of({ pos, result: null })],
+      effects: [setSignatureHelpResult.of(null)],
     });
   }
 };
 
 const openSignatureHelp: Command = (view: EditorView) => {
-  triggerSignatureHelpRequest(view);
+  triggerSignatureHelpRequest(view, view.state);
   return true;
 };
 
@@ -99,27 +114,62 @@ export const signatureHelp = (
 ) => {
   const signatureHelpTooltipField = StateField.define<SignatureHelpState>({
     create: () => ({
+      pos: -1,
       result: null,
-      tooltip: null,
     }),
     update(state, tr) {
+      let pos = state.pos;
+      let result = state.result;
       for (const effect of tr.effects) {
-        if (effect.is(setSignatureHelpEffect)) {
-          return reduceSignatureHelpState(state, effect.value, apiReferenceMap);
+        if (effect.is(setSignatureHelpRequestPosition)) {
+          pos = effect.value;
+          if (pos === -1) {
+            result = null;
+          }
+        } else if (effect.is(setSignatureHelpResult)) {
+          result = effect.value;
+          if (result === null) {
+            // No need to ask for more updates until triggered again.
+            pos = -1;
+          }
         }
       }
-      return state;
+
+      pos = pos === -1 ? -1 : tr.changes.mapPos(pos);
+      if (state.pos === pos && state.result === result) {
+        // Avoid pointless tooltip updates. If nothing else it makes e2e tests hard.
+        return state;
+      }
+      return {
+        pos,
+        result,
+      };
     },
-    provide: (f) => showTooltip.from(f, (val) => val.tooltip),
+    provide: (f) =>
+      showTooltip.from(f, (val) => {
+        const { result, pos } = val;
+        if (result) {
+          return {
+            pos,
+            above: true,
+            // This isn't great but the impact is really bad when it conflicts with autocomplete.
+            // strictSide: true,
+            create: () => {
+              const dom = document.createElement("div");
+              dom.className = "cm-signature-tooltip";
+              dom.appendChild(formatSignatureHelp(result, apiReferenceMap));
+              return { dom };
+            },
+          };
+        }
+        return null;
+      }),
   });
 
   const closeSignatureHelp: Command = (view: EditorView) => {
-    if (view.state.field(signatureHelpTooltipField).tooltip) {
+    if (view.state.field(signatureHelpTooltipField).pos !== -1) {
       view.dispatch({
-        effects: setSignatureHelpEffect.of({
-          pos: -1,
-          result: null,
-        }),
+        effects: setSignatureHelpRequestPosition.of(-1),
       });
       return true;
     }
@@ -139,64 +189,29 @@ export const signatureHelp = (
     constructor(view: EditorView, private automatic: boolean) {
       super(view);
     }
-    update({ docChanged, selectionSet, transactions }: ViewUpdate) {
+    update(update: ViewUpdate) {
       if (
-        (docChanged || selectionSet) &&
-        this.view.state.field(signatureHelpTooltipField).tooltip
+        (update.docChanged || update.selectionSet) &&
+        this.view.state.field(signatureHelpTooltipField).pos !== -1
       ) {
-        triggerSignatureHelpRequest(this.view);
-      } else if (this.automatic && docChanged) {
-        const last = transactions[transactions.length - 1];
+        triggerSignatureHelpRequest(this.view, update.state);
+      } else if (this.automatic && update.docChanged) {
+        const last = update.transactions[update.transactions.length - 1];
 
         // This needs to trigger for autocomplete adding function parens
         // as well as normal user input with `closebrackets` inserting
         // the closing bracket.
         if (last.isUserEvent("input") || last.isUserEvent("dnd.drop.call")) {
           last.changes.iterChanges((_fromA, _toA, _fromB, _toB, inserted) => {
             if (inserted.sliceString(0).trim().endsWith("()")) {
-              triggerSignatureHelpRequest(this.view);
+              triggerSignatureHelpRequest(this.view, update.state);
             }
           });
         }
       }
     }
   }
 
-  const reduceSignatureHelpState = (
-    state: SignatureHelpState,
-    effect: SignatureChangeEffect,
-    apiReferenceMap: ApiReferenceMap
-  ): SignatureHelpState => {
-    if (state.tooltip && !effect.result) {
-      return {
-        result: null,
-        tooltip: null,
-      };
-    }
-    // It's a bit weird that we always update the position, but VS Code does this too.
-    // I think ideally we'd have a notion of "same function call". Does the
-    // node have a stable identity?
-    if (effect.result) {
-      const result = effect.result;
-      return {
-        result,
-        tooltip: {
-          pos: effect.pos,
-          above: true,
-          // This isn't great but the impact is really bad when it conflicts with autocomplete.
-          // strictSide: true,
-          create: () => {
-            const dom = document.createElement("div");
-            dom.className = "cm-signature-tooltip";
-            dom.appendChild(formatSignatureHelp(result, apiReferenceMap));
-            return { dom };
-          },
-        },
-      };
-    }
-    return state;
-  };
-
   const formatSignatureHelp = (
     help: SignatureHelp,
     apiReferenceMap: ApiReferenceMap

src/editor/codemirror/lint/editingLine.ts

@@ -34,11 +34,11 @@ export const editingLinePlugin = ViewPlugin.fromClass(
         if (!foundEditOnLine && doc.lineAt(fromB).number === selectionLine) {
           foundEditOnLine = true;
           clearTimeout(this.timeout);
-          setTimeout(() => {
+          queueMicrotask(() => {
             update.view.dispatch({
               effects: [setEditingLineEffect.of(selectionLine)],
             });
-          }, 0);
+          });
           this.timeout = setTimeout(() => {
             update.view.dispatch({
               effects: [setEditingLineEffect.of(undefined)],
@@ -51,11 +51,11 @@ export const editingLinePlugin = ViewPlugin.fromClass(
         update.state.field(editingLineState) !== selectionLine
       ) {
         clearTimeout(this.timeout);
-        setTimeout(() => {
+        queueMicrotask(() => {
           update.view.dispatch({
             effects: [setEditingLineEffect.of(undefined)],
           });
-        }, 0);
+        });
       }
     }