Skip to content

Commit

Permalink
feat: error on empty package name (#8152)
Browse files Browse the repository at this point in the history
### Description

We now will error if we encounter a package with a missing or empty
package name.

This is done in the graph validation step as opposed to construction
since we don't want to impose this restriction on `@turbo/repository`
users.

### Testing Instructions

Added integration test
  • Loading branch information
chris-olszewski committed May 22, 2024
1 parent 6373a42 commit 79026a2
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 9 deletions.
4 changes: 4 additions & 0 deletions crates/turborepo-repository/src/package_graph/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,7 @@ impl<'a, T: PackageDiscovery> BuildState<'a, ResolvedPackageManager, T> {
node_lookup,
lockfile,
package_discovery,
repo_root,
..
} = self;

Expand All @@ -337,6 +338,7 @@ impl<'a, T: PackageDiscovery> BuildState<'a, ResolvedPackageManager, T> {
packages: workspaces,
lockfile,
package_manager,
repo_root: repo_root.to_owned(),
})
}
}
Expand Down Expand Up @@ -536,6 +538,7 @@ impl<'a, T: PackageDiscovery> BuildState<'a, ResolvedLockfile, T> {
workspace_graph,
node_lookup,
lockfile,
repo_root,
..
} = self;
Ok(PackageGraph {
Expand All @@ -544,6 +547,7 @@ impl<'a, T: PackageDiscovery> BuildState<'a, ResolvedLockfile, T> {
packages: workspaces,
package_manager,
lockfile,
repo_root: repo_root.to_owned(),
})
}
}
Expand Down
37 changes: 28 additions & 9 deletions crates/turborepo-repository/src/package_graph/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ use std::{

use petgraph::visit::{depth_first_search, Reversed};
use serde::Serialize;
use turbopath::{AbsoluteSystemPath, AnchoredSystemPath, AnchoredSystemPathBuf};
use turbopath::{
AbsoluteSystemPath, AbsoluteSystemPathBuf, AnchoredSystemPath, AnchoredSystemPathBuf,
};
use turborepo_graph_utils as graph;
use turborepo_lockfiles::Lockfile;

Expand All @@ -30,6 +32,7 @@ pub struct PackageGraph {
packages: HashMap<PackageName, PackageInfo>,
package_manager: PackageManager,
lockfile: Option<Box<dyn Lockfile>>,
repo_root: AbsoluteSystemPathBuf,
}

/// The WorkspacePackage follows the Vercel glossary of terms where "Workspace"
Expand Down Expand Up @@ -128,7 +131,16 @@ impl PackageGraph {

#[tracing::instrument(skip(self))]
pub fn validate(&self) -> Result<(), Error> {
graph::validate_graph(&self.graph).map_err(Error::InvalidPackageGraph)
for info in self.packages.values() {
let name = info.package_json.name.as_deref();
if matches!(name, None | Some("")) {
let package_json_path = self.repo_root.resolve(info.package_json_path());
return Err(Error::PackageJsonMissingName(package_json_path));
}
}
graph::validate_graph(&self.graph).map_err(Error::InvalidPackageGraph)?;

Ok(())
}

pub fn remove_package_dependencies(&mut self) {
Expand Down Expand Up @@ -481,17 +493,24 @@ mod test {
async fn test_single_package_is_depends_on_root() {
let root =
AbsoluteSystemPathBuf::new(if cfg!(windows) { r"C:\repo" } else { "/repo" }).unwrap();
let pkg_graph = PackageGraph::builder(&root, PackageJson::default())
.with_package_discovery(MockDiscovery)
.with_single_package_mode(true)
.build()
.await
.unwrap();
let pkg_graph = PackageGraph::builder(
&root,
PackageJson {
name: Some("my-package".to_owned()),
..Default::default()
},
)
.with_package_discovery(MockDiscovery)
.with_single_package_mode(true)
.build()
.await
.unwrap();

let closure =
pkg_graph.transitive_closure(Some(&PackageNode::Workspace(PackageName::Root)));
assert!(closure.contains(&PackageNode::Root));
assert!(pkg_graph.validate().is_ok());
let result = pkg_graph.validate();
assert!(result.is_ok(), "expected ok {:?}", result);
}

#[tokio::test]
Expand Down
9 changes: 9 additions & 0 deletions turborepo-tests/integration/tests/invalid-package-json.t
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
Setup
$ . ${TESTDIR}/../../helpers/setup_integration_test.sh
Clear name field
$ jq '.name = ""' apps/my-app/package.json > package.json.new
$ mv package.json.new apps/my-app/package.json
Build should fail due to missing name field
$ ${TURBO} build 1> ERR
[1]
$ grep -F --quiet 'x package.json must have a name field:' ERR

0 comments on commit 79026a2

Please sign in to comment.