Use async.fast as a shortcut. Keep slow async around until we have tests. more precise pull block collision detection

Author: maparent

apps/roam/src/utils/fireQuery.ts

@@ -316,7 +316,8 @@ export const fireQuerySync = (args: FireQueryArgs): QueryResult[] => {
   }));
 };
 
-const PROP_NAME_RE = /:\w+\/\w+\b/g;
+const PROP_NAME_RE = /:[a-zA-Z0-9_-]+\/[a-zA-Z0-9_-]+\b/g;
+const PULL_RE = /\(pull [^)]+\)/g;
 
 const renamePropsInResult = (
   result: json | null,
@@ -372,25 +373,44 @@ const fireQuery: FireQuery = async (_args) => {
     }
 
     let queryResults: unknown[][] = [];
+    // for now assume fast results are equivalent to slow.
+    // We have seen some errors in (fast) backend queries, but not so far in local fast ones.
+    const preferSlow = false;
     if (local) {
-      // look for propNames in query. Could consider looking only in pull when that exists.
-      const propNames = new Set(
-        [...query.matchAll(PROP_NAME_RE)].map((m) => m[0]),
-      );
-      const propNamesSub: Record<string, string> = Object.fromEntries(
-        [...propNames].map((n) => [n.split("/")[1], n]),
-      );
-      if (Object.keys(propNamesSub).length === propNames.size) {
+      let useSlow = preferSlow;
+      let propNamesSub: Record<string, string> | undefined;
+      if (preferSlow) {
+        // keeping this code in case it turns out the fast results are more fragile than the slow ones
+        // TODO: Remove when we have a more comprehensive test suite.
+        const pulls = [...query.matchAll(PULL_RE)].map((r) => r[0]);
+        if (pulls.length > 0) {
+          // pull in base async query (vs fast or backend) returns non-namespaced names,
+          // so there are a few possibilities of conflict,
+          // namely: {log,version,window}/id; {graph,user}/settings; {block,user}/uid; {create,edit}/time
+          // So look for collisions in property names in pull part of query.
+          const propNames = new Set(
+            [...pulls.join(" ").matchAll(PROP_NAME_RE)].map((m) => m[0]),
+          );
+          propNamesSub = Object.fromEntries(
+            [...propNames].map((n) => [n.split("/")[1], n]),
+          );
+          useSlow = Object.keys(propNamesSub).length === propNames.size;
+        }
+      }
+      if (useSlow) {
         // no name conflict, safe to use async query
         // BUT it returns non-namespaced names, so substitute prop names back
         queryResults = await window.roamAlphaAPI.data.async.q(query, ...inputs);
-        queryResults = renamePropsInResult(
-          queryResults as json,
-          propNamesSub,
-        ) as unknown[][];
+        if (propNamesSub !== undefined)
+          queryResults = renamePropsInResult(
+            queryResults as json,
+            propNamesSub,
+          ) as unknown[][];
       } else {
-        // more janky but safer
-        queryResults = window.roamAlphaAPI.data.fast.q(query, ...inputs);
+        queryResults = await window.roamAlphaAPI.data.async.fast.q(
+          query,
+          ...inputs,
+        );
       }
     } else {
       // eslint-disable-next-line @typescript-eslint/await-thenable