From dea3b5928a2ff15f68bd3537184df650d8cda764 Mon Sep 17 00:00:00 2001
From: Shahar Soel <shachar.soel@sap.com>
Date: Wed, 23 Feb 2022 21:22:35 +0200
Subject: [PATCH] fix: only trigger validations for persisted files

fixes #185
---
 .../src/diagnostics/debounce.ts               |  4 +--
 .../diagnostics/isNewPersistedFileVersion.ts  | 27 +++++++++++++++++++
 .../src/editorChanges.ts                      | 22 ++++++++-------
 3 files changed, 40 insertions(+), 13 deletions(-)
 create mode 100644 packages/vscode-dependencies-validation/src/diagnostics/isNewPersistedFileVersion.ts

diff --git a/packages/vscode-dependencies-validation/src/diagnostics/debounce.ts b/packages/vscode-dependencies-validation/src/diagnostics/debounce.ts
index 65c8d8f8..96471e0c 100644
--- a/packages/vscode-dependencies-validation/src/diagnostics/debounce.ts
+++ b/packages/vscode-dependencies-validation/src/diagnostics/debounce.ts
@@ -11,9 +11,7 @@ const OPTIMIZED_PATHS_TO_FUNC: Map<
 > = new Map();
 
 // https://ux.stackexchange.com/questions/95336/how-long-should-the-debounce-timeout-be
-// If the debounce is too small, changes may not be persisted in time to the disk for our
-// the validation logic to run correctly.
-const DEBOUNCE_WAIT = 1500;
+const DEBOUNCE_WAIT = 500;
 
 export function getOptimizedRefreshDiagnostics(
   uri: Uri
diff --git a/packages/vscode-dependencies-validation/src/diagnostics/isNewPersistedFileVersion.ts b/packages/vscode-dependencies-validation/src/diagnostics/isNewPersistedFileVersion.ts
new file mode 100644
index 00000000..22e456d8
--- /dev/null
+++ b/packages/vscode-dependencies-validation/src/diagnostics/isNewPersistedFileVersion.ts
@@ -0,0 +1,27 @@
+const pathToVersion: Map<string, number> = new Map();
+
+/**
+ * Helper to identify when a newly persisted document in vscode represents
+ * the latest version.
+ *
+ * This is needed to help identify which of the **four** events `onDidChangeTextDocument`
+ * which happen on every key click is the "real" trigger for updating the diagnostics
+ */
+export function isNewPersistedFileVersion(
+  path: string,
+  newVer: number
+): boolean {
+  if (pathToVersion.has(path)) {
+    const oldVer = pathToVersion.get(path)!;
+    if (newVer > oldVer) {
+      pathToVersion.set(path, newVer);
+      return true;
+    } else {
+      return false;
+    }
+  } else {
+    pathToVersion.set(path, newVer);
+  }
+
+  return false;
+}
diff --git a/packages/vscode-dependencies-validation/src/editorChanges.ts b/packages/vscode-dependencies-validation/src/editorChanges.ts
index 7b18523f..67652fb2 100644
--- a/packages/vscode-dependencies-validation/src/editorChanges.ts
+++ b/packages/vscode-dependencies-validation/src/editorChanges.ts
@@ -4,14 +4,11 @@ import type {
   TextDocumentChangeEvent,
   TextEditor,
 } from "vscode";
-import { isEmpty } from "lodash";
 import { clearDiagnostics } from "./util";
 import { VscodePackageJsonChangesConfig } from "./vscodeTypes";
 import { getOptimizedRefreshDiagnostics } from "./diagnostics/debounce";
 import { shouldBeChecked } from "./diagnostics/shouldBeChecked";
-
-// TODO: when node_modules deleted or changed diagnostics are not refreshed and errors are not shown for an opened package.json
-// the package.json file should be closed and opened again to trigger diagnostic refresh
+import { isNewPersistedFileVersion } from "./diagnostics/isNewPersistedFileVersion";
 
 export function subscribeToPackageJsonChanges(
   vscodeConfig: VscodePackageJsonChangesConfig
@@ -50,7 +47,11 @@ function executeRefreshDiagnosticsOnEditorChange(
   diagnosticCollection: DiagnosticCollection
 ) {
   return (editor: TextEditor | undefined) => {
-    if (editor && shouldBeChecked(editor.document.uri.path)) {
+    if (
+      editor &&
+      shouldBeChecked(editor.document.uri.path) &&
+      !editor.document.isDirty
+    ) {
       {
         const uri = editor.document.uri;
         const optimizedRefreshDiag = getOptimizedRefreshDiagnostics(uri);
@@ -64,13 +65,14 @@ function executeRefreshDiagnosticsOnDocumentChangeEvent(
   diagnosticCollection: DiagnosticCollection
 ) {
   return (event: TextDocumentChangeEvent) => {
-    const uri = event.document.uri;
+    const document = event.document;
+    const uri = document.uri;
 
     if (
-      // this event gets called multiple (four) times for each text change
-      // but only once for "true" text changes
-      !isEmpty(event.contentChanges) &&
-      shouldBeChecked(uri.path)
+      shouldBeChecked(uri.path) &&
+      !document.isDirty &&
+      // this check should be last as it may modify global state (path -> version cache)
+      isNewPersistedFileVersion(uri.path, document.version)
     ) {
       const optimizedRefreshDiag = getOptimizedRefreshDiagnostics(uri);
       void optimizedRefreshDiag(uri, diagnosticCollection);