Skip to content

Commit

Permalink
feat: sibling tasks (#9504)
Browse files Browse the repository at this point in the history
### Description

This PR does 2 things:
- Adds a `sibling` attribute for task definitions that has the ability
to bring another task into the graph without depending on it. This is
currently only an attribute that can be set by us inside Rust, but I
hope this use case proves useful enough to eventually make this a public
API.
- Switches MFE proxy injection over to use `siblings` where each dev
task gets a `"siblings": ["mfe-pkg#proxy"]` to ensure the proxy gets
run.

A sibling relationship acts similar to `dependsOn` except that it will
not wait for the task to exit before starting and the sibling's task
hash will not affect the current task's hash.

Currently it's very hard to get `web#proxy` to run if the user runs a
command like `turbo dev --filter=docs` as just adding `web` to the
filter will mean `web#dev` gets picked up which isn't what we desire. As
you can see in `run/builder.rs` this also removes the need of
microfrontend knowledge from building the task graph.

### Testing Instructions

Added unit tests for sibling behavior as well as unit tests for MFE task
injection.

Manual testing for the MFE behavior to make sure proxy still gets picked
up.

---------

Co-authored-by: Nicholas Yang <[email protected]>
  • Loading branch information
chris-olszewski and NicholasLYang authored Nov 25, 2024
1 parent e3fcdee commit 4f5f048
Show file tree
Hide file tree
Showing 9 changed files with 405 additions and 76 deletions.
5 changes: 5 additions & 0 deletions crates/turborepo-errors/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,11 @@ impl<T> Spanned<T> {
text: self.text,
}
}

/// Gets a mutable ref to the inner value
pub fn as_inner_mut(&mut self) -> &mut T {
&mut self.value
}
}

impl<T> Spanned<Option<T>> {
Expand Down
51 changes: 51 additions & 0 deletions crates/turborepo-lib/src/engine/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,19 @@ impl<'a> EngineBuilder<'a> {
}
});

for (sibling, span) in task_definition
.siblings
.iter()
.flatten()
.map(|s| s.as_ref().split())
{
let sibling_task_id = sibling
.task_id()
.unwrap_or_else(|| TaskId::new(to_task_id.package(), sibling.task()))
.into_owned();
traversal_queue.push_back(span.to(sibling_task_id));
}

for (dep, span) in deps {
let from_task_id = dep
.task_id()
Expand Down Expand Up @@ -1333,6 +1346,44 @@ mod test {
assert_eq!(all_dependencies(&engine), expected);
}

#[test]
fn test_sibling_task() {
let repo_root_dir = TempDir::with_prefix("repo").unwrap();
let repo_root = AbsoluteSystemPathBuf::new(repo_root_dir.path().to_str().unwrap()).unwrap();
let package_graph = mock_package_graph(
&repo_root,
package_jsons! {
repo_root,
"web" => [],
"api" => []
},
);
let turbo_jsons = vec![(PackageName::Root, {
let mut t_json = turbo_json(json!({
"tasks": {
"web#dev": { "persistent": true },
"api#serve": { "persistent": true }
}
}));
t_json.with_sibling(TaskName::from("web#dev"), &TaskName::from("api#serve"));
t_json
})]
.into_iter()
.collect();
let loader = TurboJsonLoader::noop(turbo_jsons);
let engine = EngineBuilder::new(&repo_root, &package_graph, loader, false)
.with_tasks(Some(Spanned::new(TaskName::from("dev"))))
.with_workspaces(vec![PackageName::from("web")])
.build()
.unwrap();

let expected = deps! {
"web#dev" => ["___ROOT___"],
"api#serve" => ["___ROOT___"]
};
assert_eq!(all_dependencies(&engine), expected);
}

#[test]
fn test_run_package_task_exact_error() {
let repo_root_dir = TempDir::with_prefix("repo").unwrap();
Expand Down
206 changes: 202 additions & 4 deletions crates/turborepo-lib/src/micro_frontends.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,23 @@
use std::collections::{HashMap, HashSet};

use itertools::Itertools;
use tracing::warn;
use turbopath::AbsoluteSystemPath;
use turborepo_micro_frontend::{Config as MFEConfig, Error, DEFAULT_MICRO_FRONTENDS_CONFIG};
use turborepo_repository::package_graph::PackageGraph;
use turborepo_micro_frontend::{
Config as MFEConfig, Error, DEFAULT_MICRO_FRONTENDS_CONFIG, MICRO_FRONTENDS_PACKAGES,
};
use turborepo_repository::package_graph::{PackageGraph, PackageName};

use crate::run::task_id::TaskId;
use crate::{
config,
run::task_id::{TaskId, TaskName},
turbo_json::TurboJson,
};

#[derive(Debug, Clone)]
pub struct MicroFrontendsConfigs {
configs: HashMap<String, HashSet<TaskId<'static>>>,
mfe_package: Option<&'static str>,
}

impl MicroFrontendsConfigs {
Expand Down Expand Up @@ -44,7 +52,23 @@ impl MicroFrontendsConfigs {
configs.insert(package_name.to_string(), tasks);
}

Ok((!configs.is_empty()).then_some(Self { configs }))
let mfe_package = package_graph
.packages()
.map(|(pkg, _)| pkg.as_str())
.sorted()
// We use `find_map` here instead of a simple `find` so we get the &'static str
// instead of the &str tied to the lifetime of the package graph.
.find_map(|pkg| {
MICRO_FRONTENDS_PACKAGES
.iter()
.find(|static_pkg| pkg == **static_pkg)
})
.copied();

Ok((!configs.is_empty()).then_some(Self {
configs,
mfe_package,
}))
}

pub fn contains_package(&self, package_name: &str) -> bool {
Expand All @@ -64,4 +88,178 @@ impl MicroFrontendsConfigs {
.values()
.any(|dev_tasks| dev_tasks.contains(task_id))
}

pub fn update_turbo_json(
&self,
package_name: &PackageName,
turbo_json: Result<TurboJson, config::Error>,
) -> Result<TurboJson, config::Error> {
// If package either
// - contains the proxy task
// - a member of one of the microfrontends
// then we need to modify its task definitions
if let Some(FindResult { dev, proxy }) = self.package_turbo_json_update(package_name) {
// We need to modify turbo.json, use default one if there isn't one present
let mut turbo_json = turbo_json.or_else(|err| match err {
config::Error::NoTurboJSON => Ok(TurboJson::default()),
err => Err(err),
})?;

// If the current package contains the proxy task, then add that definition
if proxy.package() == package_name.as_str() {
turbo_json.with_proxy(self.mfe_package);
}

if let Some(dev) = dev {
// If this package has a dev task that's part of the MFE, then we make sure the
// proxy gets included in the task graph.
turbo_json.with_sibling(
TaskName::from(dev.task()).into_owned(),
&proxy.as_task_name(),
);
}

Ok(turbo_json)
} else {
turbo_json
}
}

fn package_turbo_json_update<'a>(
&'a self,
package_name: &PackageName,
) -> Option<FindResult<'a>> {
self.configs().find_map(|(config, tasks)| {
let dev_task = tasks.iter().find_map(|task| {
(task.package() == package_name.as_str()).then(|| FindResult {
dev: Some(task.as_borrowed()),
proxy: TaskId::new(config, "proxy"),
})
});
let proxy_owner = (config.as_str() == package_name.as_str()).then(|| FindResult {
dev: None,
proxy: TaskId::new(config, "proxy"),
});
dev_task.or(proxy_owner)
})
}
}

#[derive(Debug, PartialEq, Eq)]
struct FindResult<'a> {
dev: Option<TaskId<'a>>,
proxy: TaskId<'a>,
}

#[cfg(test)]
mod test {
use test_case::test_case;

use super::*;

macro_rules! mfe_configs {
{$($config_owner:expr => $dev_tasks:expr),+} => {
{
let mut _map = std::collections::HashMap::new();
$(
let mut _dev_tasks = std::collections::HashSet::new();
for _dev_task in $dev_tasks.as_slice() {
_dev_tasks.insert(crate::run::task_id::TaskName::from(*_dev_task).task_id().unwrap().into_owned());
}
_map.insert($config_owner.to_string(), _dev_tasks);
)+
_map
}
};
}

struct PackageUpdateTest {
package_name: &'static str,
result: Option<TestFindResult>,
}

struct TestFindResult {
dev: Option<&'static str>,
proxy: &'static str,
}

impl PackageUpdateTest {
pub const fn new(package_name: &'static str) -> Self {
Self {
package_name,
result: None,
}
}

pub const fn dev(mut self, dev: &'static str, proxy: &'static str) -> Self {
self.result = Some(TestFindResult {
dev: Some(dev),
proxy,
});
self
}

pub const fn proxy_only(mut self, proxy: &'static str) -> Self {
self.result = Some(TestFindResult { dev: None, proxy });
self
}

pub fn package_name(&self) -> PackageName {
PackageName::from(self.package_name)
}

pub fn expected(&self) -> Option<FindResult> {
match self.result {
Some(TestFindResult {
dev: Some(dev),
proxy,
}) => Some(FindResult {
dev: Some(Self::str_to_task(dev)),
proxy: Self::str_to_task(proxy),
}),
Some(TestFindResult { dev: None, proxy }) => Some(FindResult {
dev: None,
proxy: Self::str_to_task(proxy),
}),
None => None,
}
}

fn str_to_task(s: &str) -> TaskId<'static> {
crate::run::task_id::TaskName::from(s)
.task_id()
.unwrap()
.into_owned()
}
}

const NON_MFE_PKG: PackageUpdateTest = PackageUpdateTest::new("other-pkg");
const MFE_CONFIG_PKG: PackageUpdateTest =
PackageUpdateTest::new("mfe-config-pkg").proxy_only("mfe-config-pkg#proxy");
const MFE_CONFIG_PKG_DEV_TASK: PackageUpdateTest =
PackageUpdateTest::new("web").dev("web#dev", "mfe-config-pkg#proxy");
const DEFAULT_APP_PROXY: PackageUpdateTest =
PackageUpdateTest::new("mfe-docs").dev("mfe-docs#serve", "mfe-web#proxy");
const DEFAULT_APP_PROXY_AND_DEV: PackageUpdateTest =
PackageUpdateTest::new("mfe-web").dev("mfe-web#dev", "mfe-web#proxy");

#[test_case(NON_MFE_PKG)]
#[test_case(MFE_CONFIG_PKG)]
#[test_case(MFE_CONFIG_PKG_DEV_TASK)]
#[test_case(DEFAULT_APP_PROXY)]
#[test_case(DEFAULT_APP_PROXY_AND_DEV)]
fn test_package_turbo_json_update(test: PackageUpdateTest) {
let configs = mfe_configs!(
"mfe-config-pkg" => ["web#dev", "docs#dev"],
"mfe-web" => ["mfe-web#dev", "mfe-docs#serve"]
);
let mfe = MicroFrontendsConfigs {
configs,
mfe_package: None,
};
assert_eq!(
mfe.package_turbo_json_update(&test.package_name()),
test.expected()
);
}
}
Loading

0 comments on commit 4f5f048

Please sign in to comment.