fix: check cached versions during transpile (#7760)

Fixes: #7759

cli/graph.rs

@@ -16,6 +16,7 @@ use crate::specifier_handler::FetchFuture;
 use crate::specifier_handler::SpecifierHandler;
 use crate::tsc_config::IgnoredCompilerOptions;
 use crate::tsc_config::TsConfig;
+use crate::version;
 use crate::AnyError;
 
 use deno_core::futures::stream::FuturesUnordered;
@@ -163,6 +164,17 @@ fn parse_deno_types(comment: &str) -> Option<String> {
   }
 }
 
+/// A hashing function that takes the source code, version and optionally a
+/// user provided config and generates a string hash which can be stored to
+/// determine if the cached emit is valid or not.
+fn get_version(source: &TextDocument, version: &str, config: &[u8]) -> String {
+  crate::checksum::gen(&[
+    source.to_str().unwrap().as_bytes(),
+    version.as_bytes(),
+    config,
+  ])
+}
+
 /// A logical representation of a module within a graph.
 #[derive(Debug, Clone)]
 struct Module {
@@ -174,6 +186,7 @@ struct Module {
   maybe_import_map: Option<Rc<RefCell<ImportMap>>>,
   maybe_parsed_module: Option<ParsedModule>,
   maybe_types: Option<(String, ModuleSpecifier)>,
+  maybe_version: Option<String>,
   media_type: MediaType,
   specifier: ModuleSpecifier,
   source: TextDocument,
@@ -190,6 +203,7 @@ impl Default for Module {
       maybe_import_map: None,
       maybe_parsed_module: None,
       maybe_types: None,
+      maybe_version: None,
       media_type: MediaType::Unknown,
       specifier: ModuleSpecifier::resolve_url("https://deno.land/x/").unwrap(),
       source: TextDocument::new(Vec::new(), Option::<&str>::None),
@@ -209,6 +223,16 @@ impl Module {
     }
   }
 
+  /// Return `true` if the current hash of the module matches the stored
+  /// version.
+  pub fn emit_valid(&self, config: &[u8]) -> bool {
+    if let Some(version) = self.maybe_version.clone() {
+      version == get_version(&self.source, version::DENO, config)
+    } else {
+      false
+    }
+  }
+
   pub fn hydrate(&mut self, cached_module: CachedModule) {
     self.media_type = cached_module.media_type;
     self.source = cached_module.source;
@@ -230,6 +254,7 @@ impl Module {
     };
     self.is_dirty = false;
     self.emits = cached_module.emits;
+    self.maybe_version = cached_module.maybe_version;
     self.is_hydrated = true;
   }
 
@@ -351,6 +376,11 @@ impl Module {
 
     Ok(specifier)
   }
+
+  /// Calculate the hashed version of the module and update the `maybe_version`.
+  pub fn set_version(&mut self, config: &[u8]) {
+    self.maybe_version = Some(get_version(&self.source, version::DENO, config))
+  }
 }
 
 #[derive(Clone, Debug, PartialEq)]
@@ -428,6 +458,9 @@ impl Graph {
           maybe_map.clone(),
         )?;
         module.is_dirty = false;
+        if let Some(version) = &module.maybe_version {
+          handler.set_version(&module.specifier, version.clone())?;
+        }
       }
     }
     for root_specifier in self.roots.iter() {
@@ -508,14 +541,14 @@ impl Graph {
 
     let mut emit_count: u128 = 0;
     for (_, module) in self.modules.iter_mut() {
+      // TODO(kitsonk) a lot of this logic should be refactored into `Module` as
+      // we start to support other methods on the graph.  Especially managing
+      // the dirty state is something the module itself should "own".
+
       // if the module is a Dts file we should skip it
       if module.media_type == MediaType::Dts {
         continue;
       }
-      // skip modules that already have a valid emit
-      if module.emits.contains_key(&emit_type) {
-        continue;
-      }
       // if we don't have check_js enabled, we won't touch non TypeScript
       // modules
       if !(check_js
@@ -524,6 +557,11 @@ impl Graph {
       {
         continue;
       }
+      let config = ts_config.as_bytes();
+      // skip modules that already have a valid emit
+      if module.emits.contains_key(&emit_type) && module.emit_valid(&config) {
+        continue;
+      }
       if module.maybe_parsed_module.is_none() {
         module.parse()?;
       }
@@ -531,6 +569,7 @@ impl Graph {
       let emit = parsed_module.transpile(&emit_options)?;
       emit_count += 1;
       module.emits.insert(emit_type.clone(), emit);
+      module.set_version(&config);
       module.is_dirty = true;
     }
     self.flush(&emit_type)?;
@@ -736,6 +775,86 @@ mod tests {
   use std::path::PathBuf;
   use std::sync::Mutex;
 
+  #[test]
+  fn test_get_version() {
+    let doc_a =
+      TextDocument::new(b"console.log(42);".to_vec(), Option::<&str>::None);
+    let version_a = get_version(&doc_a, "1.2.3", b"");
+    let doc_b =
+      TextDocument::new(b"console.log(42);".to_vec(), Option::<&str>::None);
+    let version_b = get_version(&doc_b, "1.2.3", b"");
+    assert_eq!(version_a, version_b);
+
+    let version_c = get_version(&doc_a, "1.2.3", b"options");
+    assert_ne!(version_a, version_c);
+
+    let version_d = get_version(&doc_b, "1.2.3", b"options");
+    assert_eq!(version_c, version_d);
+
+    let version_e = get_version(&doc_a, "1.2.4", b"");
+    assert_ne!(version_a, version_e);
+
+    let version_f = get_version(&doc_b, "1.2.4", b"");
+    assert_eq!(version_e, version_f);
+  }
+
+  #[test]
+  fn test_module_emit_valid() {
+    let source =
+      TextDocument::new(b"console.log(42);".to_vec(), Option::<&str>::None);
+    let maybe_version = Some(get_version(&source, version::DENO, b""));
+    let module = Module {
+      source,
+      maybe_version,
+      ..Module::default()
+    };
+    assert!(module.emit_valid(b""));
+
+    let source =
+      TextDocument::new(b"console.log(42);".to_vec(), Option::<&str>::None);
+    let old_source =
+      TextDocument::new(b"console.log(43);".to_vec(), Option::<&str>::None);
+    let maybe_version = Some(get_version(&old_source, version::DENO, b""));
+    let module = Module {
+      source,
+      maybe_version,
+      ..Module::default()
+    };
+    assert!(!module.emit_valid(b""));
+
+    let source =
+      TextDocument::new(b"console.log(42);".to_vec(), Option::<&str>::None);
+    let maybe_version = Some(get_version(&source, "0.0.0", b""));
+    let module = Module {
+      source,
+      maybe_version,
+      ..Module::default()
+    };
+    assert!(!module.emit_valid(b""));
+
+    let source =
+      TextDocument::new(b"console.log(42);".to_vec(), Option::<&str>::None);
+    let module = Module {
+      source,
+      ..Module::default()
+    };
+    assert!(!module.emit_valid(b""));
+  }
+
+  #[test]
+  fn test_module_set_version() {
+    let source =
+      TextDocument::new(b"console.log(42);".to_vec(), Option::<&str>::None);
+    let expected = Some(get_version(&source, version::DENO, b""));
+    let mut module = Module {
+      source,
+      ..Module::default()
+    };
+    assert!(module.maybe_version.is_none());
+    module.set_version(b"");
+    assert_eq!(module.maybe_version, expected);
+  }
+
   #[tokio::test]
   async fn test_graph_builder() {
     let c = PathBuf::from(env::var_os("CARGO_MANIFEST_DIR").unwrap());

cli/specifier_handler.rs

@@ -20,15 +20,12 @@ use std::env;
 use std::error::Error;
 use std::fmt;
 use std::pin::Pin;
-use std::result;
 use std::sync::Arc;
 
-type Result<V> = result::Result<V, AnyError>;
-
 pub type DependencyMap = HashMap<String, Dependency>;
 pub type EmitMap = HashMap<EmitType, (TextDocument, Option<TextDocument>)>;
 pub type FetchFuture =
-  Pin<Box<(dyn Future<Output = Result<CachedModule>> + 'static)>>;
+  Pin<Box<(dyn Future<Output = Result<CachedModule, AnyError>> + 'static)>>;
 
 #[derive(Debug, Clone)]
 pub struct CachedModule {
@@ -89,7 +86,7 @@ pub struct Dependency {
 
 pub trait SpecifierHandler {
   /// Instructs the handler to fetch a specifier or retrieve its value from the
-  /// cache if there is a valid cached version.
+  /// cache.
   fn fetch(&mut self, specifier: ModuleSpecifier) -> FetchFuture;
 
   /// Get the optional build info from the cache for a given module specifier.
@@ -101,7 +98,7 @@ pub trait SpecifierHandler {
     &self,
     specifier: &ModuleSpecifier,
     emit_type: &EmitType,
-  ) -> Result<Option<TextDocument>>;
+  ) -> Result<Option<TextDocument>, AnyError>;
 
   /// Set the emitted code (and maybe map) for a given module specifier.  The
   /// cache type indicates what form the emit is related to.
@@ -111,7 +108,7 @@ pub trait SpecifierHandler {
     emit_type: &EmitType,
     code: TextDocument,
     maybe_map: Option<TextDocument>,
-  ) -> Result<()>;
+  ) -> Result<(), AnyError>;
 
   /// When parsed out of a JavaScript module source, the triple slash reference
   /// to the types should be stored in the cache.
@@ -119,7 +116,7 @@ pub trait SpecifierHandler {
     &mut self,
     specifier: &ModuleSpecifier,
     types: String,
-  ) -> Result<()>;
+  ) -> Result<(), AnyError>;
 
   /// Set the build info for a module specifier, also providing the cache type.
   fn set_build_info(
@@ -127,22 +124,22 @@ pub trait SpecifierHandler {
     specifier: &ModuleSpecifier,
     emit_type: &EmitType,
     build_info: TextDocument,
-  ) -> Result<()>;
+  ) -> Result<(), AnyError>;
 
   /// Set the graph dependencies for a given module specifier.
   fn set_deps(
     &mut self,
     specifier: &ModuleSpecifier,
     dependencies: DependencyMap,
-  ) -> Result<()>;
+  ) -> Result<(), AnyError>;
 
   /// Set the version of the source for a given module, which is used to help
-  /// determine if a module needs to be re-type-checked.
+  /// determine if a module needs to be re-emitted.
   fn set_version(
     &mut self,
     specifier: &ModuleSpecifier,
     version: String,
-  ) -> Result<()>;
+  ) -> Result<(), AnyError>;
 }
 
 impl fmt::Debug for dyn SpecifierHandler {
@@ -185,11 +182,12 @@ pub struct CompiledFileMetadata {
 }
 
 impl CompiledFileMetadata {
-  pub fn from_json_string(metadata_string: &str) -> Result<Self> {
+  pub fn from_bytes(bytes: &[u8]) -> Result<Self, AnyError> {
+    let metadata_string = std::str::from_utf8(bytes)?;
     serde_json::from_str::<Self>(metadata_string).map_err(|e| e.into())
   }
 
-  pub fn to_json_string(&self) -> Result<String> {
+  pub fn to_json_string(&self) -> Result<String, AnyError> {
     serde_json::to_string(self).map_err(|e| e.into())
   }
 }
@@ -207,7 +205,7 @@ impl FetchHandler {
   pub fn new(
     global_state: &Arc<GlobalState>,
     permissions: Permissions,
-  ) -> Result<Self> {
+  ) -> Result<Self, AnyError> {
     let custom_root = env::var("DENO_DIR").map(String::into).ok();
     let deno_dir = DenoDir::new(custom_root)?;
     let disk_cache = deno_dir.gen_cache;
@@ -234,9 +232,8 @@ impl SpecifierHandler for FetchHandler {
       let url = source_file.url;
       let filename = disk_cache.get_cache_filename_with_extension(&url, "meta");
       let maybe_version = if let Ok(bytes) = disk_cache.get(&filename) {
-        if let Ok(metadata_string) = std::str::from_utf8(&bytes) {
         if let Ok(compiled_file_metadata) =
-            CompiledFileMetadata::from_json_string(metadata_string)
+          CompiledFileMetadata::from_bytes(&bytes)
         {
           Some(compiled_file_metadata.version_hash)
         } else {
@@ -244,9 +241,6 @@ impl SpecifierHandler for FetchHandler {
         }
       } else {
         None
-        }
-      } else {
-        None
       };
 
       let filename =
@@ -280,7 +274,7 @@ impl SpecifierHandler for FetchHandler {
     &self,
     specifier: &ModuleSpecifier,
     emit_type: &EmitType,
-  ) -> Result<Option<TextDocument>> {
+  ) -> Result<Option<TextDocument>, AnyError> {
     if emit_type != &EmitType::Cli {
       return Err(UnsupportedEmitType(emit_type.clone()).into());
     }
@@ -299,7 +293,7 @@ impl SpecifierHandler for FetchHandler {
     specifier: &ModuleSpecifier,
     emit_type: &EmitType,
     build_info: TextDocument,
-  ) -> Result<()> {
+  ) -> Result<(), AnyError> {
     if emit_type != &EmitType::Cli {
       return Err(UnsupportedEmitType(emit_type.clone()).into());
     }
@@ -318,7 +312,7 @@ impl SpecifierHandler for FetchHandler {
     emit_type: &EmitType,
     code: TextDocument,
     maybe_map: Option<TextDocument>,
-  ) -> Result<()> {
+  ) -> Result<(), AnyError> {
     if emit_type != &EmitType::Cli {
       return Err(UnsupportedEmitType(emit_type.clone()).into());
     }
@@ -341,7 +335,7 @@ impl SpecifierHandler for FetchHandler {
     &mut self,
     _specifier: &ModuleSpecifier,
     _dependencies: DependencyMap,
-  ) -> Result<()> {
+  ) -> Result<(), AnyError> {
     // file_fetcher doesn't have the concept of caching dependencies
     Ok(())
   }
@@ -350,7 +344,7 @@ impl SpecifierHandler for FetchHandler {
     &mut self,
     _specifier: &ModuleSpecifier,
     _types: String,
-  ) -> Result<()> {
+  ) -> Result<(), AnyError> {
     // file_fetcher doesn't have the concept of caching of the types
     Ok(())
   }
@@ -359,7 +353,7 @@ impl SpecifierHandler for FetchHandler {
     &mut self,
     specifier: &ModuleSpecifier,
     version_hash: String,
-  ) -> Result<()> {
+  ) -> Result<(), AnyError> {
     let compiled_file_metadata = CompiledFileMetadata { version_hash };
     let filename = self
       .disk_cache
@@ -408,7 +402,10 @@ pub mod tests {
   impl MockSpecifierHandler {}
 
   impl MockSpecifierHandler {
-    fn get_cache(&self, specifier: ModuleSpecifier) -> Result<CachedModule> {
+    fn get_cache(
+      &self,
+      specifier: ModuleSpecifier,
+    ) -> Result<CachedModule, AnyError> {
       let specifier_text = specifier
         .to_string()
         .replace(":///", "_")
@@ -449,7 +446,7 @@ pub mod tests {
       &self,
       specifier: &ModuleSpecifier,
       _cache_type: &EmitType,
-    ) -> Result<Option<TextDocument>> {
+    ) -> Result<Option<TextDocument>, AnyError> {
       Ok(self.build_info.get(specifier).cloned())
     }
     fn set_cache(
@@ -458,7 +455,7 @@ pub mod tests {
       cache_type: &EmitType,
       code: TextDocument,
       maybe_map: Option<TextDocument>,
-    ) -> Result<()> {
+    ) -> Result<(), AnyError> {
       self.cache_calls.push((
         specifier.clone(),
         cache_type.clone(),
@@ -471,7 +468,7 @@ pub mod tests {
       &mut self,
       specifier: &ModuleSpecifier,
       types: String,
-    ) -> Result<()> {
+    ) -> Result<(), AnyError> {
       self.types_calls.push((specifier.clone(), types));
       Ok(())
     }
@@ -480,7 +477,7 @@ pub mod tests {
       specifier: &ModuleSpecifier,
       cache_type: &EmitType,
       build_info: TextDocument,
-    ) -> Result<()> {
+    ) -> Result<(), AnyError> {
       self
         .build_info
         .insert(specifier.clone(), build_info.clone());
@@ -495,7 +492,7 @@ pub mod tests {
       &mut self,
       specifier: &ModuleSpecifier,
       dependencies: DependencyMap,
-    ) -> Result<()> {
+    ) -> Result<(), AnyError> {
       self.deps_calls.push((specifier.clone(), dependencies));
       Ok(())
     }
@@ -503,7 +500,7 @@ pub mod tests {
       &mut self,
       specifier: &ModuleSpecifier,
       version: String,
-    ) -> Result<()> {
+    ) -> Result<(), AnyError> {
       self.version_calls.push((specifier.clone(), version));
       Ok(())
     }

cli/tests/integration_tests.rs

@@ -344,6 +344,88 @@ fn cache_test() {
   // DENO_DIR?
 }
 
+#[test]
+fn cache_invalidation_test() {
+  let deno_dir = TempDir::new().expect("tempdir fail");
+  let fixture_path = deno_dir.path().join("fixture.ts");
+  {
+    let mut file = std::fs::File::create(fixture_path.clone())
+      .expect("could not create fixture");
+    file
+      .write_all(b"console.log(\"42\");")
+      .expect("could not write fixture");
+  }
+  let output = Command::new(util::deno_exe_path())
+    .env("DENO_DIR", deno_dir.path())
+    .current_dir(util::root_path())
+    .arg("run")
+    .arg(fixture_path.to_str().unwrap())
+    .output()
+    .expect("Failed to spawn script");
+  assert!(output.status.success());
+  let actual = std::str::from_utf8(&output.stdout).unwrap();
+  assert_eq!(actual, "42\n");
+  {
+    let mut file = std::fs::File::create(fixture_path.clone())
+      .expect("could not create fixture");
+    file
+      .write_all(b"console.log(\"43\");")
+      .expect("could not write fixture");
+  }
+  let output = Command::new(util::deno_exe_path())
+    .env("DENO_DIR", deno_dir.path())
+    .current_dir(util::root_path())
+    .arg("run")
+    .arg(fixture_path.to_str().unwrap())
+    .output()
+    .expect("Failed to spawn script");
+  assert!(output.status.success());
+  let actual = std::str::from_utf8(&output.stdout).unwrap();
+  assert_eq!(actual, "43\n");
+}
+
+#[test]
+fn cache_invalidation_test_no_check() {
+  let deno_dir = TempDir::new().expect("tempdir fail");
+  let fixture_path = deno_dir.path().join("fixture.ts");
+  {
+    let mut file = std::fs::File::create(fixture_path.clone())
+      .expect("could not create fixture");
+    file
+      .write_all(b"console.log(\"42\");")
+      .expect("could not write fixture");
+  }
+  let output = Command::new(util::deno_exe_path())
+    .env("DENO_DIR", deno_dir.path())
+    .current_dir(util::root_path())
+    .arg("run")
+    .arg("--no-check")
+    .arg(fixture_path.to_str().unwrap())
+    .output()
+    .expect("Failed to spawn script");
+  assert!(output.status.success());
+  let actual = std::str::from_utf8(&output.stdout).unwrap();
+  assert_eq!(actual, "42\n");
+  {
+    let mut file = std::fs::File::create(fixture_path.clone())
+      .expect("could not create fixture");
+    file
+      .write_all(b"console.log(\"43\");")
+      .expect("could not write fixture");
+  }
+  let output = Command::new(util::deno_exe_path())
+    .env("DENO_DIR", deno_dir.path())
+    .current_dir(util::root_path())
+    .arg("run")
+    .arg("--no-check")
+    .arg(fixture_path.to_str().unwrap())
+    .output()
+    .expect("Failed to spawn script");
+  assert!(output.status.success());
+  let actual = std::str::from_utf8(&output.stdout).unwrap();
+  assert_eq!(actual, "43\n");
+}
+
 #[test]
 fn fmt_test() {
   let t = TempDir::new().expect("tempdir fail");

cli/tsc_config.rs

@@ -210,6 +210,10 @@ impl TsConfig {
     TsConfig(value)
   }
 
+  pub fn as_bytes(&self) -> Vec<u8> {
+    self.0.to_string().as_bytes().to_owned()
+  }
+
   /// Take an optional string representing a user provided TypeScript config file
   /// which was passed in via the `--config` compiler option and merge it with
   /// the configuration.  Returning the result which optionally contains any
@@ -233,7 +237,8 @@ impl TsConfig {
           ),
         )
       })?;
-      let config_text = std::fs::read_to_string(config_path.clone())?;
+      let config_bytes = std::fs::read(config_path.clone())?;
+      let config_text = std::str::from_utf8(&config_bytes)?;
       let (value, maybe_ignored_options) =
         parse_config(&config_text, &config_path)?;
       json_merge(&mut self.0, &value);