Skip to content
This repository was archived by the owner on Sep 17, 2023. It is now read-only.

Commit 5390137

Browse files
committed
refactor!: use impls to reduce duplicated code
This commit refactors the entire codebase, using less of a `C` style and moves hopefully towards a more idiomatic Rust style. The motivations for the refactor were to reduce duplicated code (pulling code away from the call-site and into shared `impl`s) and to improve the readability in the implementations of each sub-command. This commit includes minor performance optimizations, namely through reduced cloning, leading to ~10% faster runtimes on the `pin` and `link` sub-commands. Non-Breaking Changes ==================== crate `regex` has been replaced with `rsplit_once`, bringing runtime and compile-time improvements. This commit builds on the findings of the previous commit, in which use of the top-level `dist` directory introduced a race condition when invoking multiple make targets in parallel. After refactoring to avoid the race condition, it became apparent that the top-level `dist` directory was reduced to a pointless indirection. We reduce complexity in this commit by entirely removing the top-level `dist` directory as an endpoint for `npm pack`-ed archives. Instead, we let `npm pack` place each archive directly in the package directory containing that package's source code, and then hardlink the archive into the required dependent package's `.internal-npm-dependencies` directory. This implementation removes the desire to use npm 7's `--pack-destination` flag, which means we will not have to eventually break support for npm 6 and earlier. Breaking changes ================ BREAKING CHANGE: drop support for the `--ignore` flag We drop support for the `--ignore` flag as it was poorly documented, not widely used, and adding a bit of complexity in the code that wasn't justified by the utility it added. Future Work =========== While this refactor does bring some marginal performance improvements, we will need to break out of the nearby local maximum by refactoring again to utilize multiple CPU cores. This refactor will again have wide-reaching diffs and implications, so we shouldn't spend too much time with the current architecture and instead look forward to a more parallel-iterators based approach. We can map-reduce reading of the package manifests and then work with the data once all results are in. Each sub-command currently runs in about 80 milliseconds on my humble i7 for a 60-package project, so I do not anticipate that this refactor will be required on a wall-clock basis any time soon.
1 parent f894194 commit 5390137

15 files changed

+638
-717
lines changed

Cargo.lock

Lines changed: 0 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ askama = "0.11"
1212
clap = { version = "3.1.1", features = ["cargo", "derive"] }
1313
globwalk = "0.8.1"
1414
pathdiff = "0.2.1"
15-
regex = { version = "1.5", default-features = false, features = ["std", "perf"] }
1615
serde = { version = "1.0", features = ["derive"] }
1716
serde_json = { version = "1.0", features = ["preserve_order"] }
1817

src/configuration_file.rs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
use std::error::Error;
2+
use std::path::{Path, PathBuf};
3+
4+
/// Configuration file for some component of the monorepo.
5+
pub trait ConfigurationFile<T> {
6+
/// Basename of the configuration file.
7+
const FILENAME: &'static str;
8+
9+
/// Create an instance of this configuration file by reading
10+
/// the specified file from this directory on disk.
11+
fn from_directory<P>(monorepo_root: P, directory: P) -> Result<T, Box<dyn Error>>
12+
where
13+
P: AsRef<Path>;
14+
15+
/// Relative path to directory containing this configuration file,
16+
/// from monorepo root.
17+
fn directory(&self) -> PathBuf;
18+
19+
/// Relative path to this configuration file from the monorepo root.
20+
fn path(&self) -> PathBuf;
21+
22+
/// Write this configuration file to disk.
23+
fn write(&self) -> Result<(), Box<dyn Error>>;
24+
}

src/dependencies.rs

Lines changed: 0 additions & 136 deletions
This file was deleted.

src/io.rs

Lines changed: 7 additions & 128 deletions
Original file line numberDiff line numberDiff line change
@@ -1,91 +1,25 @@
1-
use std::collections::HashMap;
21
use std::error::Error;
32
use std::fs::File;
4-
use std::io::{BufReader, BufWriter, Write};
5-
use std::path::{Path, PathBuf};
6-
7-
use globwalk::{FileType, GlobWalkerBuilder};
3+
use std::io::{BufWriter, Write};
4+
use std::path::Path;
85

96
use serde::{Deserialize, Serialize};
107
use serde_json;
11-
use serde_json::Value;
12-
13-
#[derive(Serialize, Deserialize, Debug)]
14-
pub struct LernaManifest {
15-
pub version: String,
16-
pub packages: Vec<String>,
17-
}
18-
19-
#[derive(Serialize, Deserialize, Debug)]
20-
#[serde(rename_all = "camelCase")]
21-
pub struct PackageManifest {
22-
pub name: String,
23-
pub version: String,
24-
#[serde(flatten)]
25-
pub extra_fields: Value,
26-
}
278

28-
#[derive(Serialize, Deserialize, PartialEq)]
29-
pub struct TypeScriptProjectReference {
9+
#[derive(Serialize, Deserialize, Debug, PartialEq)]
10+
pub struct TypescriptProjectReference {
3011
pub path: String,
3112
}
3213

3314
#[derive(Serialize, PartialEq)]
34-
pub struct TypeScriptParentProjectReferences {
15+
pub struct TypescriptParentProjectReference {
3516
pub files: Vec<String>,
36-
pub references: Vec<TypeScriptProjectReference>,
37-
}
38-
39-
pub fn read_lerna_manifest<P: AsRef<Path>>(root: P) -> Result<LernaManifest, Box<dyn Error>> {
40-
let file = File::open(root.as_ref().join("lerna.json"))?;
41-
let reader = BufReader::new(file);
42-
let u = serde_json::from_reader(reader)?;
43-
Ok(u)
44-
}
45-
46-
pub fn read_package_manifest<P: AsRef<Path>>(
47-
manifest: P,
48-
) -> Result<PackageManifest, Box<dyn Error>> {
49-
let file = File::open(manifest)?;
50-
let reader = BufReader::new(file);
51-
let u = serde_json::from_reader(reader)?;
52-
Ok(u)
53-
}
54-
55-
pub fn write_package_manifest<P: AsRef<Path>>(
56-
path: P,
57-
manifest: &PackageManifest,
58-
) -> Result<(), Box<dyn Error>> {
59-
let file = File::create(path)?;
60-
let mut writer = BufWriter::new(file);
61-
serde_json::to_writer_pretty(&mut writer, manifest)?;
62-
writer.write_all(b"\n")?;
63-
writer.flush()?;
64-
Ok(())
65-
}
66-
67-
pub fn read_tsconfig<P: AsRef<Path>>(path: P) -> Result<serde_json::Value, Box<dyn Error>> {
68-
let file = File::open(path)?;
69-
let reader = BufReader::new(file);
70-
let u = serde_json::from_reader(reader)?;
71-
Ok(u)
72-
}
73-
74-
pub fn write_tsconfig<P: AsRef<Path>>(
75-
path: P,
76-
tsconfig: &serde_json::Value,
77-
) -> Result<(), Box<dyn Error>> {
78-
let file = File::create(path)?;
79-
let mut writer = BufWriter::new(file);
80-
serde_json::to_writer_pretty(&mut writer, tsconfig)?;
81-
writer.write_all(b"\n")?;
82-
writer.flush()?;
83-
Ok(())
17+
pub references: Vec<TypescriptProjectReference>,
8418
}
8519

8620
pub fn write_project_references<P: AsRef<Path>>(
8721
path: P,
88-
references: &TypeScriptParentProjectReferences,
22+
references: &TypescriptParentProjectReference,
8923
) -> Result<(), Box<dyn Error>> {
9024
let file = File::create(&path)?;
9125
let mut writer = BufWriter::new(file);
@@ -94,58 +28,3 @@ pub fn write_project_references<P: AsRef<Path>>(
9428
writer.flush()?;
9529
Ok(())
9630
}
97-
98-
pub fn get_internal_package_manifest_files<P: AsRef<Path>>(
99-
root: P,
100-
lerna_manifest: &LernaManifest,
101-
ignore_globs: &Vec<String>,
102-
) -> Result<Vec<PathBuf>, Box<dyn Error>> {
103-
// dawid's tip: consider rayon for parallel iterators
104-
105-
let mut package_manifests: Vec<String> = lerna_manifest
106-
.packages
107-
.iter()
108-
.map(|package_manifest_glob| {
109-
Path::new(package_manifest_glob)
110-
.join("package.json")
111-
.to_str()
112-
.expect("Path not valid UTF-8")
113-
.to_string()
114-
})
115-
// dawid's tip: return an iterator from this function, avoid collecting
116-
.collect();
117-
118-
// ignore paths to speed up file-system walk
119-
for glob in ignore_globs {
120-
package_manifests.push(glob.to_string());
121-
}
122-
package_manifests.push("!node_modules/".to_string());
123-
124-
let manifest_files = GlobWalkerBuilder::from_patterns(&root, &package_manifests)
125-
.file_type(FileType::FILE)
126-
.min_depth(1)
127-
.build()
128-
.expect("Unable to create glob")
129-
.into_iter()
130-
.filter_map(Result::ok)
131-
.map(|dir_entry| dir_entry.into_path())
132-
.collect();
133-
134-
Ok(manifest_files)
135-
}
136-
137-
pub fn read_internal_package_manifests<P: AsRef<Path>>(
138-
internal_package_manifest_files: &Vec<P>,
139-
) -> Result<HashMap<PathBuf, PackageManifest>, Box<dyn Error>> {
140-
let mut package_manifests = HashMap::new();
141-
142-
for manifest_file in internal_package_manifest_files {
143-
let package_manifest_contents = read_package_manifest(manifest_file)?;
144-
package_manifests.insert(
145-
manifest_file.as_ref().to_path_buf(),
146-
package_manifest_contents,
147-
);
148-
}
149-
150-
Ok(package_manifests)
151-
}

0 commit comments

Comments
 (0)