chore(core): code review

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);

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<String> = 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
                 }

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}"];
 

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::*;

packages/nx/src/native/glob/glob_transform.rs

@@ -113,23 +113,15 @@ fn build_segment(
     }
 }
 
-fn remove_first<T>(vec: &mut Vec<T>) -> Option<T> {
-    if vec.is_empty() {
-        return None;
-    }
-    Some(vec.remove(0))
-}
-
 pub fn partition_glob(glob: &str) -> (String, Vec<String>) {
-    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<String> = 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<String>) {
             _ => {
                 pattern_segments.push(group);
             }
-        }
-    }
+        });
 
     (
         leading_dir_segments.join("/"),

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']);
   });
 

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<string>
-) {
+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;