fix(vue-model-api): do not evict ReactiveINodeJS from cache when objects manged by it are still used

The effect of this issue was that subscribed Vue elements didn't update when a change happened in Modelix data.

Switching to `shallowRef` fixes the issue, because the created `shallowRef`s have a reference to the `ReactiveINodeJS` instances.

Implementing and using the `set` functions of `customRef`s would have had the same effect. But the implementation with `shallowRef`s is cleaner, because we also do not have to save the `trigger` functions in `ReactiveINodeJS`.

See also https://vuejs.org/guide/extras/reactivity-in-depth#integration-with-external-state-systems

Fixes: MODELIX-1041

Author: Oleksandr Dzhychko

vue-model-api/src/internal/ReactiveINodeJS.ts

@@ -1,6 +1,8 @@
 import type { IConceptJS, INodeJS } from "@modelix/ts-model-api";
-import { customRef, markRaw } from "vue";
+import type { Ref } from "vue";
+import { markRaw, shallowRef } from "vue";
 import type { Cache } from "./Cache";
+import type { Nullable } from "@modelix/model-client";
 
 export function toReactiveINodeJS(
   node: INodeJS,
@@ -37,15 +39,19 @@ function unwrapReactiveINodeJS(
 // eslint-disable-next-line @typescript-eslint/no-explicit-any
 type INodeReferenceJS = any;
 
-interface TrackAndTrigger {
-  track: () => void;
-  trigger: () => void;
-}
-
 export class ReactiveINodeJS implements INodeJS {
-  private byRoleTrackAndTrigger: Map<string | undefined, TrackAndTrigger> =
+  private byRoleRefToProperty: Map<string, Ref<string | undefined>> = new Map();
+  private byRoleRefToReferenceTargetNode: Map<
+    string,
+    Ref<INodeJS | undefined>
+  > = new Map();
+  private byRoleRefToReferenceTargetRef: Map<
+    string,
+    Ref<INodeReferenceJS | undefined>
+  > = new Map();
+  private byRoleRefToChildren: Map<string | undefined, Ref<INodeJS[]>> =
     new Map();
-  private trackAndTriggerForAllChildren: TrackAndTrigger | undefined;
+  private refForAllChildren: Ref<INodeJS[]> | undefined = undefined;
 
   constructor(
     public readonly unreactiveNode: INodeJS,
@@ -56,6 +62,30 @@ export class ReactiveINodeJS implements INodeJS {
     markRaw(this);
   }
 
+  private propertyGetter = (role: string) =>
+    this.unreactiveNode.getPropertyValue(role);
+
+  private referenceTargetRefGetter = (role: string) =>
+    this.unreactiveNode.getReferenceTargetRef(role);
+
+  private referenceTargetNodeGetter = (role: string) => {
+    const unreactiveTargetNode =
+      this.unreactiveNode.getReferenceTargetNode(role);
+    return unreactiveTargetNode
+      ? toReactiveINodeJS(unreactiveTargetNode, this.cache)
+      : unreactiveTargetNode;
+  };
+
+  private childrenGetter = (role: string | undefined) =>
+    this.unreactiveNode
+      .getChildren(role)
+      .map((node) => toReactiveINodeJS(node, this.cache));
+
+  private allChildrenGetter = () =>
+    this.unreactiveNode
+      .getAllChildren()
+      .map((node) => toReactiveINodeJS(node, this.cache));
+
   getConcept(): IConceptJS | undefined {
     return this.unreactiveNode.getConcept();
   }
@@ -89,19 +119,19 @@ export class ReactiveINodeJS implements INodeJS {
   }
 
   getChildren(role: string | undefined): INodeJS[] {
-    const { track } = this.getOrCreateTrackAndTriggerForRole(role);
-    track();
-    return this.unreactiveNode
-      .getChildren(role)
-      .map((node) => toReactiveINodeJS(node, this.cache));
+    const ref = this.getOrCreateRefForRole(
+      this.byRoleRefToChildren,
+      role,
+      this.childrenGetter,
+    );
+    return ref.value;
   }
 
   getAllChildren(): INodeJS[] {
-    const { track } = this.getOrCreateTrackAndTriggerForAllChildren();
-    track();
-    return this.unreactiveNode
-      .getAllChildren()
-      .map((node) => toReactiveINodeJS(node, this.cache));
+    if (this.refForAllChildren == undefined) {
+      this.refForAllChildren = shallowRef(this.allChildrenGetter());
+    }
+    return this.refForAllChildren!.value;
   }
 
   moveChild(role: string | undefined, index: number, child: INodeJS): void {
@@ -134,18 +164,21 @@ export class ReactiveINodeJS implements INodeJS {
   }
 
   getReferenceTargetNode(role: string): INodeJS | undefined {
-    const { track } = this.getOrCreateTrackAndTriggerForRole(role);
-    track();
-    const unreacitveNode = this.unreactiveNode.getReferenceTargetNode(role);
-    return unreacitveNode
-      ? toReactiveINodeJS(unreacitveNode, this.cache)
-      : unreacitveNode;
+    const ref = this.getOrCreateRefForRole(
+      this.byRoleRefToReferenceTargetNode,
+      role,
+      this.referenceTargetNodeGetter,
+    );
+    return ref.value;
   }
 
   getReferenceTargetRef(role: string): INodeReferenceJS | undefined {
-    const { track } = this.getOrCreateTrackAndTriggerForRole(role);
-    track();
-    return this.unreactiveNode.getReferenceTargetRef(role);
+    const ref = this.getOrCreateRefForRole(
+      this.byRoleRefToReferenceTargetRef,
+      role,
+      this.referenceTargetRefGetter,
+    );
+    return ref.value;
   }
 
   setReferenceTargetNode(role: string, target: INodeJS | undefined): void {
@@ -173,69 +206,57 @@ export class ReactiveINodeJS implements INodeJS {
   }
 
   getPropertyValue(role: string): string | undefined {
-    const { track } = this.getOrCreateTrackAndTriggerForRole(role);
-    track();
-    return this.unreactiveNode.getPropertyValue(role);
+    const ref = this.getOrCreateRefForRole(this.byRoleRefToProperty, role, this.propertyGetter);
+    return ref.value;
   }
 
   setPropertyValue(role: string, value: string | undefined): void {
     this.unreactiveNode.setPropertyValue(role, value);
   }
 
-  private getOrCreateTrackAndTriggerForAllChildren(): TrackAndTrigger {
-    if (this.trackAndTriggerForAllChildren === undefined) {
-      customRef((track, trigger) => {
-        this.trackAndTriggerForAllChildren = {
-          track,
-          trigger,
-        };
-        return {
-          // Getter and setter ar empty for the same reason as in `getOrCreateTrackAndTriggerForRole`
-          get() {},
-          set() {},
-        };
-      });
+  getOrCreateRefForRole<RoleT, ValueT>(
+    byRoleRefs: Map<RoleT, Ref<ValueT>>,
+    role: RoleT,
+    getValue: (role: RoleT) => ValueT,
+  ): Ref<ValueT> {
+    const maybeCreatedShallowRef = byRoleRefs.get(role);
+
+    if (maybeCreatedShallowRef != undefined) {
+      return maybeCreatedShallowRef;
+    } else {
+      const newRef = shallowRef(getValue(role));
+      byRoleRefs.set(role, newRef);
+      return newRef;
     }
-    // `this.trackAndTriggerForAllChildren` will always be set, because the `factory`
-    // argument of `customRef` will be evaluated immedialty.
-    return this.trackAndTriggerForAllChildren!;
   }
 
-  private getOrCreateTrackAndTriggerForRole(
-    role: string | undefined,
-  ): TrackAndTrigger {
-    const existing = this.byRoleTrackAndTrigger.get(role);
-    if (existing !== undefined) {
-      return existing;
+  triggerChangeInChild(role: Nullable<string>) {
+    if (this.refForAllChildren) {
+      this.refForAllChildren.value = this.allChildrenGetter();
+    }
+
+    const normalizedRole = role ?? undefined;
+    const maybeRef = this.byRoleRefToChildren.get(normalizedRole);
+    if (maybeRef) {
+      maybeRef.value = this.childrenGetter(normalizedRole);
+    }
+  }
+
+  triggerChangeInReference(role: string) {
+    const maybeTargetNodeRef = this.byRoleRefToReferenceTargetNode.get(role);
+    if (maybeTargetNodeRef) {
+      maybeTargetNodeRef.value = this.referenceTargetNodeGetter(role);
+    }
+    const maybeTargetRefRef = this.byRoleRefToReferenceTargetRef.get(role);
+    if (maybeTargetRefRef) {
+      maybeTargetRefRef.value = this.referenceTargetRefGetter(role);
+    }
+  }
+
+  triggerChangeInProperty(role: string) {
+    const maybeRef = this.byRoleRefToProperty.get(role);
+    if (maybeRef) {
+      maybeRef.value = this.propertyGetter(role);
     }
-    let created;
-    customRef((track, trigger) => {
-      created = {
-        track,
-        trigger,
-      };
-      this.byRoleTrackAndTrigger.set(role, created);
-      return {
-        // The getters and setters will never be called directly
-        // and therefore the are empty.
-        // We use `customRef` to get access to a pair of `trigger` and `track`
-        // to call them directly from outside.
-        get() {},
-        set() {},
-      };
-    });
-    // `created` will always be set, because the `factory`
-    // argument of `customRef` will be evaluated immedialty.
-    return created!;
-  }
-
-  triggerChangeInRole(role: string | undefined | null) {
-    const normalizedRole =
-      role !== undefined && role !== null ? role : undefined;
-    this.byRoleTrackAndTrigger.get(normalizedRole)?.trigger();
-  }
-
-  triggerChangeInAllChildren() {
-    this.trackAndTriggerForAllChildren?.trigger();
   }
 }

vue-model-api/src/internal/handleChange.ts

@@ -13,10 +13,11 @@ export function handleChange(change: ChangeJS, cache: Cache<ReactiveINodeJS>) {
   if (reacitveNode === undefined) {
     return;
   }
-  if (change instanceof PropertyChanged || change instanceof ReferenceChanged) {
-    reacitveNode.triggerChangeInRole(change.role);
+  if (change instanceof PropertyChanged) {
+    reacitveNode.triggerChangeInProperty(change.role);
+  } else if (change instanceof ReferenceChanged) {
+    reacitveNode.triggerChangeInReference(change.role);
   } else if (change instanceof ChildrenChanged) {
-    reacitveNode.triggerChangeInRole(change.role);
-    reacitveNode.triggerChangeInAllChildren();
+    reacitveNode.triggerChangeInChild(change.role);
   }
 }