Merge branch 'main' into alsh/asset-graph-tidy

Author: alshdavid

crates/atlaspack/src/atlaspack.rs

@@ -2,7 +2,7 @@ use std::path::PathBuf;
 use std::sync::Arc;
 
 use atlaspack_config::atlaspack_rc_config_loader::{AtlaspackRcConfigLoader, LoadConfigOptions};
-use atlaspack_core::asset_graph::{AssetGraph, AssetGraphNode, AssetNode};
+use atlaspack_core::asset_graph::{AssetGraph, AssetNode};
 use atlaspack_core::config_loader::ConfigLoader;
 use atlaspack_core::plugin::{PluginContext, PluginLogger, PluginOptions};
 use atlaspack_core::types::AtlaspackOptions;
@@ -128,7 +128,7 @@ impl Atlaspack {
 
       let asset_graph = match request_result {
         RequestResult::AssetGraph(result) => {
-          self.commit_assets(result.graph.nodes().collect())?;
+          self.commit_assets(result.graph.get_asset_nodes())?;
 
           result.graph
         }
@@ -139,19 +139,20 @@ impl Atlaspack {
     })
   }
 
-  fn commit_assets(&self, assets: Vec<&AssetGraphNode>) -> anyhow::Result<()> {
+  fn commit_assets(&self, assets: Vec<&AssetNode>) -> anyhow::Result<()> {
     let mut txn = self.db.environment().write_txn()?;
 
-    for asset_node in assets.iter() {
-      let AssetGraphNode::Asset(AssetNode { asset, .. }) = asset_node else {
-        continue;
-      };
-      self.db.put(&mut txn, &asset.id, asset.code.bytes())?;
-      if let Some(map) = &asset.map {
+    for asset_node in assets {
+      self.db.put(
+        &mut txn,
+        &asset_node.asset.id,
+        asset_node.asset.code.bytes(),
+      )?;
+      if let Some(map) = &asset_node.asset.map {
         // TODO: For some reason to_buffer strips data when rkyv was upgraded, so now we use json
         self.db.put(
           &mut txn,
-          &format!("map:{}", asset.id),
+          &format!("map:{}", asset_node.asset.id),
           map.to_json()?.as_bytes(),
         )?;
       }
@@ -192,17 +193,15 @@ mod tests {
     let nodes = assets
       .iter()
       .enumerate()
-      .map(|(idx, asset)| {
-        AssetGraphNode::Asset(AssetNode {
-          asset: Asset {
-            id: idx.to_string(),
-            code: Code::from(asset.to_string()),
-            ..Asset::default()
-          },
-          requested_symbols: HashSet::new(),
-        })
+      .map(|(idx, asset)| AssetNode {
+        asset: Asset {
+          id: idx.to_string(),
+          code: Code::from(asset.to_string()),
+          ..Asset::default()
+        },
+        requested_symbols: HashSet::new(),
       })
-      .collect::<Vec<AssetGraphNode>>();
+      .collect::<Vec<AssetNode>>();
 
     atlaspack.commit_assets(nodes.iter().collect())?;
 

crates/atlaspack/src/requests/asset_graph_request.rs

@@ -46,14 +46,14 @@ impl Request for AssetGraphRequest {
 }
 
 struct AssetGraphBuilder {
-  request_id_to_dep_node_index: HashMap<u64, NodeIndex>,
   graph: AssetGraph,
   visited: HashSet<u64>,
   work_count: u32,
   request_context: RunRequestContext,
   sender: ResultSender,
   receiver: ResultReceiver,
-  asset_request_to_asset: HashMap<u64, NodeIndex>,
+  request_id_to_asset_nx: HashMap<u64, NodeIndex>,
+  request_id_to_dependency_nx: HashMap<u64, NodeIndex>,
   waiting_asset_requests: HashMap<u64, HashSet<NodeIndex>>,
 }
 
@@ -62,14 +62,14 @@ impl AssetGraphBuilder {
     let (sender, receiver) = channel();
 
     AssetGraphBuilder {
-      request_id_to_dep_node_index: HashMap::new(),
+      request_id_to_dependency_nx: HashMap::new(),
       graph: AssetGraph::new(),
       visited: HashSet::new(),
       work_count: 0,
       request_context,
       sender,
       receiver,
-      asset_request_to_asset: HashMap::new(),
+      request_id_to_asset_nx: HashMap::new(),
       waiting_asset_requests: HashMap::new(),
     }
   }
@@ -138,16 +138,16 @@ impl AssetGraphBuilder {
   }
 
   fn handle_path_result(&mut self, result: PathRequestOutput, request_id: u64) {
-    let node = *self
-      .request_id_to_dep_node_index
+    let dependency_nx = *self
+      .request_id_to_dependency_nx
       .get(&request_id)
       .expect("Missing node index for request id {request_id}");
 
     let DependencyNode {
       dependency,
       requested_symbols,
       state,
-    } = &mut self.graph.get_dependency_node_mut(node).unwrap();
+    } = &mut self.graph.get_dependency_node_mut(&dependency_nx).unwrap();
 
     let asset_request = match result {
       PathRequestOutput::Resolved {
@@ -186,16 +186,16 @@ impl AssetGraphBuilder {
     let id = asset_request.id();
 
     if self.visited.insert(id) {
-      self.request_id_to_dep_node_index.insert(id, node);
+      self.request_id_to_dependency_nx.insert(id, dependency_nx);
       self.work_count += 1;
       let _ = self
         .request_context
         .queue_request(asset_request, self.sender.clone());
-    } else if let Some(asset_node_index) = self.asset_request_to_asset.get(&id) {
+    } else if let Some(asset_node_index) = self.request_id_to_asset_nx.get(&id) {
       // We have already completed this AssetRequest so we can connect the
       // Dependency to the Asset immediately
-      self.graph.add_edge(&node, asset_node_index);
-      self.propagate_requested_symbols(*asset_node_index, node);
+      self.graph.add_edge(&dependency_nx, asset_node_index);
+      self.propagate_requested_symbols(*asset_node_index, dependency_nx);
     } else {
       // The AssetRequest has already been kicked off but is yet to
       // complete. Register this Dependency to be connected once it
@@ -204,9 +204,9 @@ impl AssetGraphBuilder {
         .waiting_asset_requests
         .entry(id)
         .and_modify(|nodes| {
-          nodes.insert(node);
+          nodes.insert(dependency_nx);
         })
-        .or_insert_with(|| HashSet::from([node]));
+        .or_insert_with(|| HashSet::from([dependency_nx]));
     }
   }
 
@@ -233,23 +233,19 @@ impl AssetGraphBuilder {
       discovered_assets,
       dependencies,
     } = result;
-    let incoming_dep_node_index = *self
-      .request_id_to_dep_node_index
+    let incoming_dependency_nx = *self
+      .request_id_to_dependency_nx
       .get(&request_id)
       .expect("Missing node index for request id {request_id}");
 
     // Connect the incoming DependencyNode to the new AssetNode
-    let asset_node_index = self.graph.add_asset(asset.clone());
+    let asset_nx = self.graph.add_asset(asset.clone());
 
-    self
-      .graph
-      .add_edge(&incoming_dep_node_index, &asset_node_index);
+    self.graph.add_edge(&incoming_dependency_nx, &asset_nx);
 
-    self
-      .asset_request_to_asset
-      .insert(request_id, asset_node_index);
+    self.request_id_to_asset_nx.insert(request_id, asset_nx);
 
-    let root_asset = (&asset, asset_node_index);
+    let root_asset = (&asset, asset_nx);
     let mut added_discovered_assets: HashMap<String, NodeIndex> = HashMap::new();
 
     // Attach the "direct" discovered assets to the graph
@@ -259,7 +255,7 @@ impl AssetGraphBuilder {
 
       self
         .graph
-        .add_edge(&incoming_dep_node_index, &asset_node_index);
+        .add_edge(&incoming_dependency_nx, &asset_node_index);
 
       self.add_asset_dependencies(
         &discovered_asset.dependencies,
@@ -268,25 +264,25 @@ impl AssetGraphBuilder {
         &mut added_discovered_assets,
         root_asset,
       );
-      self.propagate_requested_symbols(asset_node_index, incoming_dep_node_index);
+      self.propagate_requested_symbols(asset_node_index, incoming_dependency_nx);
     }
 
     self.add_asset_dependencies(
       &dependencies,
       &discovered_assets,
-      asset_node_index,
+      asset_nx,
       &mut added_discovered_assets,
       root_asset,
     );
 
-    self.propagate_requested_symbols(asset_node_index, incoming_dep_node_index);
+    self.propagate_requested_symbols(asset_nx, incoming_dependency_nx);
 
     // Connect any previously discovered Dependencies that were waiting
     // for this AssetNode to be created
     if let Some(waiting) = self.waiting_asset_requests.remove(&request_id) {
       for dep in waiting {
-        self.graph.add_edge(&dep, &asset_node_index);
-        self.propagate_requested_symbols(asset_node_index, dep);
+        self.graph.add_edge(&dep, &asset_nx);
+        self.propagate_requested_symbols(asset_nx, dep);
       }
     }
   }
@@ -393,7 +389,7 @@ impl AssetGraphBuilder {
         .unwrap()
     {
       Self::on_undeferred(
-        &mut self.request_id_to_dep_node_index,
+        &mut self.request_id_to_dependency_nx,
         &mut self.work_count,
         &mut self.request_context,
         &self.sender,
@@ -417,7 +413,7 @@ impl AssetGraphBuilder {
         dependency: Arc::new(dependency),
       };
       self
-        .request_id_to_dep_node_index
+        .request_id_to_dependency_nx
         .insert(request.id(), dep_node);
       self.work_count += 1;
       let _ = self
@@ -432,7 +428,7 @@ impl AssetGraphBuilder {
   /// Once they do have symbols in use, this callback will re-trigger resolution/transformation
   /// for those files.
   fn on_undeferred(
-    request_id_to_dep_node_index: &mut HashMap<u64, NodeIndex>,
+    request_id_to_asset_nx: &mut HashMap<u64, NodeIndex>,
     work_count: &mut u32,
     request_context: &mut RunRequestContext,
     sender: &ResultSender,
@@ -443,7 +439,7 @@ impl AssetGraphBuilder {
       dependency: dependency.clone(),
     };
 
-    request_id_to_dep_node_index.insert(request.id(), dependency_node_index);
+    request_id_to_asset_nx.insert(request.id(), dependency_node_index);
     tracing::debug!(
       "queueing a path request from on_undeferred, {}",
       dependency.specifier
@@ -501,8 +497,7 @@ mod tests {
   use std::path::{Path, PathBuf};
   use std::sync::Arc;
 
-  use atlaspack_core::asset_graph::{AssetGraphNode, AssetNode};
-  use atlaspack_core::types::{AtlaspackOptions, Code};
+  use atlaspack_core::types::*;
   use atlaspack_filesystem::in_memory_file_system::InMemoryFileSystem;
   use atlaspack_filesystem::FileSystem;
 
@@ -588,34 +583,14 @@ mod tests {
       1
     );
 
-    let file_path = asset_graph_request_result
-      .graph
-      .nodes_from(&asset_graph_request_result.graph.root_node())
-      .iter()
-      .filter_map(|n| match *n {
-        AssetGraphNode::Asset(asset_node) => Some(asset_node),
-        _ => None,
-      })
-      .collect::<Vec<&AssetNode>>()
-      .get(0)
-      .unwrap()
-      .asset
-      .file_path
-      .clone();
-
-    let code = asset_graph_request_result
+    let Asset {
+      file_path, code, ..
+    }: Asset = asset_graph_request_result
       .graph
-      .nodes_from(&asset_graph_request_result.graph.root_node())
-      .iter()
-      .filter_map(|n| match *n {
-        AssetGraphNode::Asset(asset_node) => Some(asset_node),
-        _ => None,
-      })
-      .collect::<Vec<&AssetNode>>()
+      .get_asset_nodes()
       .get(0)
       .unwrap()
       .asset
-      .code
       .clone();
 
     assert_eq!(file_path, temporary_dir.join("entry.js"));
@@ -713,19 +688,12 @@ mod tests {
       5
     );
 
-    let file_path = asset_graph_request_result
+    let Asset { file_path, .. }: Asset = asset_graph_request_result
       .graph
-      .nodes_from(&asset_graph_request_result.graph.root_node())
-      .iter()
-      .filter_map(|n| match *n {
-        AssetGraphNode::Asset(asset_node) => Some(asset_node),
-        _ => None,
-      })
-      .collect::<Vec<&AssetNode>>()
+      .get_asset_nodes()
       .get(0)
       .unwrap()
       .asset
-      .file_path
       .clone();
 
     assert_eq!(file_path, temporary_dir.join("entry.js"));