refactor(@angular-devkit/build-angular): replace most custom path normalize usage with Node.js builtins

During the build initialization phase, many paths are converted back and forth between multiple normalized forms. These conversions involve potentially expensive string operations. The majority of the custom path `normalize` function from `@angular-devkit/core` usages have now been removed in favor of the Node.js builtin path functions. This change reduces the need to perform additional string manipulation where possible.

Author: clydin

packages/angular_devkit/build_angular/src/builders/app-shell/index.ts

@@ -12,7 +12,7 @@ import {
   createBuilder,
   targetFromTargetString,
 } from '@angular-devkit/architect';
-import { JsonObject, normalize } from '@angular-devkit/core';
+import { JsonObject } from '@angular-devkit/core';
 import * as fs from 'fs';
 import * as path from 'path';
 import { normalizeOptimization } from '../../utils';
@@ -114,8 +114,8 @@ async function _renderUniversal(
 
     if (browserOptions.serviceWorker) {
       await augmentAppWithServiceWorker(
-        normalize(projectRoot),
-        normalize(outputPath),
+        projectRoot,
+        outputPath,
         browserOptions.baseHref || '/',
         browserOptions.ngswConfigPath,
       );

packages/angular_devkit/build_angular/src/builders/browser/index.ts

@@ -8,7 +8,7 @@
 
 import { BuilderContext, BuilderOutput, createBuilder } from '@angular-devkit/architect';
 import { EmittedFiles, WebpackLoggingCallback, runWebpack } from '@angular-devkit/build-webpack';
-import { logging, normalize } from '@angular-devkit/core';
+import { logging } from '@angular-devkit/core';
 import * as fs from 'fs';
 import * as path from 'path';
 import { Observable, from } from 'rxjs';
@@ -106,9 +106,9 @@ async function initialize(
   if (options.assets?.length && !adjustedOptions.assets?.length) {
     normalizeAssetPatterns(
       options.assets,
-      normalize(context.workspaceRoot),
-      normalize(projectRoot),
-      projectSourceRoot === undefined ? undefined : normalize(projectSourceRoot),
+      context.workspaceRoot,
+      projectRoot,
+      projectSourceRoot,
     ).forEach(({ output }) => {
       if (output.startsWith('..')) {
         throw new Error('An asset cannot be written to a location outside of the output path.');
@@ -268,9 +268,9 @@ export function buildWebpackBrowser(
                     await copyAssets(
                       normalizeAssetPatterns(
                         options.assets,
-                        normalize(context.workspaceRoot),
-                        normalize(projectRoot),
-                        projectSourceRoot === undefined ? undefined : normalize(projectSourceRoot),
+                        context.workspaceRoot,
+                        projectRoot,
+                        projectSourceRoot,
                       ),
                       Array.from(outputPaths.values()),
                       context.workspaceRoot,
@@ -347,8 +347,8 @@ export function buildWebpackBrowser(
                   for (const [locale, outputPath] of outputPaths.entries()) {
                     try {
                       await augmentAppWithServiceWorker(
-                        normalize(projectRoot),
-                        normalize(outputPath),
+                        projectRoot,
+                        outputPath,
                         getLocaleBaseHref(i18n, locale) || options.baseHref || '/',
                         options.ngswConfigPath,
                       );

packages/angular_devkit/build_angular/src/builders/karma/index.ts

@@ -7,9 +7,8 @@
  */
 
 import { BuilderContext, BuilderOutput, createBuilder } from '@angular-devkit/architect';
-import { getSystemPath, join, normalize } from '@angular-devkit/core';
 import { Config, ConfigOptions } from 'karma';
-import { resolve as fsResolve } from 'path';
+import * as path from 'path';
 import { Observable, from } from 'rxjs';
 import { defaultIfEmpty, switchMap } from 'rxjs/operators';
 import { Configuration } from 'webpack';
@@ -119,12 +118,13 @@ export function execute(
         }
 
         const projectMetadata = await context.getProjectMetadata(projectName);
-        const projectSourceRoot = getSystemPath(
-          join(
-            normalize(context.workspaceRoot),
-            (projectMetadata.root as string | undefined) ?? '',
-            (projectMetadata.sourceRoot as string | undefined) ?? '',
-          ),
+        const projectRoot = path.join(
+          context.workspaceRoot,
+          (projectMetadata.root as string | undefined) ?? '',
+        );
+        const projectSourceRoot = path.join(
+          projectRoot,
+          (projectMetadata.sourceRoot as string | undefined) ?? '',
         );
 
         const files = await findTests(options.include, context.workspaceRoot, projectSourceRoot);
@@ -144,7 +144,7 @@ export function execute(
         }
 
         rules.unshift({
-          test: fsResolve(context.workspaceRoot, options.main),
+          test: path.resolve(context.workspaceRoot, options.main),
           use: {
             // cannot be a simple path as it differs between environments
             loader: SingleTestTransformLoader,
@@ -163,7 +163,7 @@ export function execute(
       };
 
       const config = await karma.config.parseConfig(
-        fsResolve(context.workspaceRoot, options.karmaConfig),
+        path.resolve(context.workspaceRoot, options.karmaConfig),
         transforms.karmaOptions ? transforms.karmaOptions(karmaOptions) : karmaOptions,
         { promiseConfig: true, throwErrors: true },
       );

packages/angular_devkit/build_angular/src/utils/normalize-asset-patterns.ts

@@ -6,18 +6,9 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {
-  BaseException,
-  Path,
-  basename,
-  dirname,
-  getSystemPath,
-  join,
-  normalize,
-  relative,
-  resolve,
-} from '@angular-devkit/core';
+import { BaseException } from '@angular-devkit/core';
 import { statSync } from 'fs';
+import * as path from 'path';
 import { AssetPattern, AssetPatternClass } from '../builders/browser/schema';
 
 export class MissingAssetSourceRootException extends BaseException {
@@ -28,34 +19,34 @@ export class MissingAssetSourceRootException extends BaseException {
 
 export function normalizeAssetPatterns(
   assetPatterns: AssetPattern[],
-  root: Path,
-  projectRoot: Path,
-  maybeSourceRoot: Path | undefined,
+  workspaceRoot: string,
+  projectRoot: string,
+  projectSourceRoot: string | undefined,
 ): AssetPatternClass[] {
-  // When sourceRoot is not available, we default to ${projectRoot}/src.
-  const sourceRoot = maybeSourceRoot || join(projectRoot, 'src');
-  const resolvedSourceRoot = resolve(root, sourceRoot);
-
   if (assetPatterns.length === 0) {
     return [];
   }
 
+  // When sourceRoot is not available, we default to ${projectRoot}/src.
+  const sourceRoot = projectSourceRoot || path.join(projectRoot, 'src');
+  const resolvedSourceRoot = path.resolve(workspaceRoot, sourceRoot);
+
   return assetPatterns.map((assetPattern) => {
     // Normalize string asset patterns to objects.
     if (typeof assetPattern === 'string') {
-      const assetPath = normalize(assetPattern);
-      const resolvedAssetPath = resolve(root, assetPath);
+      const assetPath = path.normalize(assetPattern);
+      const resolvedAssetPath = path.resolve(workspaceRoot, assetPath);
 
       // Check if the string asset is within sourceRoot.
       if (!resolvedAssetPath.startsWith(resolvedSourceRoot)) {
         throw new MissingAssetSourceRootException(assetPattern);
       }
 
-      let glob: string, input: Path;
+      let glob: string, input: string;
       let isDirectory = false;
 
       try {
-        isDirectory = statSync(getSystemPath(resolvedAssetPath)).isDirectory();
+        isDirectory = statSync(resolvedAssetPath).isDirectory();
       } catch {
         isDirectory = true;
       }
@@ -67,13 +58,13 @@ export function normalizeAssetPatterns(
         input = assetPath;
       } else {
         // Files are their own glob.
-        glob = basename(assetPath);
+        glob = path.basename(assetPath);
         // Input directory is their original dirname.
-        input = dirname(assetPath);
+        input = path.dirname(assetPath);
       }
 
       // Output directory for both is the relative path from source root to input.
-      const output = relative(resolvedSourceRoot, resolve(root, input));
+      const output = path.relative(resolvedSourceRoot, path.resolve(workspaceRoot, input));
 
       // Return the asset pattern in object format.
       return { glob, input, output };

packages/angular_devkit/build_angular/src/utils/normalize-builder-schema.ts

@@ -6,7 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import { json, normalize } from '@angular-devkit/core';
+import { json } from '@angular-devkit/core';
 import {
   AssetPatternClass,
   Schema as BrowserBuilderSchema,
@@ -48,14 +48,11 @@ export function normalizeBrowserSchema(
     cache: normalizeCacheOptions(metadata, workspaceRoot),
     assets: normalizeAssetPatterns(
       options.assets || [],
-      normalize(workspaceRoot),
-      normalize(projectRoot),
-      projectSourceRoot ? normalize(projectSourceRoot) : undefined,
-    ),
-    fileReplacements: normalizeFileReplacements(
-      options.fileReplacements || [],
-      normalize(workspaceRoot),
+      workspaceRoot,
+      projectRoot,
+      projectSourceRoot,
     ),
+    fileReplacements: normalizeFileReplacements(options.fileReplacements || [], workspaceRoot),
     optimization: normalizeOptimization(options.optimization),
     sourceMap: normalizedSourceMapOptions,
     preserveSymlinks:

packages/angular_devkit/build_angular/src/utils/normalize-file-replacements.ts

@@ -6,8 +6,9 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import { BaseException, Path, getSystemPath, join, normalize } from '@angular-devkit/core';
+import { BaseException } from '@angular-devkit/core';
 import { existsSync } from 'fs';
+import * as path from 'path';
 import { FileReplacement } from '../builders/browser/schema';
 
 export class MissingFileReplacementException extends BaseException {
@@ -17,29 +18,29 @@ export class MissingFileReplacementException extends BaseException {
 }
 
 export interface NormalizedFileReplacement {
-  replace: Path;
-  with: Path;
+  replace: string;
+  with: string;
 }
 
 export function normalizeFileReplacements(
   fileReplacements: FileReplacement[],
-  root: Path,
+  workspaceRoot: string,
 ): NormalizedFileReplacement[] {
   if (fileReplacements.length === 0) {
     return [];
   }
 
   const normalizedReplacement = fileReplacements.map((replacement) =>
-    normalizeFileReplacement(replacement, root),
+    normalizeFileReplacement(replacement, workspaceRoot),
   );
 
   for (const { replace, with: replacementWith } of normalizedReplacement) {
-    if (!existsSync(getSystemPath(replacementWith))) {
-      throw new MissingFileReplacementException(getSystemPath(replacementWith));
+    if (!existsSync(replacementWith)) {
+      throw new MissingFileReplacementException(replacementWith);
     }
 
-    if (!existsSync(getSystemPath(replace))) {
-      throw new MissingFileReplacementException(getSystemPath(replace));
+    if (!existsSync(replace)) {
+      throw new MissingFileReplacementException(replace);
     }
   }
 
@@ -48,27 +49,22 @@ export function normalizeFileReplacements(
 
 function normalizeFileReplacement(
   fileReplacement: FileReplacement,
-  root?: Path,
+  root: string,
 ): NormalizedFileReplacement {
-  let replacePath: Path;
-  let withPath: Path;
+  let replacePath: string;
+  let withPath: string;
   if (fileReplacement.src && fileReplacement.replaceWith) {
-    replacePath = normalize(fileReplacement.src);
-    withPath = normalize(fileReplacement.replaceWith);
+    replacePath = fileReplacement.src;
+    withPath = fileReplacement.replaceWith;
   } else if (fileReplacement.replace && fileReplacement.with) {
-    replacePath = normalize(fileReplacement.replace);
-    withPath = normalize(fileReplacement.with);
+    replacePath = fileReplacement.replace;
+    withPath = fileReplacement.with;
   } else {
     throw new Error(`Invalid file replacement: ${JSON.stringify(fileReplacement)}`);
   }
 
-  // TODO: For 7.x should this only happen if not absolute?
-  if (root) {
-    replacePath = join(root, replacePath);
-  }
-  if (root) {
-    withPath = join(root, withPath);
-  }
-
-  return { replace: replacePath, with: withPath };
+  return {
+    replace: path.join(root, replacePath),
+    with: path.join(root, withPath),
+  };
 }

packages/angular_devkit/build_angular/src/utils/service-worker.ts

@@ -6,7 +6,6 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import { Path, getSystemPath, normalize } from '@angular-devkit/core';
 import type { Config, Filesystem } from '@angular/service-worker/config';
 import * as crypto from 'crypto';
 import { createReadStream, promises as fs, constants as fsConstants } from 'fs';
@@ -62,17 +61,15 @@ class CliFilesystem implements Filesystem {
 }
 
 export async function augmentAppWithServiceWorker(
-  appRoot: Path,
-  outputPath: Path,
+  appRoot: string,
+  outputPath: string,
   baseHref: string,
   ngswConfigPath?: string,
 ): Promise<void> {
-  const distPath = getSystemPath(normalize(outputPath));
-
   // Determine the configuration file path
   const configPath = ngswConfigPath
-    ? getSystemPath(normalize(ngswConfigPath))
-    : path.join(getSystemPath(appRoot), 'ngsw-config.json');
+    ? path.normalize(ngswConfigPath)
+    : path.join(appRoot, 'ngsw-config.json');
 
   // Read the configuration file
   let config: Config | undefined;
@@ -83,7 +80,7 @@ export async function augmentAppWithServiceWorker(
     if (error.code === 'ENOENT') {
       throw new Error(
         'Error: Expected to find an ngsw-config.json configuration file' +
-          ` in the ${getSystemPath(appRoot)} folder. Either provide one or` +
+          ` in the ${appRoot} folder. Either provide one or` +
           ' disable Service Worker in the angular.json configuration file.',
       );
     } else {
@@ -101,20 +98,20 @@ export async function augmentAppWithServiceWorker(
   ).Generator;
 
   // Generate the manifest
-  const generator = new GeneratorConstructor(new CliFilesystem(distPath), baseHref);
+  const generator = new GeneratorConstructor(new CliFilesystem(outputPath), baseHref);
   const output = await generator.process(config);
 
   // Write the manifest
   const manifest = JSON.stringify(output, null, 2);
-  await fs.writeFile(path.join(distPath, 'ngsw.json'), manifest);
+  await fs.writeFile(path.join(outputPath, 'ngsw.json'), manifest);
 
   // Find the service worker package
   const workerPath = require.resolve('@angular/service-worker/ngsw-worker.js');
 
   // Write the worker code
   await fs.copyFile(
     workerPath,
-    path.join(distPath, 'ngsw-worker.js'),
+    path.join(outputPath, 'ngsw-worker.js'),
     fsConstants.COPYFILE_FICLONE,
   );
 
@@ -123,12 +120,12 @@ export async function augmentAppWithServiceWorker(
   try {
     await fs.copyFile(
       safetyPath,
-      path.join(distPath, 'worker-basic.min.js'),
+      path.join(outputPath, 'worker-basic.min.js'),
       fsConstants.COPYFILE_FICLONE,
     );
     await fs.copyFile(
       safetyPath,
-      path.join(distPath, 'safety-worker.js'),
+      path.join(outputPath, 'safety-worker.js'),
       fsConstants.COPYFILE_FICLONE,
     );
   } catch (error) {