Merged #657 Simplify state stuff

Author: Polleps

browser/lib/src/commit.ts

@@ -491,11 +491,7 @@ export function parseAndApplyCommit(jsonAdObjStr: string, store: Store) {
   } else {
     resource.appliedCommitSignatures.add(signature);
 
-    if (isNew) {
-      store.addResources(resource);
-    } else {
-      store.notify(resource.clone());
-    }
+    store.addResources(resource);
   }
 }
 

browser/lib/src/resource.ts

@@ -36,6 +36,8 @@ export const unknownSubject = 'unknown-subject';
  */
 // eslint-disable-next-line @typescript-eslint/no-explicit-any
 export class Resource<C extends OptionalClass = any> {
+  // WARNING: WHEN ADDING A PROPERTY, ALSO ADD IT TO THE CLONE METHOD
+
   /** If the resource could not be fetched, we put that info here. */
   public error?: Error;
   /** If the commit could not be saved, we put that info here. */
@@ -57,8 +59,6 @@ export class Resource<C extends OptionalClass = any> {
   private subject: string;
   private propvals: PropVals;
 
-  private queuedFetch: Promise<unknown> | undefined;
-
   public constructor(subject: string, newResource?: boolean) {
     if (typeof subject !== 'string') {
       throw new Error(
@@ -95,19 +95,39 @@ export class Resource<C extends OptionalClass = any> {
     return props;
   }
 
-  /** Checks if the content of two Resource instances is equal
-   * Warning: does not check CommitBuilder, loading state
-   */
-  public static compare(resourceA: Resource, resourceB: Resource): boolean {
-    if (resourceA.error !== resourceB.error) {
+  /** Checks if the content of two Resource instances is equal */
+  public equals(resourceB: Resource): boolean {
+    if (this.getSubject() !== resourceB.getSubject()) {
       return false;
     }
 
-    return (
-      resourceA.getSubject() === resourceB.getSubject() &&
-      JSON.stringify(Array.from(resourceA.propvals.entries())) ===
-        JSON.stringify(Array.from(resourceB.propvals.entries()))
-    );
+    if (this.new !== resourceB.new) {
+      return false;
+    }
+
+    if (this.error !== resourceB.error) {
+      return false;
+    }
+
+    if (this.loading !== resourceB.loading) {
+      return false;
+    }
+
+    if (
+      JSON.stringify(Array.from(this.propvals.entries())) ===
+      JSON.stringify(Array.from(resourceB.propvals.entries()))
+    ) {
+      return false;
+    }
+
+    if (
+      JSON.stringify(Array.from(this.commitBuilder.set.entries())) ===
+      JSON.stringify(Array.from(resourceB.commitBuilder.set.entries()))
+    ) {
+      return false;
+    }
+
+    return true;
   }
 
   /** Checks if the agent has write rights by traversing the graph. Recursive function. */
@@ -163,12 +183,10 @@ export class Resource<C extends OptionalClass = any> {
     res.propvals = structuredClone(this.propvals);
     res.loading = this.loading;
     res.new = this.new;
-    // structured clone
     res.error = structuredClone(this.error);
     res.commitError = this.commitError;
     res.commitBuilder = this.commitBuilder.clone();
     res.appliedCommitSignatures = this.appliedCommitSignatures;
-    res.queuedFetch = this.queuedFetch;
 
     return res as Resource<C>;
   }
@@ -477,29 +495,16 @@ export class Resource<C extends OptionalClass = any> {
     const endpoint = new URL(this.getSubject()).origin + `/commit`;
 
     try {
-      // We optimistically update all viewed instances for snappy feedback
-      store.addResources(this);
-      store.notify(this);
-
-      // If a commit is already being posted we wait for it to finish
-      // because the server can not guarantee the commits will be processed in the correct order.
-
-      if (this.queuedFetch) {
-        try {
-          await this.queuedFetch;
-        } catch (e) {
-          // Continue
-        }
-      }
-
       this.commitError = undefined;
-      const createdCommitPromise = store.postCommit(commit, endpoint);
-      this.queuedFetch = createdCommitPromise;
-      store.notify(this);
-      const createdCommit = await createdCommitPromise;
-
+      store.addResources(this);
+      const createdCommit = await store.postCommit(commit, endpoint);
+      // const res = store.getResourceLoading(this.subject);
       this.setUnsafe(properties.commit.lastCommit, createdCommit.id!);
 
+      // Let all subscribers know that the commit has been applied
+      // store.addResources(this);
+      store.notifyResourceSaved(this);
+
       if (wasNew) {
         // The first `SUBSCRIBE` message will not have worked, because the resource didn't exist yet.
         // That's why we need to repeat the process
@@ -510,9 +515,6 @@ export class Resource<C extends OptionalClass = any> {
         await store.saveBatchForParent(this.getSubject());
       }
 
-      // Let all subscribers know that the commit has been applied
-      store.notifyResourceSaved(this);
-
       return createdCommit.id as string;
     } catch (e) {
       // Logic for handling error if the previousCommit is wrong.
@@ -540,7 +542,6 @@ export class Resource<C extends OptionalClass = any> {
       this.commitBuilder = oldCommitBuilder;
       this.commitError = e;
       store.addResources(this);
-      store.notify(this.clone());
       throw e;
     }
   }
@@ -582,15 +583,13 @@ export class Resource<C extends OptionalClass = any> {
 
     if (value === undefined) {
       this.removePropVal(prop);
-      store.notify(this.clone());
 
       return;
     }
 
     this.propvals.set(prop, value);
     // Add the change to the Commit Builder, so we can commit our changes later
     this.commitBuilder.addSetAction(prop, value);
-    store.notify(this.clone());
   }
 
   /**

browser/lib/src/store.ts

@@ -155,14 +155,16 @@ export class Store {
     const storeResource = this.resources.get(resource.getSubject());
 
     if (storeResource) {
-      if (Resource.compare(storeResource, resource)) {
+      if (resource.equals(storeResource)) {
         return;
       }
     }
 
-    this.resources.set(resource.getSubject(), resource);
+    const cloned = resource.clone();
+
+    this.resources.set(cloned.getSubject(), cloned);
 
-    this.notify(resource.clone());
+    this.notify(cloned);
   }
 
   /** Checks if a subject is free to use */
@@ -471,23 +473,6 @@ export class Store {
     return !window?.navigator?.onLine;
   }
 
-  /** Let's subscribers know that a resource has been changed. Time to update your views!
-   * Note that when using this in React, we need the Resource to be Cloned in order to update.
-   */
-  public async notify(resource: Resource): Promise<void> {
-    const subject = resource.getSubject();
-    const callbacks = this.subscribers.get(subject);
-
-    if (callbacks === undefined) {
-      return;
-    }
-
-    // We clone for react, because otherwise it won't rerender
-    const cloned = resource.clone();
-    this._resources.set(subject, cloned);
-    Promise.allSettled(callbacks.map(async cb => cb(cloned)));
-  }
-
   public async notifyResourceSaved(resource: Resource): Promise<void> {
     await this.eventManager.emit(StoreEvents.ResourceSaved, resource);
   }
@@ -849,6 +834,21 @@ export class Store {
 
     return url;
   }
+
+  /** Lets subscribers know that a resource has been changed. Time to update your views.
+   * Make sure the resource is a new reference, otherwise React will not rerender.
+   */
+  private async notify(resource: Resource): Promise<void> {
+    const subject = resource.getSubject();
+    const callbacks = this.subscribers.get(subject);
+
+    if (callbacks === undefined) {
+      return;
+    }
+
+    // We clone for react, because otherwise it won't rerender
+    Promise.allSettled(callbacks.map(async cb => cb(resource)));
+  }
 }
 
 /**