fix(views): engine events; update DendronTreeView reliably (#2269)

* fix: add Engine Events Interface in EngineAPIService

* spike: dendron tree view update

* fix: moving DendronTreeViewV1 to utilize new engine events

* spike: adding tests; fixing engine bugs

* spike: mark NoteSyncService onNoteChange as deprecated

* spike: fixing bugs in extension and engineAPIService

* fix: moving EngineEvents to engine-server

* fix: some fixes on NoteChangeEntry payloads from storev2

* fix: update EngineEvent event name

* spike: fix build issues

* fix: test adjustments

* spike: fixing unused import

* spike: cr feedback

* spike: testAssertsInsideCallback util fn

packages/common-all/src/dnode.ts

@@ -26,6 +26,7 @@ import {
   DNodePropsQuickInputV2,
   DNoteLoc,
   DVault,
+  NoteChangeEntry,
   NoteLocalConfig,
   NoteOpts,
   NoteProps,
@@ -348,30 +349,34 @@ export class NoteUtils {
   }
 
   /**
-   * Add node to parent
-   * Create stubs if no direct parent exists
+   * Add node to parents up the note tree, or create stubs if no direct parents exists
    * @param opts
-   * @returns All notes that were changed including the parent
+   * @returns All notes that were changed including the parents
    */
-  static addParent(opts: {
+  static addOrUpdateParents(opts: {
     note: NoteProps;
     notesList: NoteProps[];
     createStubs: boolean;
     wsRoot: string;
-  }): NoteProps[] {
+  }): NoteChangeEntry[] {
     const { note, notesList, createStubs, wsRoot } = opts;
     const parentPath = DNodeUtils.dirName(note.fname).toLowerCase();
-    let parent =
+    const parent =
       _.find(
         notesList,
         (p) =>
           p.fname.toLowerCase() === parentPath &&
           VaultUtils.isEqual(p.vault.fsPath, note.vault.fsPath, wsRoot)
       ) || null;
-    const changed: NoteProps[] = [];
+    const changed: NoteChangeEntry[] = [];
     if (parent) {
-      changed.push(parent);
+      const prevParentState = { ...parent };
       DNodeUtils.addChild(parent, note);
+      changed.push({
+        status: "update",
+        prevNote: prevParentState,
+        note: parent,
+      });
     }
     if (!parent && !createStubs) {
       const err = {
@@ -383,14 +388,25 @@ export class NoteUtils {
       throw DendronError.createFromStatus(err);
     }
     if (!parent) {
-      parent = DNodeUtils.findClosestParent(note.fname, notesList, {
+      const ancestor = DNodeUtils.findClosestParent(note.fname, notesList, {
         vault: note.vault,
         wsRoot,
       }) as NoteProps;
-      changed.push(parent);
-      const stubNodes = NoteUtils.createStubs(parent, note);
-      stubNodes.forEach((ent2) => {
-        changed.push(ent2);
+
+      const prevAncestorState = { ...ancestor };
+
+      const stubNodes = NoteUtils.createStubs(ancestor, note);
+      stubNodes.forEach((stub) => {
+        changed.push({
+          status: "create",
+          note: stub,
+        });
+      });
+
+      changed.push({
+        status: "update",
+        prevNote: prevAncestorState,
+        note: ancestor,
       });
     }
     return changed;

packages/common-all/src/types/events.ts

@@ -0,0 +1,68 @@
+import { Disposable } from "./compat";
+import { EventEmitter as NodeEventEmitter } from "events";
+
+/**
+ * Represents a typed event that can be subscribed to.
+ *
+ * This has an indentical interface signature to vscode.Event, but can be used
+ * outside of plugin-core.
+ */
+export interface Event<T> {
+  /**
+   * A function that represents an event to which you subscribe by calling it with
+   * a listener function as argument.
+   *
+   * @param listener The listener function will be called when the event happens.
+   * @param thisArgs The `this`-argument which will be used when calling the event listener.
+   */
+  (listener: (e: T) => any, thisArgs?: any): Disposable;
+}
+
+/**
+ * An event emitter can be used to create and manage an {@link Event} for others
+ * to subscribe to. One emitter always owns one event.
+ *
+ * This mimics the interface signature of vscode.EventEmitter but can be used
+ * outside of plugin-core.
+ */
+export class EventEmitter<T> {
+  _emitter: NodeEventEmitter = new NodeEventEmitter();
+  _EVENT_CHANNEL = "default";
+
+  /**
+   * The event listeners can subscribe to.
+   */
+  event: Event<T> = (listener, thisArgs?: any): Disposable => {
+    let callback = listener;
+    if (thisArgs) {
+      callback = callback.bind(thisArgs);
+    }
+
+    this._emitter.on(this._EVENT_CHANNEL, callback);
+
+    const disposable: Disposable = {
+      dispose: () => {
+        this._emitter.removeListener(this._EVENT_CHANNEL, callback);
+      },
+    };
+
+    return disposable;
+  };
+
+  /**
+   * Notify all subscribers of the {@link EventEmitter.event event}. Failure
+   * of one or more listener will not fail this function call.
+   *
+   * @param data The event object.
+   */
+  fire(data: T): void {
+    this._emitter.emit(this._EVENT_CHANNEL, data);
+  }
+
+  /**
+   * Dispose this object and free resources.
+   */
+  dispose(): void {
+    this._emitter.removeAllListeners(this._EVENT_CHANNEL);
+  }
+}

packages/common-all/src/types/index.ts

@@ -1,3 +1,4 @@
+/* eslint-disable camelcase */
 import { DNodeType, NoteProps } from "./foundation";
 
 export * from "./compat";
@@ -15,6 +16,7 @@ export * from "./compat";
 export * from "./editor";
 export * from "./lookup";
 export * from "./unified";
+export * from "./events";
 
 export type Stage = "dev" | "prod" | "test";
 export type DEngineQuery = {

packages/common-frontend/src/features/engine/slice.ts

@@ -159,7 +159,7 @@ export const engineSlice = createSlice({
       }
       // this is a new node
       if (!state.notes[note.id]) {
-        NoteUtils.addParent({
+        NoteUtils.addOrUpdateParents({
           note,
           notesList: _.values(state.notes),
           createStubs: true,

packages/common-test-utils/src/utilsv2.ts

@@ -91,3 +91,22 @@ export class TestPresetEntryV4 {
     ];
   }
 }
+
+/**
+ * If you need to do assert/expect verification inside a callback, then use this
+ * method to wrap any assert calls. Otherwise, any assert failures will result
+ * in a failed promise instead of an exception, which will cause the test to
+ * hang until the test timeout instead of failing immediately with the right
+ * error message.
+ * @param asserts a function containing your assert/expect statements that you
+ * want to test in your test case
+ * @param doneArg a jest or mocha done argument
+ */
+export function testAssertsInsideCallback(asserts: () => void, doneArg: any) {
+  try {
+    asserts();
+    doneArg();
+  } catch (err) {
+    doneArg(err);
+  }
+}

packages/engine-server/src/EngineEventEmitter.ts

@@ -0,0 +1,12 @@
+import { Disposable, Event, NoteChangeEntry } from "@dendronhq/common-all";
+
+/**
+ * Interface providing events signaling changes that have been made to engine
+ * state
+ */
+export interface EngineEventEmitter extends Disposable {
+  /**
+   * Event that fires upon the changing of note state in the engine.
+   */
+  get onEngineNoteStateChanged(): Event<NoteChangeEntry[]>;
+}

packages/engine-server/src/drivers/file/noteParser.ts

@@ -241,7 +241,6 @@ export class NoteParser extends ParserBase {
 
     // get note props
     try {
-      // noteProps = file2Note(path.join(vpath, fileMeta.fpath), vault);
       ({
         note: noteProps,
         noteHash,
@@ -265,13 +264,13 @@ export class NoteParser extends ParserBase {
 
     // add parent
     if (cleanOpts.addParent) {
-      const stubs = NoteUtils.addParent({
+      const stubs = NoteUtils.addOrUpdateParents({
         note: noteProps,
         notesList: _.uniqBy(_.values(notesByFname).concat(parents), "id"),
         createStubs: cleanOpts.createStubs,
         wsRoot: this.opts.store.wsRoot,
       });
-      out = out.concat(stubs);
+      out = out.concat(stubs.map((noteChangeEntry) => noteChangeEntry.note));
     }
     return { propsList: out, noteHash, matchHash };
   }

packages/engine-server/src/drivers/file/storev2.ts

@@ -964,7 +964,7 @@ export class FileStorage implements DStore {
       note = NoteUtils.hydrate({ noteRaw: note, noteHydrated: maybeNote });
     }
     if (opts?.newNode) {
-      NoteUtils.addParent({
+      NoteUtils.addOrUpdateParents({
         note,
         notesList: _.values(this.notes),
         createStubs: true,
@@ -989,23 +989,25 @@ export class FileStorage implements DStore {
 
   /**
    * Write a new note. Also take care of updating logic of parents and children if new note replaces an existing note
+   * @param param0
+   * @returns - Changed Entries
    */
-  async _writeNewNote({
+  private async _writeNewNote({
     note,
     existingNote,
     opts,
   }: {
     note: NoteProps;
     existingNote?: NoteProps;
     opts?: EngineWriteOptsV2;
-  }): Promise<NoteProps[]> {
+  }): Promise<NoteChangeEntry[]> {
     const ctx = "_writeNewNote";
     this.logger.info({
       ctx,
       msg: "enter",
       note: NoteUtils.toLogObj(note),
     });
-    let changed: NoteProps[] = [];
+    let changed: NoteChangeEntry[] = [];
     // in this case, we are deleting the old note and writing a new note in its place with the same hierarchy
     // the parent of this note needs to have the old note removed (because the id is now different)
     // the new note needs to have the old note's children
@@ -1030,7 +1032,7 @@ export class FileStorage implements DStore {
     // eg. if user created `baz.one.two` and neither `baz` or `baz.one` exist, then they need to be created
     // this is the default behavior
     if (!opts?.noAddParent) {
-      changed = NoteUtils.addParent({
+      changed = NoteUtils.addOrUpdateParents({
         note,
         notesList: _.values(this.notes),
         createStubs: true,
@@ -1040,7 +1042,7 @@ export class FileStorage implements DStore {
     this.logger.info({
       ctx,
       msg: "exit",
-      changed: changed.map((n) => NoteUtils.toLogObj(n)),
+      changed: changed.map((n) => NoteUtils.toLogObj(n.note)),
     });
     return changed;
   }
@@ -1050,7 +1052,7 @@ export class FileStorage implements DStore {
     opts?: EngineWriteOptsV2
   ): Promise<WriteNoteResp> {
     const ctx = `FileStore:writeNote:${note.fname}`;
-    let changed: NoteProps[] = [];
+    let changedEntries: NoteChangeEntry[] = [];
     let error: DendronError | null = null;
     this.logger.info({
       ctx,
@@ -1078,7 +1080,7 @@ export class FileStorage implements DStore {
       note = { ...maybeNote, ...note };
       noDelete = true;
     } else {
-      changed = await this._writeNewNote({
+      changedEntries = await this._writeNewNote({
         note,
         existingNote: maybeNote,
         opts,
@@ -1158,20 +1160,19 @@ export class FileStorage implements DStore {
       const { schema, schemaModule } = schemaMatch;
       NoteUtils.addSchema({ note, schema, schemaModule });
     }
-
     // if we added a new note and it overwrote an existing note
     // we now need to update the metadata of existing notes ^change
+    // TODO: Not sure the this.updateNote(ent) call is necessary, since it's already updated via _writeNewNote above.
     this.logger.info({
       ctx,
       msg: "pre:updateNotes",
     });
     await Promise.all(
-      [note].concat(changed).map((ent) => this.updateNote(ent))
+      [note]
+        .concat(changedEntries.map((entry) => entry.note))
+        .map((ent) => this.updateNote(ent))
     );
-    const changedEntries = changed.map((ent) => ({
-      note: ent,
-      status: "update" as const,
-    })) as NoteChangeEntry[];
+
     changedEntries.push({ note, status: "create" });
     if (maybeNote && !noDelete) {
       changedEntries.push({ note: maybeNote, status: "delete" });