Skip to content

Commit

Permalink
Revert "fix(watch): preserve ProcState::file_fetcher between restar…
Browse files Browse the repository at this point in the history
…ts (#15466) (#17591)

This reverts commit 3545bff.
  • Loading branch information
bartlomieju committed Jan 30, 2023
1 parent 3035dee commit d318e38
Show file tree
Hide file tree
Showing 13 changed files with 43 additions and 127 deletions.
4 changes: 3 additions & 1 deletion cli/cache/mod.rs
Expand Up @@ -50,10 +50,12 @@ pub struct FetchCacher {
impl FetchCacher {
pub fn new(
emit_cache: EmitCache,
file_fetcher: Arc<FileFetcher>,
file_fetcher: FileFetcher,
root_permissions: PermissionsContainer,
dynamic_permissions: PermissionsContainer,
) -> Self {
let file_fetcher = Arc::new(file_fetcher);

Self {
emit_cache,
dynamic_permissions,
Expand Down
1 change: 0 additions & 1 deletion cli/cache/node.rs
Expand Up @@ -24,7 +24,6 @@ struct CjsAnalysisData {
pub reexports: Vec<String>,
}

#[derive(Clone)]
pub struct NodeAnalysisCache {
db_file_path: Option<PathBuf>,
inner: Arc<Mutex<Option<Option<NodeAnalysisCacheInner>>>>,
Expand Down
8 changes: 0 additions & 8 deletions cli/cache/parsed_source.rs
Expand Up @@ -67,14 +67,6 @@ impl ParsedSourceCache {
}
}

pub fn reset_for_file_watcher(&self) -> Self {
Self {
db_cache_path: self.db_cache_path.clone(),
cli_version: self.cli_version.clone(),
sources: Default::default(),
}
}

pub fn get_parsed_source_from_module(
&self,
module: &deno_graph::Module,
Expand Down
8 changes: 4 additions & 4 deletions cli/graph_util.rs
Expand Up @@ -71,7 +71,7 @@ pub struct GraphData {

impl GraphData {
/// Store data from `graph` into `self`.
pub fn add_graph(&mut self, graph: &ModuleGraph) {
pub fn add_graph(&mut self, graph: &ModuleGraph, reload: bool) {
for graph_import in &graph.imports {
for dep in graph_import.dependencies.values() {
for resolved in [&dep.maybe_code, &dep.maybe_type] {
Expand All @@ -90,7 +90,7 @@ impl GraphData {
let mut has_npm_specifier_in_graph = false;

for (specifier, result) in graph.specifiers() {
if self.modules.contains_key(specifier) {
if !reload && self.modules.contains_key(specifier) {
continue;
}

Expand Down Expand Up @@ -477,7 +477,7 @@ impl GraphData {
impl From<&ModuleGraph> for GraphData {
fn from(graph: &ModuleGraph) -> Self {
let mut graph_data = GraphData::default();
graph_data.add_graph(graph);
graph_data.add_graph(graph, false);
graph_data
}
}
Expand Down Expand Up @@ -549,7 +549,7 @@ pub async fn create_graph_and_maybe_check(

let check_js = ps.options.check_js();
let mut graph_data = GraphData::default();
graph_data.add_graph(&graph);
graph_data.add_graph(&graph, false);
graph_data
.check(
&graph.roots,
Expand Down
1 change: 1 addition & 0 deletions cli/module_loader.rs
Expand Up @@ -277,6 +277,7 @@ impl ModuleLoader for CliModuleLoader {
lib,
root_permissions,
dynamic_permissions,
false,
)
.await
}
Expand Down
50 changes: 12 additions & 38 deletions cli/proc_state.rs
Expand Up @@ -75,7 +75,7 @@ pub struct ProcState(Arc<Inner>);

pub struct Inner {
pub dir: DenoDir,
pub file_fetcher: Arc<FileFetcher>,
pub file_fetcher: FileFetcher,
pub http_client: HttpClient,
pub options: Arc<CliOptions>,
pub emit_cache: EmitCache,
Expand Down Expand Up @@ -145,38 +145,6 @@ impl ProcState {
Ok(ps)
}

/// Reset all runtime state to its default. This should be used on file
/// watcher restarts.
pub fn reset_for_file_watcher(&mut self) {
self.0 = Arc::new(Inner {
dir: self.dir.clone(),
options: self.options.clone(),
emit_cache: self.emit_cache.clone(),
emit_options_hash: self.emit_options_hash,
emit_options: self.emit_options.clone(),
file_fetcher: self.file_fetcher.clone(),
http_client: self.http_client.clone(),
graph_data: Default::default(),
lockfile: self.lockfile.clone(),
maybe_import_map: self.maybe_import_map.clone(),
maybe_inspector_server: self.maybe_inspector_server.clone(),
root_cert_store: self.root_cert_store.clone(),
blob_store: Default::default(),
broadcast_channel: Default::default(),
shared_array_buffer_store: Default::default(),
compiled_wasm_module_store: Default::default(),
parsed_source_cache: self.parsed_source_cache.reset_for_file_watcher(),
maybe_resolver: self.maybe_resolver.clone(),
maybe_file_watcher_reporter: self.maybe_file_watcher_reporter.clone(),
node_analysis_cache: self.node_analysis_cache.clone(),
npm_cache: self.npm_cache.clone(),
npm_resolver: self.npm_resolver.clone(),
cjs_resolutions: Default::default(),
progress_bar: self.progress_bar.clone(),
node_std_graph_prepared: AtomicBool::new(false),
});
}

async fn build_with_sender(
cli_options: Arc<CliOptions>,
maybe_sender: Option<tokio::sync::mpsc::UnboundedSender<Vec<PathBuf>>>,
Expand Down Expand Up @@ -268,7 +236,7 @@ impl ProcState {
.write_hashable(&emit_options)
.finish(),
emit_options,
file_fetcher: Arc::new(file_fetcher),
file_fetcher,
http_client,
graph_data: Default::default(),
lockfile,
Expand Down Expand Up @@ -303,6 +271,7 @@ impl ProcState {
lib: TsTypeLib,
root_permissions: PermissionsContainer,
dynamic_permissions: PermissionsContainer,
reload_on_watch: bool,
) -> Result<(), AnyError> {
log::debug!("Preparing module load.");
let _pb_clear_guard = self.progress_bar.clear_guard();
Expand All @@ -311,7 +280,7 @@ impl ProcState {
r.scheme() == "npm" && NpmPackageReference::from_specifier(r).is_ok()
});

if !has_root_npm_specifier {
if !reload_on_watch && !has_root_npm_specifier {
let graph_data = self.graph_data.read();
if self.options.type_check_mode() == TypeCheckMode::None
|| graph_data.is_type_checked(&roots, &lib)
Expand Down Expand Up @@ -345,6 +314,7 @@ impl ProcState {
struct ProcStateLoader<'a> {
inner: &'a mut cache::FetchCacher,
graph_data: Arc<RwLock<GraphData>>,
reload: bool,
}
impl Loader for ProcStateLoader<'_> {
fn get_cache_info(
Expand All @@ -361,14 +331,17 @@ impl ProcState {
let graph_data = self.graph_data.read();
let found_specifier = graph_data.follow_redirect(specifier);
match graph_data.get(&found_specifier) {
Some(_) => Box::pin(futures::future::ready(Err(anyhow!("")))),
Some(_) if !self.reload => {
Box::pin(futures::future::ready(Err(anyhow!(""))))
}
_ => self.inner.load(specifier, is_dynamic),
}
}
}
let mut loader = ProcStateLoader {
inner: &mut cache,
graph_data: self.graph_data.clone(),
reload: reload_on_watch,
};

let maybe_file_watcher_reporter: Option<&dyn deno_graph::source::Reporter> =
Expand Down Expand Up @@ -407,7 +380,7 @@ impl ProcState {

let (npm_package_reqs, has_node_builtin_specifier) = {
let mut graph_data = self.graph_data.write();
graph_data.add_graph(&graph);
graph_data.add_graph(&graph, reload_on_watch);
let check_js = self.options.check_js();
graph_data
.check(
Expand Down Expand Up @@ -506,6 +479,7 @@ impl ProcState {
lib,
PermissionsContainer::allow_all(),
PermissionsContainer::allow_all(),
false,
)
.await
}
Expand All @@ -519,7 +493,7 @@ impl ProcState {
let node_std_graph = self
.create_graph(vec![node::MODULE_ALL_URL.clone()])
.await?;
self.graph_data.write().add_graph(&node_std_graph);
self.graph_data.write().add_graph(&node_std_graph, false);
self.node_std_graph_prepared.store(true, Ordering::Relaxed);
Ok(())
}
Expand Down
40 changes: 1 addition & 39 deletions cli/tests/integration/watcher_tests.rs
Expand Up @@ -1086,44 +1086,6 @@ fn test_watch_unload_handler_error_on_drop() {
check_alive_then_kill(child);
}

// Regression test for https://github.com/denoland/deno/issues/15465.
#[test]
fn run_watch_reload_once() {
let _g = util::http_server();
let t = TempDir::new();
let file_to_watch = t.path().join("file_to_watch.js");
let file_content = r#"
import { time } from "http://localhost:4545/dynamic_module.ts";
console.log(time);
"#;
write(&file_to_watch, file_content).unwrap();

let mut child = util::deno_cmd()
.current_dir(util::testdata_path())
.arg("run")
.arg("--watch")
.arg("--reload")
.arg(&file_to_watch)
.env("NO_COLOR", "1")
.stdout(std::process::Stdio::piped())
.stderr(std::process::Stdio::piped())
.spawn()
.unwrap();
let (mut stdout_lines, mut stderr_lines) = child_lines(&mut child);

wait_contains("finished", &mut stderr_lines);
let first_output = stdout_lines.next().unwrap();

write(&file_to_watch, file_content).unwrap();
// The remote dynamic module should not have been reloaded again.

wait_contains("finished", &mut stderr_lines);
let second_output = stdout_lines.next().unwrap();
assert_eq!(second_output, first_output);

check_alive_then_kill(child);
}

#[test]
fn run_watch_dynamic_imports() {
let t = TempDir::new();
Expand Down Expand Up @@ -1185,11 +1147,11 @@ fn run_watch_dynamic_imports() {
&mut stdout_lines,
);

wait_contains("finished", &mut stderr_lines);
wait_for(
|m| m.contains("Watching paths") && m.contains("imported2.js"),
&mut stderr_lines,
);
wait_contains("finished", &mut stderr_lines);

write(
&file_to_watch3,
Expand Down
12 changes: 4 additions & 8 deletions cli/tools/bench.rs
Expand Up @@ -30,7 +30,6 @@ use indexmap::IndexMap;
use log::Level;
use serde::Deserialize;
use serde::Serialize;
use std::cell::RefCell;
use std::collections::HashSet;
use std::path::Path;
use std::path::PathBuf;
Expand Down Expand Up @@ -338,6 +337,7 @@ async fn check_specifiers(
lib,
PermissionsContainer::allow_all(),
PermissionsContainer::new(permissions),
true,
)
.await?;

Expand Down Expand Up @@ -529,14 +529,12 @@ pub async fn run_benchmarks_with_watch(
Permissions::from_options(&ps.options.permissions_options())?;
let no_check = ps.options.type_check_mode() == TypeCheckMode::None;

let ps = RefCell::new(ps);

let resolver = |changed: Option<Vec<PathBuf>>| {
let paths_to_watch = bench_options.files.include.clone();
let paths_to_watch_clone = paths_to_watch.clone();
let files_changed = changed.is_some();
let bench_options = &bench_options;
let ps = ps.borrow().clone();
let ps = ps.clone();

async move {
let bench_modules =
Expand Down Expand Up @@ -640,8 +638,7 @@ pub async fn run_benchmarks_with_watch(
let operation = |modules_to_reload: Vec<ModuleSpecifier>| {
let permissions = &permissions;
let bench_options = &bench_options;
ps.borrow_mut().reset_for_file_watcher();
let ps = ps.borrow().clone();
let ps = ps.clone();

async move {
let specifiers =
Expand All @@ -666,13 +663,12 @@ pub async fn run_benchmarks_with_watch(
}
};

let clear_screen = !ps.borrow().options.no_clear_screen();
file_watcher::watch_func(
resolver,
operation,
file_watcher::PrintConfig {
job_name: "Bench".to_string(),
clear_screen,
clear_screen: !ps.options.no_clear_screen(),
},
)
.await?;
Expand Down
16 changes: 10 additions & 6 deletions cli/tools/run.rs
@@ -1,6 +1,7 @@
// Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.

use std::io::Read;
use std::path::PathBuf;
use std::sync::Arc;

use deno_ast::MediaType;
Expand Down Expand Up @@ -102,13 +103,16 @@ async fn run_with_watch(flags: Flags, script: String) -> Result<i32, AnyError> {
let flags = Arc::new(flags);
let main_module = resolve_url_or_path(&script)?;
let (sender, receiver) = tokio::sync::mpsc::unbounded_channel();
let mut ps =
ProcState::build_for_file_watcher((*flags).clone(), sender.clone()).await?;

let operation = |main_module: ModuleSpecifier| {
ps.reset_for_file_watcher();
let ps = ps.clone();
let operation = |(sender, main_module): (
tokio::sync::mpsc::UnboundedSender<Vec<PathBuf>>,
ModuleSpecifier,
)| {
let flags = flags.clone();
Ok(async move {
let ps =
ProcState::build_for_file_watcher((*flags).clone(), sender.clone())
.await?;
let permissions = PermissionsContainer::new(Permissions::from_options(
&ps.options.permissions_options(),
)?);
Expand All @@ -122,7 +126,7 @@ async fn run_with_watch(flags: Flags, script: String) -> Result<i32, AnyError> {
util::file_watcher::watch_func2(
receiver,
operation,
main_module,
(sender, main_module),
util::file_watcher::PrintConfig {
job_name: "Process".to_string(),
clear_screen: !flags.no_clear_screen,
Expand Down

0 comments on commit d318e38

Please sign in to comment.