Skip to content

refactor: remove dead TaskDependencyType and clean up task graph construction #283

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
yangshun wants to merge 1 commit into from
Closed

refactor: remove dead TaskDependencyType and clean up task graph construction #283

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 25 additions & 49 deletions crates/vite_task_graph/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,25 +17,8 @@ use vite_workspace::{PackageNodeIndex, WorkspaceRoot, package_graph::IndexedPack

use crate::display::TaskDisplay;

/// The type of a task dependency edge in the task graph.
///
/// Currently only `Explicit` is produced (from `dependsOn` in `vite-task.json`).
/// Topological ordering is handled at query time via the package subgraph rather
/// than by pre-computing edges in the task graph.
#[derive(Debug, Clone, Copy, Serialize)]
pub struct TaskDependencyType;

impl TaskDependencyType {
/// Returns `true` — all task graph edges are explicit `dependsOn` dependencies.
///
/// Kept as an associated function for use as a filter predicate in
/// `add_dependencies`. Always returns `true` since `TaskDependencyType`
/// only represents explicit edges now.
#[must_use]
pub const fn is_explicit() -> bool {
true
}
}
// Task dependency edges are all explicit `dependsOn` dependencies.
// Topological ordering is handled at query time via the package subgraph.

/// Uniquely identifies a task, by its name and the package where it's defined.
#[derive(Debug, PartialEq, Eq, Hash, Clone, PartialOrd, Ord)]
Expand Down Expand Up @@ -180,7 +163,7 @@ pub type TaskEdgeIndex = EdgeIndex<TaskIx>;
/// External factors (e.g. additional args from cli, current working directory, environmental variables) are not stored here.
#[derive(Debug)]
pub struct IndexedTaskGraph {
task_graph: DiGraph<TaskNode, TaskDependencyType, TaskIx>,
task_graph: DiGraph<TaskNode, (), TaskIx>,

/// Preserve the package graph for two purposes:
/// - `self.task_graph` refers packages via `PackageNodeIndex`. To display package names and paths, we need to lookup them in `package_graph`.
Expand All @@ -200,7 +183,7 @@ pub struct IndexedTaskGraph {
pre_post_scripts_enabled: bool,
}

pub type TaskGraph = DiGraph<TaskNode, TaskDependencyType, TaskIx>;
pub type TaskGraph = DiGraph<TaskNode, (), TaskIx>;

impl IndexedTaskGraph {
/// Load the task graph from a discovered workspace using the provided config loader.
Expand All @@ -219,18 +202,19 @@ impl IndexedTaskGraph {
workspace_root: &WorkspaceRoot,
config_loader: &dyn loader::UserConfigLoader,
) -> Result<Self, TaskGraphLoadError> {
let mut task_graph = DiGraph::<TaskNode, TaskDependencyType, TaskIx>::default();
let mut task_graph = DiGraph::<TaskNode, (), TaskIx>::default();

let package_graph = vite_workspace::load_package_graph(workspace_root)?;

// Record dependency specifiers for each task node to add explicit dependencies later
let mut task_ids_with_dependency_specifiers: Vec<(TaskId, Option<Arc<[Str]>>)> = Vec::new();

// index tasks by ids
let estimated_task_count = package_graph.node_count() * 2;
let mut node_indices_by_task_id: FxHashMap<TaskId, TaskNodeIndex> =
FxHashMap::with_capacity_and_hasher(task_graph.node_count(), FxBuildHasher);
FxHashMap::with_capacity_and_hasher(estimated_task_count, FxBuildHasher);
let mut task_ids_by_node_index: FxHashMap<TaskNodeIndex, TaskId> =
FxHashMap::with_capacity_and_hasher(task_graph.node_count(), FxBuildHasher);
FxHashMap::with_capacity_and_hasher(estimated_task_count, FxBuildHasher);

// First pass: load all configs, extract root cache config, validate
let mut root_cache = None;
Expand Down Expand Up @@ -305,6 +289,12 @@ impl IndexedTaskGraph {

let dependency_specifiers = task_user_config.options.depends_on.clone();

let task_display = TaskDisplay {
package_name: package.package_json.name.clone(),
task_name: task_name.clone(),
package_path: Arc::clone(&package_dir),
};

// Resolve the task configuration from the user config
let resolved_config = ResolvedTaskConfig::resolve(
task_user_config,
Expand All @@ -313,22 +303,11 @@ impl IndexedTaskGraph {
)
.map_err(|err| TaskGraphLoadError::ResolveConfigError {
error: err,
task_display: TaskDisplay {
package_name: package.package_json.name.clone(),
task_name: task_name.clone(),
package_path: Arc::clone(&package_dir),
},
task_display: task_display.clone(),
})?;

let task_node = TaskNode {
task_display: TaskDisplay {
package_name: package.package_json.name.clone(),
task_name: task_name.clone(),
package_path: Arc::clone(&package_dir),
},
resolved_config,
source: TaskSource::TaskConfig,
};
let task_node =
TaskNode { task_display, resolved_config, source: TaskSource::TaskConfig };

let node_index = task_graph.add_node(task_node);
task_ids_with_dependency_specifiers.push((task_id.clone(), dependency_specifiers));
Expand All @@ -339,25 +318,22 @@ impl IndexedTaskGraph {
// For remaining package.json scripts not in the tasks map, create tasks with default config
for (script_name, package_json_script) in package_json_scripts {
let task_id = TaskId { task_name: Str::from(script_name), package_index };
let task_display = TaskDisplay {
package_name: package.package_json.name.clone(),
task_name: script_name.into(),
package_path: Arc::clone(&package_dir),
};
let resolved_config = ResolvedTaskConfig::resolve_package_json_script(
&package_dir,
package_json_script,
&workspace_root.path,
)
.map_err(|err| TaskGraphLoadError::ResolveConfigError {
error: err,
task_display: TaskDisplay {
package_name: package.package_json.name.clone(),
task_name: script_name.into(),
package_path: Arc::clone(&package_dir),
},
task_display: task_display.clone(),
})?;
let node_index = task_graph.add_node(TaskNode {
task_display: TaskDisplay {
package_name: package.package_json.name.clone(),
task_name: script_name.into(),
package_path: Arc::clone(&package_dir),
},
task_display,
resolved_config,
source: TaskSource::PackageJsonScript,
});
Expand Down Expand Up @@ -390,7 +366,7 @@ impl IndexedTaskGraph {
specifier,
task_display: me.display_task(from_node_index),
})?;
me.task_graph.update_edge(from_node_index, to_node_index, TaskDependencyType);
me.task_graph.update_edge(from_node_index, to_node_index, ());
}
}

Expand Down
28 changes: 10 additions & 18 deletions crates/vite_task_graph/src/query/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use vite_str::Str;
use vite_workspace::PackageNodeIndex;
pub use vite_workspace::package_graph::{PackageQuery, PackageQueryResolveError};

use crate::{IndexedTaskGraph, TaskDependencyType, TaskId, TaskNodeIndex};
use crate::{IndexedTaskGraph, TaskId, TaskNodeIndex};

/// A task execution graph queried from a `TaskQuery`.
///
Expand Down Expand Up @@ -109,7 +109,7 @@ impl IndexedTaskGraph {

// Expand explicit dependsOn edges (may add new task nodes from outside the subgraph).
if query.include_explicit_deps {
self.add_dependencies(&mut execution_graph, |_| TaskDependencyType::is_explicit());
self.add_dependencies(&mut execution_graph);
}

Ok(TaskQueryResult { execution_graph, unmatched_selectors: resolution.unmatched_selectors })
Expand Down Expand Up @@ -177,16 +177,11 @@ impl IndexedTaskGraph {
}
}

/// Recursively add dependencies to the execution graph based on filtered edges.
/// Recursively add explicit `dependsOn` dependencies to the execution graph.
///
/// Starts from the current nodes in `execution_graph` and follows outgoing edges
/// that match `filter_edge`, adding new nodes to the frontier until no new nodes
/// are discovered.
fn add_dependencies(
&self,
execution_graph: &mut TaskExecutionGraph,
mut filter_edge: impl FnMut(TaskDependencyType) -> bool,
) {
/// Starts from the current nodes in `execution_graph` and follows outgoing edges,
/// adding new nodes to the frontier until no new nodes are discovered.
fn add_dependencies(&self, execution_graph: &mut TaskExecutionGraph) {
let mut frontier: FxHashSet<TaskNodeIndex> = execution_graph.nodes().collect();

// Continue until no new nodes are added to the frontier.
Expand All @@ -196,13 +191,10 @@ impl IndexedTaskGraph {
for from_node in frontier {
for edge_ref in self.task_graph.edges(from_node) {
let to_node = edge_ref.target();
let dep_type = *edge_ref.weight();
if filter_edge(dep_type) {
let is_new = !execution_graph.contains_node(to_node);
execution_graph.add_edge(from_node, to_node, ());
if is_new {
next_frontier.insert(to_node);
}
let is_new = !execution_graph.contains_node(to_node);
execution_graph.add_edge(from_node, to_node, ());
if is_new {
next_frontier.insert(to_node);
}
}
}
Expand Down
Loading