refactor: use of lock file in ModuleGraph2 (#8087)

1 files changed, 22 insertions, 19 deletions

diff --git a/cli/module_graph2.rs b/cli/module_graph2.rs
index fd4d46461..ceb507b01 100644
--- a/cli/module_graph2.rs
+++ b/cli/module_graph2.rs
@@ -48,6 +48,7 @@ use std::fmt;
 use std::path::PathBuf;
 use std::rc::Rc;
 use std::result;
+use std::sync::Arc;
 use std::sync::Mutex;
 use std::time::Instant;
 
@@ -598,6 +599,8 @@ pub struct Graph2 {
   /// calls to a module graph where the emit is already valid do not cause the
   /// graph to re-emit.
   roots_dynamic: bool,
+  // A reference to lock file that will be used to check module integrity.
+  maybe_lockfile: Option<Arc<Mutex<Lockfile>>>,
 }
 
 impl Graph2 {
@@ -606,7 +609,10 @@ impl Graph2 {
   /// The argument `handler` is an instance of a structure that implements the
   /// `SpecifierHandler` trait.
   ///
-  pub fn new(handler: Rc<RefCell<dyn SpecifierHandler>>) -> Self {
+  pub fn new(
+    handler: Rc<RefCell<dyn SpecifierHandler>>,
+    maybe_lockfile: Option<Arc<Mutex<Lockfile>>>,
+  ) -> Self {
     Graph2 {
       handler,
       maybe_tsbuildinfo: None,
@@ -614,6 +620,7 @@ impl Graph2 {
       redirects: HashMap::new(),
       roots: Vec::new(),
       roots_dynamic: true,
+      maybe_lockfile,
     }
   }
 
@@ -1026,8 +1033,8 @@ impl Graph2 {
   /// Verify the subresource integrity of the graph based upon the optional
   /// lockfile, updating the lockfile with any missing resources.  This will
   /// error if any of the resources do not match their lock status.
-  pub fn lock(&self, maybe_lockfile: &Option<Mutex<Lockfile>>) {
-    if let Some(lf) = maybe_lockfile {
+  pub fn lock(&self) {
+    if let Some(lf) = self.maybe_lockfile.as_ref() {
       let mut lockfile = lf.lock().unwrap();
       for (ms, module) in self.modules.iter() {
         let specifier = module.specifier.to_string();
@@ -1259,6 +1266,7 @@ impl GraphBuilder2 {
   pub fn new(
     handler: Rc<RefCell<dyn SpecifierHandler>>,
     maybe_import_map: Option<ImportMap>,
+    maybe_lockfile: Option<Arc<Mutex<Lockfile>>>,
   ) -> Self {
     let internal_import_map = if let Some(import_map) = maybe_import_map {
       Some(Rc::new(RefCell::new(import_map)))
@@ -1266,7 +1274,7 @@ impl GraphBuilder2 {
       None
     };
     GraphBuilder2 {
-      graph: Graph2::new(handler),
+      graph: Graph2::new(handler, maybe_lockfile),
       fetched: HashSet::new(),
       maybe_import_map: internal_import_map,
       pending: FuturesUnordered::new(),
@@ -1393,13 +1401,8 @@ impl GraphBuilder2 {
   /// Move out the graph from the builder to be utilized further.  An optional
   /// lockfile can be provided, where if the sources in the graph do not match
   /// the expected lockfile, an error will be logged and the process will exit.
-  ///
-  /// TODO(@kitsonk) this should really be owned by the graph, but currently
-  /// the lockfile is behind a mutex in program_state, which makes it really
-  /// hard to not pass around as a reference, which if the Graph owned it, it
-  /// would need lifetime parameters and lifetime parameters are 😭
-  pub fn get_graph(self, maybe_lockfile: &Option<Mutex<Lockfile>>) -> Graph2 {
-    self.graph.lock(maybe_lockfile);
+  pub fn get_graph(self) -> Graph2 {
+    self.graph.lock();
     self.graph
   }
 }
@@ -1537,13 +1540,13 @@ pub mod tests {
       fixtures,
       ..MockSpecifierHandler::default()
     }));
-    let mut builder = GraphBuilder2::new(handler.clone(), None);
+    let mut builder = GraphBuilder2::new(handler.clone(), None, None);
     builder
       .add(&specifier, false)
       .await
       .expect("module not inserted");
 
-    (builder.get_graph(&None), handler)
+    (builder.get_graph(), handler)
   }
 
   #[test]
@@ -1645,12 +1648,12 @@ pub mod tests {
         fixtures: fixtures.clone(),
         ..MockSpecifierHandler::default()
       }));
-      let mut builder = GraphBuilder2::new(handler.clone(), None);
+      let mut builder = GraphBuilder2::new(handler.clone(), None, None);
       builder
         .add(&specifier, false)
         .await
         .expect("module not inserted");
-      let graph = builder.get_graph(&None);
+      let graph = builder.get_graph();
       let (actual, stats, maybe_ignored_options) = graph
         .bundle(BundleOptions::default())
         .expect("could not bundle");
@@ -1739,7 +1742,7 @@ pub mod tests {
       fixtures,
       ..MockSpecifierHandler::default()
     }));
-    let mut builder = GraphBuilder2::new(handler.clone(), None);
+    let mut builder = GraphBuilder2::new(handler.clone(), None, None);
     builder
       .add(&specifier, false)
       .await
@@ -1845,12 +1848,12 @@ pub mod tests {
     let lockfile_path = fixtures.join("lockfile.json");
     let lockfile =
       Lockfile::new(lockfile_path, false).expect("could not load lockfile");
-    let maybe_lockfile = Some(Mutex::new(lockfile));
+    let maybe_lockfile = Some(Arc::new(Mutex::new(lockfile)));
     let handler = Rc::new(RefCell::new(MockSpecifierHandler {
       fixtures,
       ..MockSpecifierHandler::default()
     }));
-    let mut builder = GraphBuilder2::new(handler.clone(), None);
+    let mut builder = GraphBuilder2::new(handler.clone(), None, maybe_lockfile);
     let specifier =
       ModuleSpecifier::resolve_url_or_path("file:///tests/main.ts")
         .expect("could not resolve module");
@@ -1858,6 +1861,6 @@ pub mod tests {
       .add(&specifier, false)
       .await
       .expect("module not inserted");
-    builder.get_graph(&maybe_lockfile);
+    builder.get_graph();
   }
 }