fix(core): update package-manager utils to normalize yarn-path when m…

…igrating (#17088)

packages/nx/src/utils/package-manager.spec.ts

@@ -1,43 +1,87 @@
 jest.mock('fs');
 import * as fs from 'fs';
 import * as configModule from '../config/configuration';
-import { detectPackageManager } from './package-manager';
+import {
+  detectPackageManager,
+  modifyYarnRcToFitNewDirectory,
+  modifyYarnRcYmlToFitNewDirectory,
+} from './package-manager';
 
 describe('package-manager', () => {
-  it('should detect package manager in nxJson', () => {
-    jest.spyOn(configModule, 'readNxJson').mockReturnValueOnce({
-      cli: {
-        packageManager: 'pnpm',
-      },
-    });
-    const packageManager = detectPackageManager();
-    expect(packageManager).toEqual('pnpm');
-    expect(fs.existsSync).not.toHaveBeenCalled();
-  });
+  describe('detectPackageManager', () => {
+    it('should detect package manager in nxJson', () => {
+      jest.spyOn(configModule, 'readNxJson').mockReturnValueOnce({
+        cli: {
+          packageManager: 'pnpm',
+        },
+      });
+      const packageManager = detectPackageManager();
+      expect(packageManager).toEqual('pnpm');
+      expect(fs.existsSync).not.toHaveBeenCalled();
+    });
+
+    it('should detect yarn package manager from yarn.lock', () => {
+      jest.spyOn(configModule, 'readNxJson').mockReturnValueOnce({});
+      (fs.existsSync as jest.Mock).mockReturnValueOnce(true);
+      const packageManager = detectPackageManager();
+      expect(packageManager).toEqual('yarn');
+      expect(fs.existsSync).toHaveBeenNthCalledWith(1, 'yarn.lock');
+    });
+
+    it('should detect pnpm package manager from pnpm-lock.yaml', () => {
+      jest.spyOn(configModule, 'readNxJson').mockReturnValueOnce({});
+      (fs.existsSync as jest.Mock).mockImplementation((path) => {
+        return path === 'pnpm-lock.yaml';
+      });
+      const packageManager = detectPackageManager();
+      expect(packageManager).toEqual('pnpm');
+      expect(fs.existsSync).toHaveBeenCalledTimes(3);
+    });
 
-  it('should detect yarn package manager from yarn.lock', () => {
-    jest.spyOn(configModule, 'readNxJson').mockReturnValueOnce({});
-    (fs.existsSync as jest.Mock).mockReturnValueOnce(true);
-    const packageManager = detectPackageManager();
-    expect(packageManager).toEqual('yarn');
-    expect(fs.existsSync).toHaveBeenNthCalledWith(1, 'yarn.lock');
+    it('should use npm package manager as default', () => {
+      jest.spyOn(configModule, 'readNxJson').mockReturnValueOnce({});
+      (fs.existsSync as jest.Mock).mockReturnValue(false);
+      const packageManager = detectPackageManager();
+      expect(packageManager).toEqual('npm');
+      expect(fs.existsSync).toHaveBeenCalledTimes(5);
+    });
   });
 
-  it('should detect pnpm package manager from pnpm-lock.yaml', () => {
-    jest.spyOn(configModule, 'readNxJson').mockReturnValueOnce({});
-    (fs.existsSync as jest.Mock).mockImplementation((path) => {
-      return path === 'pnpm-lock.yaml';
+  describe('modifyYarnRcYmlToFitNewDirectory', () => {
+    it('should update paths properly', () => {
+      expect(
+        modifyYarnRcYmlToFitNewDirectory('yarnPath: ./bin/yarn.js')
+      ).toEqual('');
+    });
+
+    it('should update plugins appropriately', () => {
+      expect(
+        modifyYarnRcYmlToFitNewDirectory(
+          [
+            'enableProgressBars: false',
+            'plugins:',
+            '  - ./scripts/yarn-plugin.js',
+            '  - path: .yarn/plugins/imported-plugin.js',
+            '    spec: imported-plugin',
+          ].join('\n')
+        )
+      ).toEqual('enableProgressBars: false\n');
     });
-    const packageManager = detectPackageManager();
-    expect(packageManager).toEqual('pnpm');
-    expect(fs.existsSync).toHaveBeenCalledTimes(3);
   });
 
-  it('should use npm package manager as default', () => {
-    jest.spyOn(configModule, 'readNxJson').mockReturnValueOnce({});
-    (fs.existsSync as jest.Mock).mockReturnValue(false);
-    const packageManager = detectPackageManager();
-    expect(packageManager).toEqual('npm');
-    expect(fs.existsSync).toHaveBeenCalledTimes(5);
+  describe('modifyYarnRcToFitNewDirectory', () => {
+    it('should update paths properly', () => {
+      expect(modifyYarnRcToFitNewDirectory('yarn-path ./bin/yarn.js')).toEqual(
+        ''
+      );
+    });
+
+    it('should not update other options', () => {
+      expect(
+        modifyYarnRcToFitNewDirectory(
+          ['yarn-path ./bin/yarn.js', 'enableProgressBars false'].join('\n')
+        )
+      ).toEqual('enableProgressBars false');
+    });
   });
 });

packages/nx/src/utils/package-manager.ts

@@ -1,10 +1,10 @@
 import { exec, execSync } from 'child_process';
-import { copyFileSync, existsSync } from 'fs';
+import { copyFileSync, existsSync, writeFileSync } from 'fs';
 import { remove } from 'fs-extra';
 import { dirname, join, relative } from 'path';
 import { dirSync } from 'tmp';
 import { promisify } from 'util';
-import { writeJsonFile } from './fileutils';
+import { readFileIfExisting, writeJsonFile } from './fileutils';
 import { readModulePackageJson } from './package-json';
 import { gte, lt } from 'semver';
 import { workspaceRoot } from './workspace-root';
@@ -142,6 +142,55 @@ export function findFileInPackageJsonDirectory(
   return existsSync(path) ? path : null;
 }
 
+/**
+ * We copy yarnrc.yml to the temporary directory to ensure things like the specified
+ * package registry are still used. However, there are a few relative paths that can
+ * cause issues, so we modify them to fit the new directory.
+ *
+ * Exported for testing - not meant to be used outside of this file.
+ *
+ * @param contents The string contents of the yarnrc.yml file
+ * @returns Updated string contents of the yarnrc.yml file
+ */
+export function modifyYarnRcYmlToFitNewDirectory(contents: string): string {
+  const { parseSyml, stringifySyml } = require('@yarnpkg/parsers');
+  const parsed: {
+    yarnPath?: string;
+    plugins?: (string | { path: string; spec: string })[];
+  } = parseSyml(contents);
+
+  if (parsed.yarnPath) {
+    // yarnPath is relative to the workspace root, so we need to make it relative
+    // to the new directory s.t. it still points to the same yarn binary.
+    delete parsed.yarnPath;
+  }
+  if (parsed.plugins) {
+    // Plugins specified by a string are relative paths from workspace root.
+    // ex: https://yarnpkg.com/advanced/plugin-tutorial#writing-our-first-plugin
+    delete parsed.plugins;
+  }
+  return stringifySyml(parsed);
+}
+
+/**
+ * We copy .yarnrc to the temporary directory to ensure things like the specified
+ * package registry are still used. However, there are a few relative paths that can
+ * cause issues, so we modify them to fit the new directory.
+ *
+ * Exported for testing - not meant to be used outside of this file.
+ *
+ * @param contents The string contents of the yarnrc.yml file
+ * @returns Updated string contents of the yarnrc.yml file
+ */
+export function modifyYarnRcToFitNewDirectory(contents: string): string {
+  const lines = contents.split('\n');
+  const yarnPathIndex = lines.findIndex((line) => line.startsWith('yarn-path'));
+  if (yarnPathIndex !== -1) {
+    lines.splice(yarnPathIndex, 1);
+  }
+  return lines.join('\n');
+}
+
 export function copyPackageManagerConfigurationFiles(
   root: string,
   destination: string
@@ -154,8 +203,24 @@ export function copyPackageManagerConfigurationFiles(
       // but now relative to the destination. `relative` makes `{workspaceRoot}/some/path`
       // look like `./some/path`, and joining that gets us `{destination}/some/path
       const destinationPath = join(destination, relative(root, f));
-      // Copy config file if it exists, so that the package manager still follows it.
-      copyFileSync(f, destinationPath);
+      switch (packageManagerConfigFile) {
+        case '.npmrc': {
+          copyFileSync(f, destinationPath);
+          break;
+        }
+        case '.yarnrc': {
+          const updated = modifyYarnRcToFitNewDirectory(readFileIfExisting(f));
+          writeFileSync(destinationPath, updated);
+          break;
+        }
+        case '.yarnrc.yml': {
+          const updated = modifyYarnRcYmlToFitNewDirectory(
+            readFileIfExisting(f)
+          );
+          writeFileSync(destinationPath, updated);
+          break;
+        }
+      }
     }
   }
 }

packages/plugin/src/generators/lint-checks/generator.ts

@@ -3,6 +3,7 @@ import {
   formatFiles,
   joinPathFragments,
   logger,
+  output,
   ProjectConfiguration,
   readJson,
   readProjectConfiguration,
@@ -190,53 +191,66 @@ function updateProjectEslintConfig(
   // Update the project level lint configuration to specify
   // the plugin schema rule for generated files
   const eslintPath = `${options.root}/.eslintrc.json`;
-  const eslintConfig = readJson<ESLint.Config>(host, eslintPath);
-  eslintConfig.overrides ??= [];
-  let entry: ESLint.ConfigOverride<ESLint.RulesRecord> =
-    eslintConfig.overrides.find(
-      (x) =>
-        Object.keys(x.rules ?? {}).includes('@nx/nx-plugin-checks') ||
-        Object.keys(x.rules ?? {}).includes('@nrwl/nx/nx-plugin-checks')
-    );
-  const newentry = !entry;
-  entry ??= { files: [] };
-  entry.files = [
-    ...new Set([
-      ...(entry.files ?? []),
-      ...[
-        './package.json',
-        packageJson.generators,
-        packageJson.executors,
-        packageJson.schematics,
-        packageJson.builders,
-      ].filter((f) => !!f),
-    ]),
-  ];
-  entry.parser = 'jsonc-eslint-parser';
-  entry.rules ??= {
-    '@nx/nx-plugin-checks': 'error',
-  };
+  if (host.exists(eslintPath)) {
+    const eslintConfig = readJson<ESLint.Config>(host, eslintPath);
+    eslintConfig.overrides ??= [];
+    let entry: ESLint.ConfigOverride<ESLint.RulesRecord> =
+      eslintConfig.overrides.find(
+        (x) =>
+          Object.keys(x.rules ?? {}).includes('@nx/nx-plugin-checks') ||
+          Object.keys(x.rules ?? {}).includes('@nrwl/nx/nx-plugin-checks')
+      );
+    const newentry = !entry;
+    entry ??= { files: [] };
+    entry.files = [
+      ...new Set([
+        ...(entry.files ?? []),
+        ...[
+          './package.json',
+          packageJson.generators,
+          packageJson.executors,
+          packageJson.schematics,
+          packageJson.builders,
+        ].filter((f) => !!f),
+      ]),
+    ];
+    entry.parser = 'jsonc-eslint-parser';
+    entry.rules ??= {
+      '@nx/nx-plugin-checks': 'error',
+    };
 
-  if (newentry) {
-    eslintConfig.overrides.push(entry);
-  }
+    if (newentry) {
+      eslintConfig.overrides.push(entry);
+    }
 
-  writeJson(host, eslintPath, eslintConfig);
+    writeJson(host, eslintPath, eslintConfig);
+  }
 }
 
 // Update the root eslint to specify a parser for json files
 // This is required, otherwise every json file that is not overriden
 // will display false errors in the IDE
 function updateRootEslintConfig(host: Tree) {
-  const rootESLint = readJson<ESLint.Config>(host, '.eslintrc.json');
-  rootESLint.overrides ??= [];
-  if (!eslintConfigContainsJsonOverride(rootESLint)) {
-    rootESLint.overrides.push({
-      files: '*.json',
-      parser: 'jsonc-eslint-parser',
-      rules: {},
+  if (host.exists('.eslintrc.json')) {
+    const rootESLint = readJson<ESLint.Config>(host, '.eslintrc.json');
+    rootESLint.overrides ??= [];
+    if (!eslintConfigContainsJsonOverride(rootESLint)) {
+      rootESLint.overrides.push({
+        files: '*.json',
+        parser: 'jsonc-eslint-parser',
+        rules: {},
+      });
+      writeJson(host, '.eslintrc.json', rootESLint);
+    }
+  } else {
+    output.note({
+      title: 'Unable to update root eslint config.',
+      bodyLines: [
+        'We only automatically update the root eslint config if it is json.',
+        'If you are using a different format, you will need to update it manually.',
+        'You need to set the parser to jsonc-eslint-parser for json files.',
+      ],
     });
-    writeJson(host, '.eslintrc.json', rootESLint);
   }
 }