From c9ec7d0f049c4586eef9b70ee8f960685447830c Mon Sep 17 00:00:00 2001 From: Craigory Coppola Date: Wed, 24 May 2023 17:15:14 -0400 Subject: [PATCH] fix(core): update package-manager utils to normalize yarn-path when migrating (#17088) --- packages/nx/src/utils/package-manager.spec.ts | 104 +++++++++++++----- packages/nx/src/utils/package-manager.ts | 73 +++++++++++- .../src/generators/lint-checks/generator.ts | 90 ++++++++------- 3 files changed, 195 insertions(+), 72 deletions(-) diff --git a/packages/nx/src/utils/package-manager.spec.ts b/packages/nx/src/utils/package-manager.spec.ts index 0caea133b9598..92a24d70ba505 100644 --- a/packages/nx/src/utils/package-manager.spec.ts +++ b/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'); + }); }); }); diff --git a/packages/nx/src/utils/package-manager.ts b/packages/nx/src/utils/package-manager.ts index a20ca693d6dfc..2358673ca3634 100644 --- a/packages/nx/src/utils/package-manager.ts +++ b/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; + } + } } } } diff --git a/packages/plugin/src/generators/lint-checks/generator.ts b/packages/plugin/src/generators/lint-checks/generator.ts index 8482f9ae56f74..10257c7e8739c 100644 --- a/packages/plugin/src/generators/lint-checks/generator.ts +++ b/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(host, eslintPath); - eslintConfig.overrides ??= []; - let entry: ESLint.ConfigOverride = - 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(host, eslintPath); + eslintConfig.overrides ??= []; + let entry: ESLint.ConfigOverride = + 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(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(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); } }