From 15f848d43e18e081f54731445658bfcebfb4cb61 Mon Sep 17 00:00:00 2001 From: FrozenPandaz Date: Fri, 30 Aug 2024 18:13:05 -0400 Subject: [PATCH 1/7] chore(repo): wip --- .../nx/src/native/cache/expand_outputs.rs | 23 ++++++++++++++----- .../native/plugins/js/ts_import_locators.rs | 2 +- packages/nx/src/native/walker.rs | 19 +++++++++------ .../nx/src/native/workspace/files_hashing.rs | 4 ++-- 4 files changed, 32 insertions(+), 16 deletions(-) diff --git a/packages/nx/src/native/cache/expand_outputs.rs b/packages/nx/src/native/cache/expand_outputs.rs index 6a91769831a9e..ccd9e5e78dafe 100644 --- a/packages/nx/src/native/cache/expand_outputs.rs +++ b/packages/nx/src/native/cache/expand_outputs.rs @@ -2,8 +2,9 @@ use std::path::{Path, PathBuf}; use tracing::trace; use crate::native::glob::{build_glob_set, contains_glob_pattern}; +use crate::native::logger::enable_logger; use crate::native::utils::Normalize; -use crate::native::walker::nx_walker_sync; +use crate::native::walker::{nx_walker, nx_walker_sync}; #[napi] pub fn expand_outputs(directory: String, entries: Vec) -> anyhow::Result> { @@ -77,6 +78,8 @@ pub fn get_files_for_outputs( directory: String, entries: Vec, ) -> anyhow::Result> { + enable_logger(); + let directory: PathBuf = directory.into(); let mut globs: Vec = vec![]; @@ -86,7 +89,9 @@ pub fn get_files_for_outputs( let path = directory.join(&entry); if !path.exists() { - globs.push(entry); + if contains_glob_pattern(&entry) { + globs.push(entry); + } } else if path.is_dir() { directories.push(entry); } else { @@ -96,14 +101,20 @@ pub fn get_files_for_outputs( if !globs.is_empty() { // todo(jcammisuli): optimize this as nx_walker_sync is very slow on the root directory. We need to change this to only search smaller directories + // Traverse up the path to find a directory + // Iterate through the globs and boil it down to directories (hopefully not the root) to walk + // Disallow {workspaceRoot}/**/dist/apps/*.txt because it would be slow. Throw an error. + // Use the is_glob() function on the parent until it is not a glob instead of exists + let glob_set = build_glob_set(&globs)?; - let found_paths = nx_walker_sync(&directory, None).filter_map(|path| { - if glob_set.is_match(&path) { - Some(path.to_normalized_string()) + trace!("walking directory: {:?}", directory); + let found_paths: Vec = nx_walker(&directory, false).filter_map(|file| { + if glob_set.is_match(&file.full_path) { + Some(file.normalized_path) } else { None } - }); + }).collect(); files.extend(found_paths); } diff --git a/packages/nx/src/native/plugins/js/ts_import_locators.rs b/packages/nx/src/native/plugins/js/ts_import_locators.rs index 109e03dafee4f..8a0ac034135db 100644 --- a/packages/nx/src/native/plugins/js/ts_import_locators.rs +++ b/packages/nx/src/native/plugins/js/ts_import_locators.rs @@ -1480,7 +1480,7 @@ import(myTag`react@${version}`); let root = PathBuf::from(ancestors.next().unwrap()); let glob = build_glob_set(&["**/*.[jt]s"]).unwrap(); - let files = nx_walker(root.clone()) + let files = nx_walker(root.clone(), true) .filter(|file| glob.is_match(&file.full_path)) .map(|file| file.full_path) .collect::>(); diff --git a/packages/nx/src/native/walker.rs b/packages/nx/src/native/walker.rs index 5375f6de794b4..1fc5f6a5ec47c 100644 --- a/packages/nx/src/native/walker.rs +++ b/packages/nx/src/native/walker.rs @@ -6,6 +6,7 @@ use crate::native::glob::build_glob_set; use crate::native::utils::{get_mod_time, Normalize}; use walkdir::WalkDir; +use crate::native::logger::enable_logger; #[derive(PartialEq, Debug, Ord, PartialOrd, Eq, Clone)] pub struct NxFile { @@ -93,7 +94,7 @@ where /// Walk the directory and ignore files from .gitignore and .nxignore #[cfg(not(target_arch = "wasm32"))] -pub fn nx_walker

(directory: P) -> impl Iterator +pub fn nx_walker

(directory: P, use_ignores: bool) -> impl Iterator where P: AsRef, { @@ -102,9 +103,10 @@ where use crossbeam_channel::unbounded; use tracing::trace; + enable_logger(); let directory = directory.as_ref(); - let mut walker = create_walker(directory); + let mut walker = create_walker(directory, use_ignores); let cpus = available_parallelism().map_or(2, |n| n.get()) - 1; @@ -151,7 +153,7 @@ where receiver_thread.join().unwrap() } -fn create_walker

(directory: P) -> WalkBuilder +fn create_walker

(directory: P, use_ignores: bool) -> WalkBuilder where P: AsRef { @@ -169,7 +171,10 @@ where let mut walker = WalkBuilder::new(&directory); walker.require_git(false); walker.hidden(false); - walker.add_custom_ignore_filename(".nxignore"); + walker.git_ignore(!use_ignores); + if use_ignores { + walker.add_custom_ignore_filename(".nxignore"); + } // We should make sure to always ignore node_modules and the .git folder walker.filter_entry(move |entry| { @@ -210,12 +215,12 @@ mod test { #[test] fn it_walks_a_directory() { // handle empty workspaces - let content = nx_walker("/does/not/exist").collect::>(); + let content = nx_walker("/does/not/exist", true).collect::>(); assert!(content.is_empty()); let temp_dir = setup_fs(); - let mut content = nx_walker(&temp_dir).collect::>(); + let mut content = nx_walker(&temp_dir, true).collect::>(); content.sort(); let content = content .into_iter() @@ -282,7 +287,7 @@ nested/child-two/ ) .unwrap(); - let mut file_names = nx_walker(temp_dir) + let mut file_names = nx_walker(temp_dir, true) .map( |NxFile { normalized_path: relative_path, diff --git a/packages/nx/src/native/workspace/files_hashing.rs b/packages/nx/src/native/workspace/files_hashing.rs index b4f836a99e0c1..087d4cedd43fa 100644 --- a/packages/nx/src/native/workspace/files_hashing.rs +++ b/packages/nx/src/native/workspace/files_hashing.rs @@ -10,7 +10,7 @@ use crate::native::walker::{nx_walker, NxFile}; use crate::native::workspace::files_archive::{NxFileHashed, NxFileHashes}; pub fn full_files_hash(workspace_root: &Path) -> NxFileHashes { - let files = nx_walker(workspace_root).collect::>(); + let files = nx_walker(workspace_root, true).collect::>(); trace!("Found {} files", files.len()); hash_files(files).into_iter().collect() } @@ -19,7 +19,7 @@ pub fn selective_files_hash( workspace_root: &Path, mut archived_files: NxFileHashes, ) -> NxFileHashes { - let files = nx_walker(workspace_root).collect::>(); + let files = nx_walker(workspace_root, true).collect::>(); let mut archived = vec![]; let mut not_archived = vec![]; let now = std::time::Instant::now(); From d931e759a8f68d9e1e9d26246394e0a519a30e96 Mon Sep 17 00:00:00 2001 From: FrozenPandaz Date: Tue, 3 Sep 2024 17:49:53 -0400 Subject: [PATCH 2/7] fix(misc): fix --- packages/nx/src/native/walker.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/nx/src/native/walker.rs b/packages/nx/src/native/walker.rs index 1fc5f6a5ec47c..15c9c91870639 100644 --- a/packages/nx/src/native/walker.rs +++ b/packages/nx/src/native/walker.rs @@ -171,7 +171,7 @@ where let mut walker = WalkBuilder::new(&directory); walker.require_git(false); walker.hidden(false); - walker.git_ignore(!use_ignores); + walker.git_ignore(use_ignores); if use_ignores { walker.add_custom_ignore_filename(".nxignore"); } From 818fbd58bef2d36e54e09f010744c5de29400125 Mon Sep 17 00:00:00 2001 From: Craigory Coppola Date: Wed, 4 Sep 2024 14:08:41 -0400 Subject: [PATCH 3/7] fix(core): optimize glob output resolution time --- .../update-15-0-0/prefix-outputs.ts | 36 +++++--- .../nx/src/native/cache/expand_outputs.rs | 82 +++++++++++++++---- packages/nx/src/native/cache/mod.rs | 3 +- .../nx/src/native/cache/validate_outputs.rs | 65 +++++++++++++++ packages/nx/src/native/glob.rs | 2 + packages/nx/src/native/glob/glob_transform.rs | 69 +++++++++++++++- packages/nx/src/native/index.d.ts | 4 + packages/nx/src/native/native-bindings.js | 2 + packages/nx/src/native/walker.rs | 10 +-- packages/nx/src/tasks-runner/utils.spec.ts | 73 ++++++++++------- packages/nx/src/tasks-runner/utils.ts | 19 ++--- 11 files changed, 285 insertions(+), 80 deletions(-) create mode 100644 packages/nx/src/native/cache/validate_outputs.rs diff --git a/packages/nx/src/migrations/update-15-0-0/prefix-outputs.ts b/packages/nx/src/migrations/update-15-0-0/prefix-outputs.ts index f1cf4da50eeea..fa7f2d7bb0312 100644 --- a/packages/nx/src/migrations/update-15-0-0/prefix-outputs.ts +++ b/packages/nx/src/migrations/update-15-0-0/prefix-outputs.ts @@ -13,6 +13,7 @@ import { } from '../../tasks-runner/utils'; import { updateJson } from '../../generators/utils/json'; import { PackageJson } from '../../utils/package-json'; +import { getTransformableOutputs } from 'nx/src/native'; export default async function (tree: Tree) { // If the workspace doesn't have a nx.json, don't make any changes @@ -28,10 +29,13 @@ export default async function (tree: Tree) { continue; } - try { - validateOutputs(target.outputs); - } catch (e) { - target.outputs = transformLegacyOutputs(project.root, e); + const transformableOutputs = getTransformableOutputs(target.outputs); + if (transformableOutputs.length) { + target.outputs = transformLegacyOutputs( + project.root, + target.outputs, + new Set(transformableOutputs) + ); } } try { @@ -44,10 +48,15 @@ export default async function (tree: Tree) { (json) => { for (const target of Object.values(json.nx?.targets ?? {})) { if (target.outputs) { - try { - validateOutputs(target.outputs); - } catch (e) { - target.outputs = transformLegacyOutputs(project.root, e); + const transformableOutputs = getTransformableOutputs( + target.outputs + ); + if (transformableOutputs.length) { + target.outputs = transformLegacyOutputs( + project.root, + target.outputs, + new Set(transformableOutputs) + ); } } } @@ -64,10 +73,13 @@ export default async function (tree: Tree) { if (!target.outputs) { continue; } - try { - validateOutputs(target.outputs); - } catch (e: any) { - target.outputs = transformLegacyOutputs('{projectRoot}', e); + const transformableOutputs = getTransformableOutputs(target.outputs); + if (transformableOutputs.length) { + target.outputs = transformLegacyOutputs( + '{projectRoot}', + target.outputs, + new Set(transformableOutputs) + ); } } diff --git a/packages/nx/src/native/cache/expand_outputs.rs b/packages/nx/src/native/cache/expand_outputs.rs index ccd9e5e78dafe..6b6410e69ba14 100644 --- a/packages/nx/src/native/cache/expand_outputs.rs +++ b/packages/nx/src/native/cache/expand_outputs.rs @@ -1,7 +1,8 @@ +use hashbrown::HashMap; use std::path::{Path, PathBuf}; use tracing::trace; -use crate::native::glob::{build_glob_set, contains_glob_pattern}; +use crate::native::glob::{build_glob_set, contains_glob_pattern, partition_glob}; use crate::native::logger::enable_logger; use crate::native::utils::Normalize; use crate::native::walker::{nx_walker, nx_walker_sync}; @@ -71,6 +72,22 @@ where Ok(found_paths) } +fn partition_globs_into_map(globs: Vec) -> HashMap> { + globs + .iter() + .map(|glob| partition_glob(glob)) + // Right now we have an iterator where each item is (root: String, patterns: String[]). + // We want a singular root, with the patterns mapped to it. + .fold( + HashMap::>::new(), + |mut map, (root, patterns)| { + let entry = map.entry(root).or_insert(vec![]); + entry.extend(patterns); + map + }, + ) +} + #[napi] /// Expands the given outputs into a list of existing files. /// This is used when hashing outputs @@ -100,23 +117,34 @@ pub fn get_files_for_outputs( } if !globs.is_empty() { - // todo(jcammisuli): optimize this as nx_walker_sync is very slow on the root directory. We need to change this to only search smaller directories - // Traverse up the path to find a directory - // Iterate through the globs and boil it down to directories (hopefully not the root) to walk - // Disallow {workspaceRoot}/**/dist/apps/*.txt because it would be slow. Throw an error. - // Use the is_glob() function on the parent until it is not a glob instead of exists - - let glob_set = build_glob_set(&globs)?; - trace!("walking directory: {:?}", directory); - let found_paths: Vec = nx_walker(&directory, false).filter_map(|file| { - if glob_set.is_match(&file.full_path) { - Some(file.normalized_path) - } else { - None - } - }).collect(); + let partitioned_globs = partition_globs_into_map(globs); + println!("partitioned_globs: {:?}", partitioned_globs); + for (root, patterns) in partitioned_globs { + let root_path = directory.join(&root); + let glob_set = build_glob_set(&patterns)?; + trace!("walking directory: {:?}", root_path); + println!("walking directory: {:?}", root_path); + + let found_paths: Vec = nx_walker(&root_path, false) + .filter_map(|file| { + println!("file: {}", &file.normalized_path); + println!("matches: {}", glob_set.is_match(&file.normalized_path)); + if glob_set.is_match(&file.normalized_path) { + Some( + // root_path contains full directory, + // root is only the leading dirs from glob + PathBuf::from(&root) + .join(&file.normalized_path) + .to_normalized_string(), + ) + } else { + None + } + }) + .collect(); - files.extend(found_paths); + files.extend(found_paths); + } } if !directories.is_empty() { @@ -232,4 +260,24 @@ mod test { ] ); } + + #[test] + fn should_get_files_for_outputs_with_glob() { + let temp = setup_fs(); + let entries = vec![ + "packages/nx/src/native/*.node".to_string(), + "folder/nested-folder".to_string(), + "test.txt".to_string(), + ]; + let mut result = get_files_for_outputs(temp.display().to_string(), entries).unwrap(); + result.sort(); + assert_eq!( + result, + vec![ + "folder/nested-folder", + "packages/nx/src/native/nx.darwin-arm64.node", + "test.txt" + ] + ); + } } diff --git a/packages/nx/src/native/cache/mod.rs b/packages/nx/src/native/cache/mod.rs index 76c528ada9de2..e682df36301fb 100644 --- a/packages/nx/src/native/cache/mod.rs +++ b/packages/nx/src/native/cache/mod.rs @@ -1,3 +1,4 @@ +pub mod cache; pub mod expand_outputs; pub mod file_ops; -pub mod cache; +pub mod validate_outputs; diff --git a/packages/nx/src/native/cache/validate_outputs.rs b/packages/nx/src/native/cache/validate_outputs.rs new file mode 100644 index 0000000000000..72773ae688725 --- /dev/null +++ b/packages/nx/src/native/cache/validate_outputs.rs @@ -0,0 +1,65 @@ +use itertools::Itertools; +use regex::Regex; + +use crate::native::glob::{contains_glob_pattern, partition_glob}; + +const ALLOWED_WORKSPACE_ROOT_OUTPUT_PREFIXES: [&str; 2] = ["!{workspaceRoot}", "{workspaceRoot}"]; + +fn is_missing_prefix(output: &str) -> bool { + let re = Regex::new(r"^!?\{[\s\S]+\}").expect("Output pattern regex should compile"); + + !re.is_match(output) +} + +#[napi] +pub fn validate_outputs(outputs: Vec) -> anyhow::Result<()> { + let (missing_prefix, workspace_globs) = outputs.iter().fold( + (vec![], vec![]), + |(mut missing_prefix, mut workspace_globs), output| { + if is_missing_prefix(output) { + missing_prefix.push(output); + } + + for prefix in ALLOWED_WORKSPACE_ROOT_OUTPUT_PREFIXES.iter() { + if let Some(trimmed) = output.strip_prefix(prefix) { + if contains_glob_pattern(&trimmed) { + let (root, _) = partition_glob(&trimmed); + if root.is_empty() { + workspace_globs.push(output); + } + } + } + } + + (missing_prefix, workspace_globs) + }, + ); + + if workspace_globs.is_empty() && missing_prefix.is_empty() { + return Ok(()); + } + + let mut error_message = String::new(); + if !missing_prefix.is_empty() { + error_message.push_str(&format!( + "The following outputs are invalid: \n - {}\n\nRun `nx repair` to fix this.", + missing_prefix.iter().join("\n - ") + )); + } + if !workspace_globs.is_empty() { + error_message.push_str(&format!( + "The following outputs are defined by a glob pattern from the workspace root: \n - {}\n\nThese can be slow, replace them with a more specific pattern.", + workspace_globs.iter().join("\n - ") + )); + } + + Err(anyhow::anyhow!(error_message)) +} + +#[napi] +pub fn get_transformable_outputs(outputs: Vec) -> Vec { + outputs + .into_iter() + .filter(|output| is_missing_prefix(output)) + .collect() +} diff --git a/packages/nx/src/native/glob.rs b/packages/nx/src/native/glob.rs index 16eab6b07b947..4cd723d7544a2 100644 --- a/packages/nx/src/native/glob.rs +++ b/packages/nx/src/native/glob.rs @@ -123,6 +123,8 @@ pub(crate) fn contains_glob_pattern(value: &str) -> bool { || value.contains(')') } +pub(crate) use glob_transform::partition_glob; + #[cfg(test)] mod test { use super::*; diff --git a/packages/nx/src/native/glob/glob_transform.rs b/packages/nx/src/native/glob/glob_transform.rs index 9ebd7e303bd32..810332839df73 100644 --- a/packages/nx/src/native/glob/glob_transform.rs +++ b/packages/nx/src/native/glob/glob_transform.rs @@ -3,14 +3,15 @@ use crate::native::glob::glob_parser::parse_glob; use itertools::Itertools; use std::collections::HashSet; +use super::contains_glob_pattern; + #[derive(Debug)] enum GlobType { Negative(String), Positive(String), } -pub fn convert_glob(glob: &str) -> anyhow::Result> { - let (negated, parsed) = parse_glob(glob)?; +fn convert_glob_segments(negated: bool, parsed: Vec>) -> Vec { let mut built_segments: Vec> = Vec::new(); for (index, glob_segment) in parsed.iter().enumerate() { let is_last = index == parsed.len() - 1; @@ -58,7 +59,12 @@ pub fn convert_glob(glob: &str) -> anyhow::Result> { .into_iter() .collect::>(); globs.sort(); - Ok(globs) + globs +} + +pub fn convert_glob(glob: &str) -> anyhow::Result> { + let (negated, parsed) = parse_glob(glob)?; + Ok(convert_glob_segments(negated, parsed)) } fn build_segment( @@ -107,6 +113,42 @@ fn build_segment( } } +fn remove_first(vec: &mut Vec) -> Option { + if vec.is_empty() { + return None; + } + Some(vec.remove(0)) +} + +pub fn partition_glob(glob: &str) -> (String, Vec) { + let (negated, mut groups) = parse_glob(glob).unwrap(); + // Partition glob into leading directories and patterns that should be matched + let mut leading_dir_segments: Vec = vec![]; + let mut pattern_segments = vec![]; + while let Some(group) = remove_first(&mut groups) { + if group.is_empty() { + continue; + } + match &group[0] { + GlobGroup::NonSpecial(value) => { + if !contains_glob_pattern(&value) && pattern_segments.is_empty() { + leading_dir_segments.push(value.to_string()); + } else { + pattern_segments.push(group); + } + } + _ => { + pattern_segments.push(group); + } + } + } + + ( + leading_dir_segments.join("/"), + convert_glob_segments(negated, pattern_segments), + ) +} + #[cfg(test)] mod test { use super::convert_glob; @@ -233,4 +275,25 @@ mod test { ] ); } + + #[test] + fn should_partition_glob_with_leading_dirs() { + let (leading_dirs, globs) = super::partition_glob("dist/app/**/!(README|LICENSE).(js|ts)"); + assert_eq!(leading_dirs, "dist/app"); + assert_eq!(globs, ["!**/{README,LICENSE}.{js,ts}", "**/*.{js,ts}",]); + } + + #[test] + fn should_partition_glob_with_leading_dirs_and_simple_patterns() { + let (leading_dirs, globs) = super::partition_glob("dist/app/**/*.css"); + assert_eq!(leading_dirs, "dist/app"); + assert_eq!(globs, ["**/*.css"]); + } + + #[test] + fn should_partition_glob_with_leading_dirs_and_no_patterns() { + let (leading_dirs, globs) = super::partition_glob("dist/app/"); + assert_eq!(leading_dirs, "dist/app"); + assert_eq!(globs, [] as [String; 0]); + } } diff --git a/packages/nx/src/native/index.d.ts b/packages/nx/src/native/index.d.ts index 3c3c3e5bc20c2..2ab95f69f98a0 100644 --- a/packages/nx/src/native/index.d.ts +++ b/packages/nx/src/native/index.d.ts @@ -150,6 +150,8 @@ export declare export function getBinaryTarget(): string */ export declare export function getFilesForOutputs(directory: string, entries: Array): Array +export declare export function getTransformableOutputs(outputs: Array): Array + export declare export function hashArray(input: Array): string export interface HashDetails { @@ -263,6 +265,8 @@ export interface UpdatedWorkspaceFiles { externalReferences: NxWorkspaceFilesExternals } +export declare export function validateOutputs(outputs: Array): void + export interface WatchEvent { path: string type: EventType diff --git a/packages/nx/src/native/native-bindings.js b/packages/nx/src/native/native-bindings.js index 5a259bf472715..cd921e8e8710f 100644 --- a/packages/nx/src/native/native-bindings.js +++ b/packages/nx/src/native/native-bindings.js @@ -378,10 +378,12 @@ module.exports.expandOutputs = nativeBinding.expandOutputs module.exports.findImports = nativeBinding.findImports module.exports.getBinaryTarget = nativeBinding.getBinaryTarget module.exports.getFilesForOutputs = nativeBinding.getFilesForOutputs +module.exports.getTransformableOutputs = nativeBinding.getTransformableOutputs module.exports.hashArray = nativeBinding.hashArray module.exports.hashFile = nativeBinding.hashFile module.exports.IS_WASM = nativeBinding.IS_WASM module.exports.remove = nativeBinding.remove module.exports.testOnlyTransferFileMap = nativeBinding.testOnlyTransferFileMap module.exports.transferProjectGraph = nativeBinding.transferProjectGraph +module.exports.validateOutputs = nativeBinding.validateOutputs module.exports.WorkspaceErrors = nativeBinding.WorkspaceErrors diff --git a/packages/nx/src/native/walker.rs b/packages/nx/src/native/walker.rs index 15c9c91870639..3ae7f57368135 100644 --- a/packages/nx/src/native/walker.rs +++ b/packages/nx/src/native/walker.rs @@ -1,12 +1,12 @@ +use ignore::WalkBuilder; use std::fmt::Debug; use std::path::{Path, PathBuf}; -use ignore::{WalkBuilder}; use crate::native::glob::build_glob_set; +use crate::native::logger::enable_logger; use crate::native::utils::{get_mod_time, Normalize}; use walkdir::WalkDir; -use crate::native::logger::enable_logger; #[derive(PartialEq, Debug, Ord, PartialOrd, Eq, Clone)] pub struct NxFile { @@ -141,7 +141,7 @@ where normalized_path: file_path.to_normalized_string(), mod_time: get_mod_time(&metadata), }) - .ok(); + .ok(); Continue }) @@ -155,7 +155,7 @@ where fn create_walker

(directory: P, use_ignores: bool) -> WalkBuilder where - P: AsRef + P: AsRef, { let directory: PathBuf = directory.as_ref().into(); @@ -166,7 +166,7 @@ where "**/.nx/workspace-data", "**/.yarn/cache", ]) - .expect("These static ignores always build"); + .expect("These static ignores always build"); let mut walker = WalkBuilder::new(&directory); walker.require_git(false); diff --git a/packages/nx/src/tasks-runner/utils.spec.ts b/packages/nx/src/tasks-runner/utils.spec.ts index eb0bc01b54ffc..4d07e8d9bc35e 100644 --- a/packages/nx/src/tasks-runner/utils.spec.ts +++ b/packages/nx/src/tasks-runner/utils.spec.ts @@ -405,47 +405,32 @@ describe('utils', () => { describe('transformLegacyOutputs', () => { it('should prefix paths with {workspaceRoot}', () => { const outputs = ['dist']; - try { - validateOutputs(outputs); - } catch (e) { - const result = transformLegacyOutputs('myapp', e); - expect(result).toEqual(['{workspaceRoot}/dist']); - } - expect.assertions(1); + const result = transformLegacyOutputs( + 'myapp', + outputs, + new Set(['dist']) + ); + expect(result).toEqual(['{workspaceRoot}/dist']); }); it('should prefix unix-absolute paths with {workspaceRoot}', () => { const outputs = ['/dist']; - try { - validateOutputs(outputs); - } catch (e) { - const result = transformLegacyOutputs('myapp', e); - expect(result).toEqual(['{workspaceRoot}/dist']); - } - expect.assertions(1); + const result = transformLegacyOutputs('myapp', outputs, new Set(outputs)); + expect(result).toEqual(['{workspaceRoot}/dist']); }); }); it('should prefix relative paths with {projectRoot}', () => { - const outputs = ['/dist']; - try { - validateOutputs(outputs); - } catch (e) { - const result = transformLegacyOutputs('myapp', e); - expect(result).toEqual(['{workspaceRoot}/dist']); - } - expect.assertions(1); + const outputs = ['./dist']; + const result = transformLegacyOutputs('myapp', outputs, new Set(outputs)); + expect(result).toEqual(['{projectRoot}/dist']); }); it('should prefix paths within the project with {projectRoot}', () => { const outputs = ['myapp/dist']; - try { - validateOutputs(outputs); - } catch (e) { - const result = transformLegacyOutputs('myapp', e); - expect(result).toEqual(['{projectRoot}/dist']); - } - expect.assertions(1); + + const result = transformLegacyOutputs('myapp', outputs, new Set(outputs)); + expect(result).toEqual(['{projectRoot}/dist']); }); describe('expandDependencyConfigSyntaxSugar', () => { @@ -772,6 +757,36 @@ describe('utils', () => { "The 'outputs' field must contain only strings, but received types: [string, number, object, boolean, object, object]" ); }); + + it('throws an error if the output is a glob pattern from the workspace root', () => { + expect(() => validateOutputs(['{workspaceRoot}/**/dist/*.js'])) + .toThrowErrorMatchingInlineSnapshot(` + "The following outputs are defined by a glob pattern from the workspace root: + - {workspaceRoot}/**/dist/*.js + + These can be slow, replace them with a more specific pattern." + `); + }); + + it("shouldn't throw an error if the output is a glob pattern from the project root", () => { + expect(() => validateOutputs(['{projectRoot}/*.js'])).not.toThrow(); + }); + + it("shouldn't throw an error if the pattern is a glob based in a subdirectory of workspace root", () => { + expect(() => + validateOutputs(['{workspaceRoot}/dist/**/*.js']) + ).not.toThrow(); + }); + + it("throws an error if the output doesn't start with a prefix", () => { + expect(() => validateOutputs(['dist'])) + .toThrowErrorMatchingInlineSnapshot(` + "The following outputs are invalid: + - dist + + Run \`nx repair\` to fix this." + `); + }); }); }); diff --git a/packages/nx/src/tasks-runner/utils.ts b/packages/nx/src/tasks-runner/utils.ts index 938d80404a73d..5484ea9fc600e 100644 --- a/packages/nx/src/tasks-runner/utils.ts +++ b/packages/nx/src/tasks-runner/utils.ts @@ -18,6 +18,7 @@ import { readProjectsConfigurationFromProjectGraph } from '../project-graph/proj import { findMatchingProjects } from '../utils/find-matching-projects'; import { minimatch } from 'minimatch'; import { isGlobPattern } from '../utils/globs'; +import { validateOutputs as nativeValidateOutputs } from '../native'; export type NormalizedTargetDependencyConfig = TargetDependencyConfig & { projects: string[]; @@ -253,24 +254,16 @@ function assertOutputsAreValidType(outputs: unknown) { export function validateOutputs(outputs: string[]) { assertOutputsAreValidType(outputs); - const invalidOutputs = new Set(); - - for (const output of outputs) { - if (!/^!?{[\s\S]+}/.test(output)) { - invalidOutputs.add(output); - } - } - if (invalidOutputs.size > 0) { - throw new InvalidOutputsError(outputs, invalidOutputs); - } + nativeValidateOutputs(outputs); } export function transformLegacyOutputs( projectRoot: string, - error: InvalidOutputsError + outputs: string[], + transformableOutputs: Set ) { - return error.outputs.map((output) => { - if (!error.invalidOutputs.has(output)) { + return outputs.map((output) => { + if (!transformableOutputs.has(output)) { return output; } From 1c3b07d3e6a81faddbafda3992bf958d6805bcad Mon Sep 17 00:00:00 2001 From: Craigory Coppola Date: Wed, 4 Sep 2024 15:25:07 -0400 Subject: [PATCH 4/7] chore(core): code review --- .../update-15-0-0/prefix-outputs.ts | 29 +++---------------- .../nx/src/native/cache/expand_outputs.rs | 12 +++----- .../nx/src/native/cache/validate_outputs.rs | 2 +- packages/nx/src/native/glob.rs | 4 +-- packages/nx/src/native/glob/glob_transform.rs | 21 ++++---------- packages/nx/src/tasks-runner/utils.spec.ts | 12 +++----- packages/nx/src/tasks-runner/utils.ts | 15 ++++++---- 7 files changed, 29 insertions(+), 66 deletions(-) diff --git a/packages/nx/src/migrations/update-15-0-0/prefix-outputs.ts b/packages/nx/src/migrations/update-15-0-0/prefix-outputs.ts index fa7f2d7bb0312..3c5f281670bf8 100644 --- a/packages/nx/src/migrations/update-15-0-0/prefix-outputs.ts +++ b/packages/nx/src/migrations/update-15-0-0/prefix-outputs.ts @@ -13,7 +13,6 @@ import { } from '../../tasks-runner/utils'; import { updateJson } from '../../generators/utils/json'; import { PackageJson } from '../../utils/package-json'; -import { getTransformableOutputs } from 'nx/src/native'; export default async function (tree: Tree) { // If the workspace doesn't have a nx.json, don't make any changes @@ -29,14 +28,7 @@ export default async function (tree: Tree) { continue; } - const transformableOutputs = getTransformableOutputs(target.outputs); - if (transformableOutputs.length) { - target.outputs = transformLegacyOutputs( - project.root, - target.outputs, - new Set(transformableOutputs) - ); - } + target.outputs = transformLegacyOutputs(project.root, target.outputs); } try { updateProjectConfiguration(tree, projectName, project); @@ -48,16 +40,10 @@ export default async function (tree: Tree) { (json) => { for (const target of Object.values(json.nx?.targets ?? {})) { if (target.outputs) { - const transformableOutputs = getTransformableOutputs( + target.outputs = transformLegacyOutputs( + project.root, target.outputs ); - if (transformableOutputs.length) { - target.outputs = transformLegacyOutputs( - project.root, - target.outputs, - new Set(transformableOutputs) - ); - } } } @@ -73,14 +59,7 @@ export default async function (tree: Tree) { if (!target.outputs) { continue; } - const transformableOutputs = getTransformableOutputs(target.outputs); - if (transformableOutputs.length) { - target.outputs = transformLegacyOutputs( - '{projectRoot}', - target.outputs, - new Set(transformableOutputs) - ); - } + target.outputs = transformLegacyOutputs('{projectRoot}', target.outputs); } updateNxJson(tree, nxJson); diff --git a/packages/nx/src/native/cache/expand_outputs.rs b/packages/nx/src/native/cache/expand_outputs.rs index 6b6410e69ba14..516400eb43a3f 100644 --- a/packages/nx/src/native/cache/expand_outputs.rs +++ b/packages/nx/src/native/cache/expand_outputs.rs @@ -2,7 +2,7 @@ use hashbrown::HashMap; use std::path::{Path, PathBuf}; use tracing::trace; -use crate::native::glob::{build_glob_set, contains_glob_pattern, partition_glob}; +use crate::native::glob::{build_glob_set, contains_glob_pattern, glob_transform::partition_glob}; use crate::native::logger::enable_logger; use crate::native::utils::Normalize; use crate::native::walker::{nx_walker, nx_walker_sync}; @@ -118,17 +118,13 @@ pub fn get_files_for_outputs( if !globs.is_empty() { let partitioned_globs = partition_globs_into_map(globs); - println!("partitioned_globs: {:?}", partitioned_globs); for (root, patterns) in partitioned_globs { let root_path = directory.join(&root); let glob_set = build_glob_set(&patterns)?; trace!("walking directory: {:?}", root_path); - println!("walking directory: {:?}", root_path); let found_paths: Vec = nx_walker(&root_path, false) .filter_map(|file| { - println!("file: {}", &file.normalized_path); - println!("matches: {}", glob_set.is_match(&file.normalized_path)); if glob_set.is_match(&file.normalized_path) { Some( // root_path contains full directory, @@ -151,11 +147,11 @@ pub fn get_files_for_outputs( for dir in directories { let dir = PathBuf::from(dir); let dir_path = directory.join(&dir); - let files_in_dir = nx_walker_sync(&dir_path, None).filter_map(|e| { - let path = dir_path.join(&e); + let files_in_dir = nx_walker(&dir_path, false).filter_map(|e| { + let path = dir_path.join(&e.normalized_path); if path.is_file() { - Some(dir.join(e).to_normalized_string()) + Some(dir.join(e.normalized_path).to_normalized_string()) } else { None } diff --git a/packages/nx/src/native/cache/validate_outputs.rs b/packages/nx/src/native/cache/validate_outputs.rs index 72773ae688725..aa8a91684f93b 100644 --- a/packages/nx/src/native/cache/validate_outputs.rs +++ b/packages/nx/src/native/cache/validate_outputs.rs @@ -1,7 +1,7 @@ use itertools::Itertools; use regex::Regex; -use crate::native::glob::{contains_glob_pattern, partition_glob}; +use crate::native::glob::{contains_glob_pattern, glob_transform::partition_glob}; const ALLOWED_WORKSPACE_ROOT_OUTPUT_PREFIXES: [&str; 2] = ["!{workspaceRoot}", "{workspaceRoot}"]; diff --git a/packages/nx/src/native/glob.rs b/packages/nx/src/native/glob.rs index 4cd723d7544a2..d91ab482d294c 100644 --- a/packages/nx/src/native/glob.rs +++ b/packages/nx/src/native/glob.rs @@ -1,6 +1,6 @@ mod glob_group; mod glob_parser; -mod glob_transform; +pub mod glob_transform; use crate::native::glob::glob_transform::convert_glob; use globset::{GlobBuilder, GlobSet, GlobSetBuilder}; @@ -123,8 +123,6 @@ pub(crate) fn contains_glob_pattern(value: &str) -> bool { || value.contains(')') } -pub(crate) use glob_transform::partition_glob; - #[cfg(test)] mod test { use super::*; diff --git a/packages/nx/src/native/glob/glob_transform.rs b/packages/nx/src/native/glob/glob_transform.rs index 810332839df73..5672f8559b2ed 100644 --- a/packages/nx/src/native/glob/glob_transform.rs +++ b/packages/nx/src/native/glob/glob_transform.rs @@ -113,23 +113,15 @@ fn build_segment( } } -fn remove_first(vec: &mut Vec) -> Option { - if vec.is_empty() { - return None; - } - Some(vec.remove(0)) -} - pub fn partition_glob(glob: &str) -> (String, Vec) { - let (negated, mut groups) = parse_glob(glob).unwrap(); + let (negated, groups) = parse_glob(glob).unwrap(); // Partition glob into leading directories and patterns that should be matched let mut leading_dir_segments: Vec = vec![]; let mut pattern_segments = vec![]; - while let Some(group) = remove_first(&mut groups) { - if group.is_empty() { - continue; - } - match &group[0] { + groups + .into_iter() + .filter(|group| !group.is_empty()) + .for_each(|group| match &group[0] { GlobGroup::NonSpecial(value) => { if !contains_glob_pattern(&value) && pattern_segments.is_empty() { leading_dir_segments.push(value.to_string()); @@ -140,8 +132,7 @@ pub fn partition_glob(glob: &str) -> (String, Vec) { _ => { pattern_segments.push(group); } - } - } + }); ( leading_dir_segments.join("/"), diff --git a/packages/nx/src/tasks-runner/utils.spec.ts b/packages/nx/src/tasks-runner/utils.spec.ts index 4d07e8d9bc35e..a5d6bb7c3953b 100644 --- a/packages/nx/src/tasks-runner/utils.spec.ts +++ b/packages/nx/src/tasks-runner/utils.spec.ts @@ -405,31 +405,27 @@ describe('utils', () => { describe('transformLegacyOutputs', () => { it('should prefix paths with {workspaceRoot}', () => { const outputs = ['dist']; - const result = transformLegacyOutputs( - 'myapp', - outputs, - new Set(['dist']) - ); + const result = transformLegacyOutputs('myapp', outputs); expect(result).toEqual(['{workspaceRoot}/dist']); }); it('should prefix unix-absolute paths with {workspaceRoot}', () => { const outputs = ['/dist']; - const result = transformLegacyOutputs('myapp', outputs, new Set(outputs)); + const result = transformLegacyOutputs('myapp', outputs); expect(result).toEqual(['{workspaceRoot}/dist']); }); }); it('should prefix relative paths with {projectRoot}', () => { const outputs = ['./dist']; - const result = transformLegacyOutputs('myapp', outputs, new Set(outputs)); + const result = transformLegacyOutputs('myapp', outputs); expect(result).toEqual(['{projectRoot}/dist']); }); it('should prefix paths within the project with {projectRoot}', () => { const outputs = ['myapp/dist']; - const result = transformLegacyOutputs('myapp', outputs, new Set(outputs)); + const result = transformLegacyOutputs('myapp', outputs); expect(result).toEqual(['{projectRoot}/dist']); }); diff --git a/packages/nx/src/tasks-runner/utils.ts b/packages/nx/src/tasks-runner/utils.ts index 5484ea9fc600e..660c054b39184 100644 --- a/packages/nx/src/tasks-runner/utils.ts +++ b/packages/nx/src/tasks-runner/utils.ts @@ -18,7 +18,10 @@ import { readProjectsConfigurationFromProjectGraph } from '../project-graph/proj import { findMatchingProjects } from '../utils/find-matching-projects'; import { minimatch } from 'minimatch'; import { isGlobPattern } from '../utils/globs'; -import { validateOutputs as nativeValidateOutputs } from '../native'; +import { + getTransformableOutputs, + validateOutputs as nativeValidateOutputs, +} from '../native'; export type NormalizedTargetDependencyConfig = TargetDependencyConfig & { projects: string[]; @@ -257,11 +260,11 @@ export function validateOutputs(outputs: string[]) { nativeValidateOutputs(outputs); } -export function transformLegacyOutputs( - projectRoot: string, - outputs: string[], - transformableOutputs: Set -) { +export function transformLegacyOutputs(projectRoot: string, outputs: string[]) { + const transformableOutputs = new Set(getTransformableOutputs(outputs)); + if (transformableOutputs.size === 0) { + return outputs; + } return outputs.map((output) => { if (!transformableOutputs.has(output)) { return output; From f60190715816f98bae43a54fd6f364a978e837aa Mon Sep 17 00:00:00 2001 From: Craigory Coppola Date: Wed, 4 Sep 2024 17:17:29 -0400 Subject: [PATCH 5/7] fix(storybook): use correct outputs --- packages/nx/src/native/cache/validate_outputs.rs | 13 +++++++++++++ packages/storybook/src/plugins/plugin.ts | 2 +- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/packages/nx/src/native/cache/validate_outputs.rs b/packages/nx/src/native/cache/validate_outputs.rs index aa8a91684f93b..86c25bbb9ffdc 100644 --- a/packages/nx/src/native/cache/validate_outputs.rs +++ b/packages/nx/src/native/cache/validate_outputs.rs @@ -63,3 +63,16 @@ pub fn get_transformable_outputs(outputs: Vec) -> Vec { .filter(|output| is_missing_prefix(output)) .collect() } + +#[cfg(test)] +mod test { + use super::is_missing_prefix; + + #[test] + fn test_is_missing_prefix() { + assert!(is_missing_prefix("dist")); + assert!(is_missing_prefix("!dist")); + assert!(!is_missing_prefix("{workspaceRoot}/dist")); + assert!(!is_missing_prefix("!{workspaceRoot}/dist")); + } +} diff --git a/packages/storybook/src/plugins/plugin.ts b/packages/storybook/src/plugins/plugin.ts index abb1352a2e425..e4b7dfababec4 100644 --- a/packages/storybook/src/plugins/plugin.ts +++ b/packages/storybook/src/plugins/plugin.ts @@ -291,7 +291,7 @@ function normalizeOutputPath(projectRoot: string): string | undefined { if (projectRoot === '.') { return `{projectRoot}/storybook-static`; } else { - return `{workspaceRoot}/{projectRoot}/storybook-static`; + return `{projectRoot}/storybook-static`; } } From c45304269dc0a36873e15b87dc2f0a5b266e846a Mon Sep 17 00:00:00 2001 From: FrozenPandaz Date: Thu, 5 Sep 2024 14:52:55 -0400 Subject: [PATCH 6/7] chore(core): fixes --- .../nx/src/native/cache/expand_outputs.rs | 12 ++--- .../nx/src/native/cache/validate_outputs.rs | 21 +++++---- packages/nx/src/native/glob/glob_transform.rs | 45 ++++++++++--------- packages/storybook/src/plugins/plugin.spec.ts | 6 +-- packages/storybook/src/plugins/plugin.ts | 20 +++------ 5 files changed, 50 insertions(+), 54 deletions(-) diff --git a/packages/nx/src/native/cache/expand_outputs.rs b/packages/nx/src/native/cache/expand_outputs.rs index 516400eb43a3f..2f51b5213ac9a 100644 --- a/packages/nx/src/native/cache/expand_outputs.rs +++ b/packages/nx/src/native/cache/expand_outputs.rs @@ -72,18 +72,20 @@ where Ok(found_paths) } -fn partition_globs_into_map(globs: Vec) -> HashMap> { +fn partition_globs_into_map(globs: Vec) -> anyhow::Result>> { globs .iter() .map(|glob| partition_glob(glob)) // Right now we have an iterator where each item is (root: String, patterns: String[]). // We want a singular root, with the patterns mapped to it. .fold( - HashMap::>::new(), - |mut map, (root, patterns)| { + Ok(HashMap::>::new()), + |map_result, parsed_glob| { + let mut map = map_result?; + let (root, patterns) = parsed_glob?; let entry = map.entry(root).or_insert(vec![]); entry.extend(patterns); - map + Ok(map) }, ) } @@ -117,7 +119,7 @@ pub fn get_files_for_outputs( } if !globs.is_empty() { - let partitioned_globs = partition_globs_into_map(globs); + let partitioned_globs = partition_globs_into_map(globs)?; for (root, patterns) in partitioned_globs { let root_path = directory.join(&root); let glob_set = build_glob_set(&patterns)?; diff --git a/packages/nx/src/native/cache/validate_outputs.rs b/packages/nx/src/native/cache/validate_outputs.rs index 86c25bbb9ffdc..18171ab612e63 100644 --- a/packages/nx/src/native/cache/validate_outputs.rs +++ b/packages/nx/src/native/cache/validate_outputs.rs @@ -13,27 +13,26 @@ fn is_missing_prefix(output: &str) -> bool { #[napi] pub fn validate_outputs(outputs: Vec) -> anyhow::Result<()> { - let (missing_prefix, workspace_globs) = outputs.iter().fold( - (vec![], vec![]), - |(mut missing_prefix, mut workspace_globs), output| { - if is_missing_prefix(output) { - missing_prefix.push(output); - } + let outputs_len = outputs.len(); + let mut missing_prefix = Vec::with_capacity(outputs_len); + let mut workspace_globs = Vec::with_capacity(outputs_len); + for output in outputs.iter() { + if is_missing_prefix(output) { + missing_prefix.push(output); + } else { for prefix in ALLOWED_WORKSPACE_ROOT_OUTPUT_PREFIXES.iter() { if let Some(trimmed) = output.strip_prefix(prefix) { if contains_glob_pattern(&trimmed) { - let (root, _) = partition_glob(&trimmed); + let (root, _) = partition_glob(&trimmed)?; if root.is_empty() { workspace_globs.push(output); } } } } - - (missing_prefix, workspace_globs) - }, - ); + } + } if workspace_globs.is_empty() && missing_prefix.is_empty() { return Ok(()); diff --git a/packages/nx/src/native/glob/glob_transform.rs b/packages/nx/src/native/glob/glob_transform.rs index 5672f8559b2ed..2644f72feb646 100644 --- a/packages/nx/src/native/glob/glob_transform.rs +++ b/packages/nx/src/native/glob/glob_transform.rs @@ -2,7 +2,7 @@ use crate::native::glob::glob_group::GlobGroup; use crate::native::glob::glob_parser::parse_glob; use itertools::Itertools; use std::collections::HashSet; - +use itertools::Either::{Left, Right}; use super::contains_glob_pattern; #[derive(Debug)] @@ -113,31 +113,29 @@ fn build_segment( } } -pub fn partition_glob(glob: &str) -> (String, Vec) { - let (negated, groups) = parse_glob(glob).unwrap(); +pub fn partition_glob(glob: &str) -> anyhow::Result<(String, Vec)> { + let (negated, groups) = parse_glob(glob)?; // Partition glob into leading directories and patterns that should be matched - let mut leading_dir_segments: Vec = vec![]; - let mut pattern_segments = vec![]; - groups + let mut has_patterns = false; + let (leading_dir_segments, pattern_segments): (Vec, _) = groups .into_iter() .filter(|group| !group.is_empty()) - .for_each(|group| match &group[0] { - GlobGroup::NonSpecial(value) => { - if !contains_glob_pattern(&value) && pattern_segments.is_empty() { - leading_dir_segments.push(value.to_string()); - } else { - pattern_segments.push(group); + .partition_map(|group| { + match &group[0] { + GlobGroup::NonSpecial(value) if !contains_glob_pattern(&value) && !has_patterns => { + Left(value.to_string()) + } + _ => { + has_patterns = true; + Right(group) } - } - _ => { - pattern_segments.push(group); } }); - ( + Ok(( leading_dir_segments.join("/"), convert_glob_segments(negated, pattern_segments), - ) + )) } #[cfg(test)] @@ -269,21 +267,28 @@ mod test { #[test] fn should_partition_glob_with_leading_dirs() { - let (leading_dirs, globs) = super::partition_glob("dist/app/**/!(README|LICENSE).(js|ts)"); + let (leading_dirs, globs) = super::partition_glob("dist/app/**/!(README|LICENSE).(js|ts)").unwrap(); assert_eq!(leading_dirs, "dist/app"); assert_eq!(globs, ["!**/{README,LICENSE}.{js,ts}", "**/*.{js,ts}",]); } #[test] fn should_partition_glob_with_leading_dirs_and_simple_patterns() { - let (leading_dirs, globs) = super::partition_glob("dist/app/**/*.css"); + let (leading_dirs, globs) = super::partition_glob("dist/app/**/*.css").unwrap(); assert_eq!(leading_dirs, "dist/app"); assert_eq!(globs, ["**/*.css"]); } + #[test] + fn should_partition_glob_with_leading_dirs_dirs_and_patterns() { + let (leading_dirs, globs) = super::partition_glob("dist/app/**/js/*.js").unwrap(); + assert_eq!(leading_dirs, "dist/app"); + assert_eq!(globs, ["**/js/*.js"]); + } + #[test] fn should_partition_glob_with_leading_dirs_and_no_patterns() { - let (leading_dirs, globs) = super::partition_glob("dist/app/"); + let (leading_dirs, globs) = super::partition_glob("dist/app/").unwrap(); assert_eq!(leading_dirs, "dist/app"); assert_eq!(globs, [] as [String; 0]); } diff --git a/packages/storybook/src/plugins/plugin.spec.ts b/packages/storybook/src/plugins/plugin.spec.ts index 7288be2f5c423..89be68b87a141 100644 --- a/packages/storybook/src/plugins/plugin.spec.ts +++ b/packages/storybook/src/plugins/plugin.spec.ts @@ -74,7 +74,7 @@ describe('@nx/storybook/plugin', () => { }, cache: true, outputs: [ - '{workspaceRoot}/{projectRoot}/storybook-static', + '{projectRoot}/storybook-static', '{options.output-dir}', '{options.outputDir}', '{options.o}', @@ -127,7 +127,7 @@ describe('@nx/storybook/plugin', () => { }, cache: true, outputs: [ - '{workspaceRoot}/{projectRoot}/storybook-static', + '{projectRoot}/storybook-static', '{options.output-dir}', '{options.outputDir}', '{options.o}', @@ -192,7 +192,7 @@ describe('@nx/storybook/plugin', () => { }, cache: true, outputs: [ - '{workspaceRoot}/{projectRoot}/storybook-static', + '{projectRoot}/storybook-static', '{options.output-dir}', '{options.outputDir}', '{options.o}', diff --git a/packages/storybook/src/plugins/plugin.ts b/packages/storybook/src/plugins/plugin.ts index e4b7dfababec4..7fc597cb24b92 100644 --- a/packages/storybook/src/plugins/plugin.ts +++ b/packages/storybook/src/plugins/plugin.ts @@ -2,16 +2,16 @@ import { CreateDependencies, CreateNodes, CreateNodesContext, - TargetConfiguration, detectPackageManager, joinPathFragments, parseJson, readJsonFile, + TargetConfiguration, writeJsonFile, } from '@nx/devkit'; import { dirname, join } from 'path'; import { getNamedInputs } from '@nx/devkit/src/utils/get-named-inputs'; -import { existsSync, readFileSync, readdirSync } from 'fs'; +import { existsSync, readdirSync, readFileSync } from 'fs'; import { calculateHashForCreateNodes } from '@nx/devkit/src/utils/calculate-hash-for-create-nodes'; import { workspaceDataDirectory } from 'nx/src/utils/cache-directory'; import { getLockFileName } from '@nx/js'; @@ -109,7 +109,7 @@ async function buildStorybookTargets( context: CreateNodesContext, projectName: string ) { - const buildOutputs = getOutputs(projectRoot); + const buildOutputs = getOutputs(); const namedInputs = getNamedInputs(projectRoot, context); @@ -274,11 +274,9 @@ async function getStorybookFramework( return typeof framework === 'string' ? framework : framework.name; } -function getOutputs(projectRoot: string): string[] { - const normalizedOutputPath = normalizeOutputPath(projectRoot); - +function getOutputs(): string[] { const outputs = [ - normalizedOutputPath, + `{projectRoot}/storybook-static`, `{options.output-dir}`, `{options.outputDir}`, `{options.o}`, @@ -287,14 +285,6 @@ function getOutputs(projectRoot: string): string[] { return outputs; } -function normalizeOutputPath(projectRoot: string): string | undefined { - if (projectRoot === '.') { - return `{projectRoot}/storybook-static`; - } else { - return `{projectRoot}/storybook-static`; - } -} - function normalizeOptions( options: StorybookPluginOptions ): StorybookPluginOptions { From e70bbbda529053302e2a4b9301f72a69b3aa0909 Mon Sep 17 00:00:00 2001 From: Craigory Coppola Date: Thu, 5 Sep 2024 18:04:08 -0400 Subject: [PATCH 7/7] chore(storybook): update snapshots --- .../nx/src/utils/collapse-expanded-outputs.spec.ts | 4 ++-- .../convert-to-inferred/convert-to-inferred.spec.ts | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/nx/src/utils/collapse-expanded-outputs.spec.ts b/packages/nx/src/utils/collapse-expanded-outputs.spec.ts index a0ef185527473..45267e2b98aca 100644 --- a/packages/nx/src/utils/collapse-expanded-outputs.spec.ts +++ b/packages/nx/src/utils/collapse-expanded-outputs.spec.ts @@ -89,10 +89,10 @@ describe('collapseExpandedOutputs', () => { it('should collapse long lists of files in nested directories', async () => { const outputs = []; // Create dist/apps/app1/n/m.js + dist/apps/app1/n/m.d.ts - for (let i = 0; i < 6; i++) { + for (let i = 0; i < 6000; i++) { outputs.push(`dist/apps/app1/${i}.js`); outputs.push(`dist/apps/app1/${i}.d.ts`); - for (let j = 0; j < 6; j++) { + for (let j = 0; j < 600; j++) { outputs.push(`dist/apps/app1/${i}/${j}.js`); outputs.push(`dist/apps/app1/${i}/${j}.d.ts`); } diff --git a/packages/storybook/src/generators/convert-to-inferred/convert-to-inferred.spec.ts b/packages/storybook/src/generators/convert-to-inferred/convert-to-inferred.spec.ts index 88009f5474dee..46f21b380b451 100644 --- a/packages/storybook/src/generators/convert-to-inferred/convert-to-inferred.spec.ts +++ b/packages/storybook/src/generators/convert-to-inferred/convert-to-inferred.spec.ts @@ -236,7 +236,7 @@ describe('Storybook - Convert To Inferred', () => { }, "outputs": [ "{projectRoot}/{options.output-dir}", - "{workspaceRoot}/{projectRoot}/storybook-static", + "{projectRoot}/storybook-static", "{options.output-dir}", "{options.outputDir}", "{options.o}", @@ -312,7 +312,7 @@ describe('Storybook - Convert To Inferred', () => { }, "outputs": [ "{projectRoot}/{options.output-dir}", - "{workspaceRoot}/{projectRoot}/storybook-static", + "{projectRoot}/storybook-static", "{options.output-dir}", "{options.outputDir}", "{options.o}", @@ -387,7 +387,7 @@ describe('Storybook - Convert To Inferred', () => { }, "outputs": [ "{projectRoot}/{options.output-dir}", - "{workspaceRoot}/{projectRoot}/storybook-static", + "{projectRoot}/storybook-static", "{options.output-dir}", "{options.outputDir}", "{options.o}", @@ -446,7 +446,7 @@ describe('Storybook - Convert To Inferred', () => { }, "outputs": [ "{projectRoot}/{options.output-dir}", - "{workspaceRoot}/{projectRoot}/storybook-static", + "{projectRoot}/storybook-static", "{options.output-dir}", "{options.outputDir}", "{options.o}", @@ -565,7 +565,7 @@ describe('Storybook - Convert To Inferred', () => { }, "outputs": [ "{projectRoot}/{options.output-dir}", - "{workspaceRoot}/{projectRoot}/storybook-static", + "{projectRoot}/storybook-static", "{options.output-dir}", "{options.outputDir}", "{options.o}", @@ -597,7 +597,7 @@ describe('Storybook - Convert To Inferred', () => { }, "outputs": [ "{projectRoot}/{options.output-dir}", - "{workspaceRoot}/{projectRoot}/storybook-static", + "{projectRoot}/storybook-static", "{options.output-dir}", "{options.outputDir}", "{options.o}",