From 9c3bb8e3ba729640e7ccf6c2bf493ac1ab90b648 Mon Sep 17 00:00:00 2001 From: AgentEnder Date: Thu, 18 May 2023 11:33:48 -0400 Subject: [PATCH] fix(core): update package-manager utils to normalize yarn-path when migrating --- packages/nx/src/utils/package-manager.spec.ts | 114 +++++++++++++----- packages/nx/src/utils/package-manager.ts | 109 ++++++++++++++++- .../src/generators/lint-checks/generator.ts | 90 ++++++++------ 3 files changed, 242 insertions(+), 71 deletions(-) diff --git a/packages/nx/src/utils/package-manager.spec.ts b/packages/nx/src/utils/package-manager.spec.ts index 0caea133b9598e..45cfab011081ff 100644 --- a/packages/nx/src/utils/package-manager.spec.ts +++ b/packages/nx/src/utils/package-manager.spec.ts @@ -1,43 +1,99 @@ 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', - }, + 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); }); - 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 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( + '/home/user/projects/nx', + '/tmp/virtual-nx-workspace', + 'yarnPath: ./bin/yarn.js' + ) + ).toEqual('yarnPath: ../../home/user/projects/nx/bin/yarn.js\n'); + }); + + it('should update plugins appropriately', () => { + expect( + modifyYarnRcYmlToFitNewDirectory( + '/home/user/projects/nx', + '/tmp/virtual-nx-workspace', + [ + 'enableProgressBars: false', + 'plugins:', + ' - ./scripts/yarn-plugin.js', + ' - path: .yarn/plugins/imported-plugin.js', + ' spec: imported-plugin', + ].join('\n') + ) + ).toEqual( + [ + 'enableProgressBars: false', + '', + 'plugins:', + ' - ../../home/user/projects/nx/scripts/yarn-plugin.js', + ' - path: ../../home/user/projects/nx/.yarn/plugins/imported-plugin.js', + ' spec: imported-plugin', + '', + ].join('\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( + '/home/user/projects/nx', + '/tmp/virtual-nx-workspace', + 'yarn-path ./bin/yarn.js' + ) + ).toEqual('yarn-path "../../home/user/projects/nx/bin/yarn.js"'); + }); }); }); diff --git a/packages/nx/src/utils/package-manager.ts b/packages/nx/src/utils/package-manager.ts index a20ca693d6dfc9..751b8794851085 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,85 @@ 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 from The directory we are copying from + * @param to The directory we are copying to + * @param contents The string contents of the yarnrc.yml file + * @returns Updated string contents of the yarnrc.yml file + */ +export function modifyYarnRcYmlToFitNewDirectory( + from: string, + to: string, + 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. + console.log(to, join(from, parsed.yarnPath)); + parsed.yarnPath = relative(to, join(from, parsed.yarnPath)); + } + if (parsed.plugins) { + parsed.plugins = parsed.plugins.map((plugin) => { + // Plugins specified by a string are relative paths from workspace root. + // ex: https://yarnpkg.com/advanced/plugin-tutorial#writing-our-first-plugin + if (typeof plugin === 'string') { + return relative(to, join(from, plugin)); + // Imported plugins are specified by an object with a path and spec. + // The path is relative to the workspace root, and the spec is a string that + // appears to not be used by yarn. Here's an example yaml file with an imported + // plugin: https://github.com/RocketChat/Rocket.Chat/blob/d9f3d9c5c6cacddaf22fa9535fd0b4b0137a5005/.yarnrc.yml + } else { + return { + ...plugin, + path: relative(to, join(from, plugin.path)), + }; + } + }); + } + 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 from The directory we are copying from + * @param to The directory we are copying to + * @param contents The string contents of the yarnrc.yml file + * @returns Updated string contents of the yarnrc.yml file + */ +export function modifyYarnRcToFitNewDirectory( + from: string, + to: string, + contents: string +): string { + const lines = contents.split('\n'); + const yarnPathIndex = lines.findIndex((line) => line.startsWith('yarn-path')); + if (yarnPathIndex !== -1) { + const currentRelativePath = lines[yarnPathIndex].split(' ')[1]; + lines[yarnPathIndex] = `yarn-path "${relative( + to, + join(from, currentRelativePath) + )}"`; + } + return lines.join('\n'); +} + export function copyPackageManagerConfigurationFiles( root: string, destination: string @@ -154,8 +233,30 @@ 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( + dirname(f), + destination, + readFileIfExisting(f) + ); + writeFileSync(destinationPath, updated); + break; + } + case '.yarnrc.yml': { + const updated = modifyYarnRcYmlToFitNewDirectory( + dirname(f), + destination, + 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 8482f9ae56f744..10257c7e8739ca 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); } }