Cosmetic selector cleanup (#1721)

Summary: Pull Request resolved: #1721 Minor cosementic cleanup of the selector implementation to make names more accurate and the code easier to maintain. Reviewed By: noritheduck Differential Revision: D35493524 fbshipit-source-id: 4f937fa057b5ef2125400e1c8b636fc49302eb9f
facebookexperimental · Apr 13, 2022 · f61a371 · f61a371
1 parent 854e574
commit f61a371
Show file tree

Hide file tree

Showing 3 changed files with 148 additions and 204 deletions.
diff --git a/packages/recoil/core/Recoil_Graph.js b/packages/recoil/core/Recoil_Graph.js
@@ -11,15 +11,15 @@
 
 'use strict';
 
-import type {DependencyMap, Graph} from './Recoil_GraphTypes';
+import type {Graph} from './Recoil_GraphTypes';
 import type {NodeKey, StateID} from './Recoil_Keys';
 import type {Store} from './Recoil_State';
 
 const differenceSets = require('recoil-shared/util/Recoil_differenceSets');
 const mapMap = require('recoil-shared/util/Recoil_mapMap');
 const nullthrows = require('recoil-shared/util/Recoil_nullthrows');
 const recoverableViolation = require('recoil-shared/util/Recoil_recoverableViolation');
-export type {DependencyMap, Graph} from './Recoil_GraphTypes';
+export type {Graph} from './Recoil_GraphTypes';
 
 function makeGraph(): Graph {
   return {
@@ -40,61 +40,54 @@ function cloneGraph(graph: Graph): Graph {
 
 // Note that this overwrites the deps of existing nodes, rather than unioning
 // the new deps with the old deps.
-function mergeDependencyMapIntoGraph(
-  deps: DependencyMap,
+function mergeDepsIntoGraph(
+  key: NodeKey,
+  newDeps: $ReadOnlySet<NodeKey>,
   graph: Graph,
   // If olderGraph is given then we will not overwrite changes made to the given
   // graph compared with olderGraph:
   olderGraph?: Graph,
 ): void {
   const {nodeDeps, nodeToNodeSubscriptions} = graph;
+  const oldDeps = nodeDeps.get(key);
 
-  deps.forEach((upstreams, downstream) => {
-    const existingUpstreams = nodeDeps.get(downstream);
+  if (oldDeps && olderGraph && oldDeps !== olderGraph.nodeDeps.get(key)) {
+    return;
+  }
 
-    if (
-      existingUpstreams &&
-      olderGraph &&
-      existingUpstreams !== olderGraph.nodeDeps.get(downstream)
-    ) {
-      return;
-    }
+  // Update nodeDeps:
+  nodeDeps.set(key, newDeps);
 
-    // Update nodeDeps:
-    nodeDeps.set(downstream, new Set(upstreams));
+  // Add new deps to nodeToNodeSubscriptions:
+  const addedDeps =
+    oldDeps == null ? newDeps : differenceSets(newDeps, oldDeps);
+  for (const dep of addedDeps) {
+    if (!nodeToNodeSubscriptions.has(dep)) {
+      nodeToNodeSubscriptions.set(dep, new Set());
+    }
+    const existing = nullthrows(nodeToNodeSubscriptions.get(dep));
+    existing.add(key);
+  }
 
-    // Add new deps to nodeToNodeSubscriptions:
-    const addedUpstreams =
-      existingUpstreams == null
-        ? upstreams
-        : differenceSets(upstreams, existingUpstreams);
-    addedUpstreams.forEach(upstream => {
-      if (!nodeToNodeSubscriptions.has(upstream)) {
-        nodeToNodeSubscriptions.set(upstream, new Set());
+  // Remove removed deps from nodeToNodeSubscriptions:
+  if (oldDeps) {
+    const removedDeps = differenceSets(oldDeps, newDeps);
+    for (const dep of removedDeps) {
+      if (!nodeToNodeSubscriptions.has(dep)) {
+        return;
+      }
+      const existing = nullthrows(nodeToNodeSubscriptions.get(dep));
+      existing.delete(key);
+      if (existing.size === 0) {
+        nodeToNodeSubscriptions.delete(dep);
       }
-      const existing = nullthrows(nodeToNodeSubscriptions.get(upstream));
-      existing.add(downstream);
-    });
-
-    // Remove removed deps from nodeToNodeSubscriptions:
-    if (existingUpstreams) {
-      const removedUpstreams = differenceSets(existingUpstreams, upstreams);
-      removedUpstreams.forEach(upstream => {
-        if (!nodeToNodeSubscriptions.has(upstream)) {
-          return;
-        }
-        const existing = nullthrows(nodeToNodeSubscriptions.get(upstream));
-        existing.delete(downstream);
-        if (existing.size === 0) {
-          nodeToNodeSubscriptions.delete(upstream);
-        }
-      });
     }
-  });
+  }
 }
 
-function saveDependencyMapToStore(
-  dependencyMap: DependencyMap,
+function saveDepsToStore(
+  key: NodeKey,
+  deps: $ReadOnlySet<NodeKey>,
   store: Store,
   version: StateID,
 ): void {
@@ -115,13 +108,13 @@ function saveDependencyMapToStore(
   // Merge the dependencies discovered into the store's dependency map
   // for the version that was read:
   const graph = store.getGraph(version);
-  mergeDependencyMapIntoGraph(dependencyMap, graph);
+  mergeDepsIntoGraph(key, deps, graph);
 
   // If this version is not the latest version, also write these dependencies
   // into later versions if they don't already have their own:
   if (version === storeState.previousTree?.version) {
     const currentGraph = store.getGraph(storeState.currentTree.version);
-    mergeDependencyMapIntoGraph(dependencyMap, currentGraph, graph);
+    mergeDepsIntoGraph(key, deps, currentGraph, graph);
   }
   if (
     version === storeState.previousTree?.version ||
@@ -130,39 +123,13 @@ function saveDependencyMapToStore(
     const nextVersion = storeState.nextTree?.version;
     if (nextVersion !== undefined) {
       const nextGraph = store.getGraph(nextVersion);
-      mergeDependencyMapIntoGraph(dependencyMap, nextGraph, graph);
+      mergeDepsIntoGraph(key, deps, nextGraph, graph);
     }
   }
 }
 
-function mergeDepsIntoDependencyMap(
-  from: DependencyMap,
-  into: DependencyMap,
-): void {
-  from.forEach((upstreamDeps, downstreamNode) => {
-    if (!into.has(downstreamNode)) {
-      into.set(downstreamNode, new Set());
-    }
-    const deps = nullthrows(into.get(downstreamNode));
-    upstreamDeps.forEach(dep => deps.add(dep));
-  });
-}
-
-function addToDependencyMap(
-  downstream: NodeKey,
-  upstream: NodeKey,
-  dependencyMap: DependencyMap,
-): void {
-  if (!dependencyMap.has(downstream)) {
-    dependencyMap.set(downstream, new Set());
-  }
-  nullthrows(dependencyMap.get(downstream)).add(upstream);
-}
-
 module.exports = {
-  addToDependencyMap,
   cloneGraph,
   graph: makeGraph,
-  mergeDepsIntoDependencyMap,
-  saveDependencyMapToStore,
+  saveDepsToStore,
 };
diff --git a/packages/recoil/core/Recoil_GraphTypes.js b/packages/recoil/core/Recoil_GraphTypes.js
@@ -17,13 +17,11 @@ export type Graph = $ReadOnly<{
   // TODO rename these properties to be more descriptive and symetric.
   // Upstream Node dependencies
   // NOTE: if you ever make the sets in nodeDeps mutable you must change the
-  // logic in mergeDependencyMapIntoGraph that relies on reference equality
+  // logic in mergeDepsIntoGraph() that relies on reference equality
   // of these sets in avoiding overwriting newer deps with older ones.
   nodeDeps: Map<NodeKey, $ReadOnlySet<NodeKey>>,
   // Downstream Node subscriptions
   nodeToNodeSubscriptions: Map<NodeKey, Set<NodeKey>>,
 }>;
 
-export type DependencyMap = Map<NodeKey, Set<NodeKey>>;
-
 module.exports = ({}: {...});