fix(core): add workspaces path if package path is not included

nrwl · Nov 20, 2024 · 20b7eb9 · 20b7eb9
1 parent 09a01eb
commit 20b7eb9
Show file tree

Hide file tree

Showing 11 changed files with 708 additions and 297 deletions.
diff --git a/e2e/nx/src/import.test.ts b/e2e/nx/src/import.test.ts
@@ -5,9 +5,9 @@ import {
   newProject,
   runCLI,
   runCommand,
-  updateJson,
-  updateFile,
   e2eCwd,
+  readJson,
+  readFile,
 } from '@nx/e2e/utils';
 import { writeFileSync, mkdirSync, rmdirSync } from 'fs';
 import { execSync } from 'node:child_process';
@@ -21,24 +21,13 @@ describe('Nx Import', () => {
       packages: ['@nx/js'],
     });
 
-    if (getSelectedPackageManager() === 'pnpm') {
-      updateFile(
-        'pnpm-workspace.yaml',
-        `packages:
-  - 'projects/*'
-`
-      );
-    } else {
-      updateJson('package.json', (json) => {
-        json.workspaces = ['projects/*'];
-        return json;
-      });
-    }
-
     try {
       rmdirSync(join(tempImportE2ERoot));
     } catch {}
+  });
 
+  beforeEach(() => {
+    // Clean up the temp import directory before each test to not have any uncommited changes
     runCommand(`git add .`);
     runCommand(`git commit -am "Update" --allow-empty`);
   });
@@ -85,6 +74,14 @@ describe('Nx Import', () => {
       }
     );
 
+    if (getSelectedPackageManager() === 'pnpm') {
+      const workspaceYaml = readFile('pnpm-workspace.yaml');
+      expect(workspaceYaml).toMatch(/(projects\/vite-app)/);
+    } else {
+      const packageJson = readJson('package.json');
+      expect(packageJson.workspaces).toContain('projects/vite-app');
+    }
+
     checkFilesExist(
       'projects/vite-app/.gitignore',
       'projects/vite-app/package.json',
@@ -110,9 +107,13 @@ describe('Nx Import', () => {
     execSync(`git commit -am "initial commit"`, {
       cwd: repoPath,
     });
-    execSync(`git checkout -b main`, {
-      cwd: repoPath,
-    });
+    try {
+      execSync(`git checkout -b main`, {
+        cwd: repoPath,
+      });
+    } catch {
+      // This fails if git is already configured to have `main` branch, but that's OK
+    }
     mkdirSync(join(repoPath, 'packages/a'), { recursive: true });
     writeFileSync(join(repoPath, 'packages/a/README.md'), `# A`);
     execSync(`git add .`, {

diff --git a/package.json b/package.json
@@ -134,7 +134,6 @@
     "@types/jasmine": "~2.8.6",
     "@types/jasminewd2": "~2.0.3",
     "@types/jest": "29.5.12",
-    "@types/js-yaml": "^4.0.5",
     "@types/marked": "^2.0.0",
     "@types/node": "20.16.10",
     "@types/npm-package-arg": "6.1.1",
@@ -314,6 +313,7 @@
     "webpack-sources": "^3.2.3",
     "webpack-subresource-integrity": "^5.1.0",
     "xstate": "4.34.0",
+    "yaml": "^2.6.0",
     "yargs": "17.6.2",
     "yargs-parser": "21.1.1"
   },

diff --git a/packages/nx/package.json b/packages/nx/package.json
@@ -57,18 +57,19 @@
     "jsonc-parser": "3.2.0",
     "lines-and-columns": "2.0.3",
     "minimatch": "9.0.3",
+    "node-machine-id": "1.1.12",
     "npm-run-path": "^4.0.1",
     "open": "^8.4.0",
+    "ora": "5.3.0",
     "semver": "^7.5.3",
     "string-width": "^4.2.3",
     "tar-stream": "~2.2.0",
     "tmp": "~0.2.1",
     "tsconfig-paths": "^4.1.2",
     "tslib": "^2.3.0",
+    "yaml": "^2.6.0",
     "yargs": "^17.6.2",
-    "yargs-parser": "21.1.1",
-    "node-machine-id": "1.1.12",
-    "ora": "5.3.0"
+    "yargs-parser": "21.1.1"
   },
   "peerDependencies": {
     "@swc-node/register": "^1.8.0",
@@ -83,16 +84,16 @@
     }
   },
   "optionalDependencies": {
-    "@nx/nx-darwin-x64": "*",
     "@nx/nx-darwin-arm64": "*",
-    "@nx/nx-linux-x64-gnu": "*",
-    "@nx/nx-linux-x64-musl": "*",
-    "@nx/nx-win32-x64-msvc": "*",
+    "@nx/nx-darwin-x64": "*",
+    "@nx/nx-freebsd-x64": "*",
+    "@nx/nx-linux-arm-gnueabihf": "*",
     "@nx/nx-linux-arm64-gnu": "*",
     "@nx/nx-linux-arm64-musl": "*",
-    "@nx/nx-linux-arm-gnueabihf": "*",
+    "@nx/nx-linux-x64-gnu": "*",
+    "@nx/nx-linux-x64-musl": "*",
     "@nx/nx-win32-arm64-msvc": "*",
-    "@nx/nx-freebsd-x64": "*"
+    "@nx/nx-win32-x64-msvc": "*"
   },
   "nx-migrations": {
     "migrations": "./migrations.json",

diff --git a/packages/nx/src/command-line/import/import.ts b/packages/nx/src/command-line/import/import.ts
@@ -1,20 +1,24 @@
-import { dirname, isAbsolute, join, relative, resolve } from 'path';
-import { minimatch } from 'minimatch';
+import { isAbsolute, join, relative, resolve } from 'path';
 import { existsSync, promises as fsp } from 'node:fs';
 import * as chalk from 'chalk';
-import { load as yamlLoad } from '@zkochan/js-yaml';
 import { cloneFromUpstream, GitRepository } from '../../utils/git-utils';
 import { stat, mkdir, rm } from 'node:fs/promises';
 import { tmpdir } from 'tmp';
 import { prompt } from 'enquirer';
 import { output } from '../../utils/output';
 import * as createSpinner from 'ora';
-import { detectPlugins, installPlugins } from '../init/init-v2';
+import {
+  detectPlugins,
+  installPlugins,
+  runPackageManagerInstallPlugins,
+} from '../init/init-v2';
 import { readNxJson } from '../../config/nx-json';
 import { workspaceRoot } from '../../utils/workspace-root';
 import {
+  addPackagePathToWorkspaces,
   detectPackageManager,
   getPackageManagerCommand,
+  getPackageWorkspaces,
   isWorkspacesEnabled,
   PackageManager,
   PackageManagerCommands,
@@ -28,7 +32,7 @@ import {
   getPackagesInPackageManagerWorkspace,
   needsInstall,
 } from './utils/needs-install';
-import { readPackageJson } from '../../project-graph/file-utils';
+import { minimatch } from 'minimatch';
 
 const importRemoteName = '__tmp_nx_import__';
 
@@ -280,21 +284,44 @@ export async function importHandler(options: ImportOptions) {
     });
   }
 
+  await handleMissingWorkspacesEntry(
+    packageManager,
+    pmc,
+    relativeDestination,
+    destinationGitClient
+  );
+
   // If install fails, we should continue since the errors could be resolved later.
   let installFailed = false;
   if (plugins.length > 0) {
+    output.log({ title: 'Installing Plugins' });
     try {
-      output.log({ title: 'Installing Plugins' });
-      installPlugins(workspaceRoot, plugins, pmc, updatePackageScripts);
-
-      await destinationGitClient.amendCommit();
+      runPackageManagerInstallPlugins(workspaceRoot, pmc, plugins);
     } catch (e) {
       installFailed = true;
       output.error({
         title: `Install failed: ${e.message || 'Unknown error'}`,
-        bodyLines: [e.stack],
+        bodyLines: [
+          'The following plugins were not installed:',
+          ...plugins.map((p) => `- ${p}`),
+          'Run commands to install the plugins:',
+          ...plugins.map((p) => '- ' + chalk.bold(pmc.exec + ' nx add ' + p)),
+          'Please check the error below for more details.',
+          e.stack,
+        ],
       });
     }
+    if (!installFailed) {
+      const { succeededPlugins } = installPlugins(
+        workspaceRoot,
+        plugins,
+        pmc,
+        updatePackageScripts
+      );
+      if (succeededPlugins.length > 0) {
+        await destinationGitClient.amendCommit();
+      }
+    }
   } else if (await needsInstall(packageManager, originalPackageWorkspaces)) {
     try {
       output.log({
@@ -325,8 +352,6 @@ export async function importHandler(options: ImportOptions) {
     });
   }
 
-  await warnOnMissingWorkspacesEntry(packageManager, pmc, relativeDestination);
-
   if (source != destination) {
     output.warn({
       title: `Check configuration files`,
@@ -391,13 +416,15 @@ async function createTemporaryRemote(
   await destinationGitClient.fetch(remoteName);
 }
 
-// If the user imports a project that isn't in NPM/Yarn/PNPM workspaces, then its dependencies
-// will not be installed. We should warn users and provide instructions on how to fix this.
-async function warnOnMissingWorkspacesEntry(
+/**
+ * If the user imports a project that isn't in the workspaces entry, we should add that path to the workspaces entry.
+ */
+async function handleMissingWorkspacesEntry(
   pm: PackageManager,
   pmc: PackageManagerCommands,
-  pkgPath: string
-) {
+  pkgPath: string,
+  destinationGitClient: GitRepository
+): Promise<void> {
   if (!isWorkspacesEnabled(pm, workspaceRoot)) {
     output.warn({
       title: `Missing workspaces in package.json`,
@@ -428,39 +455,47 @@ async function warnOnMissingWorkspacesEntry(
             ],
     });
   } else {
-    // Check if the new package is included in existing workspaces entries. If not, warn the user.
-    let workspaces: string[] | null = null;
-
-    if (pm === 'npm' || pm === 'yarn' || pm === 'bun') {
-      const packageJson = readPackageJson();
-      workspaces = packageJson.workspaces;
-    } else if (pm === 'pnpm') {
-      const yamlPath = join(workspaceRoot, 'pnpm-workspace.yaml');
-      if (existsSync(yamlPath)) {
-        const yamlContent = await fsp.readFile(yamlPath, 'utf-8');
-        const yaml = yamlLoad(yamlContent);
-        workspaces = yaml.packages;
-      }
+    let workspaces: string[] = getPackageWorkspaces(pm, workspaceRoot) ?? [];
+    const isPkgIncluded = workspaces.some((w) => minimatch(pkgPath, w));
+    if (isPkgIncluded) {
+      return;
     }
 
-    if (workspaces) {
-      const isPkgIncluded = workspaces.some((w) => minimatch(pkgPath, w));
-      if (!isPkgIncluded) {
-        const pkgsDir = dirname(pkgPath);
-        output.warn({
-          title: `Project missing in workspaces`,
-          bodyLines:
-            pm === 'npm' || pm === 'yarn' || pm === 'bun'
-              ? [
-                  `The imported project (${pkgPath}) is missing the "workspaces" field in package.json.`,
-                  `Add "${pkgsDir}/*" to workspaces run "${pmc.install}".`,
-                ]
-              : [
-                  `The imported project (${pkgPath}) is missing the "packages" field in pnpm-workspaces.yaml.`,
-                  `Add "${pkgsDir}/*" to packages run "${pmc.install}".`,
-                ],
-        });
-      }
+    let added = false;
+    try {
+      added = addPackagePathToWorkspaces(pm, workspaceRoot, pkgPath);
+    } catch {
+      // ignore error
+    }
+    if (added) {
+      await destinationGitClient.amendCommit();
+      output.success({
+        title: `Project added in workspaces`,
+        bodyLines:
+          pm === 'npm' || pm === 'yarn' || pm === 'bun'
+            ? [
+                `The imported project (${pkgPath}) is missing the "workspaces" field in package.json.`,
+                `Added "${pkgPath}" to workspaces.`,
+              ]
+            : [
+                `The imported project (${pkgPath}) is missing the "packages" field in pnpm-workspaces.yaml.`,
+                `Added "${pkgPath}" to packages.`,
+              ],
+      });
+    } else {
+      output.warn({
+        title: `Project missing in workspaces`,
+        bodyLines:
+          pm === 'npm' || pm === 'yarn' || pm === 'bun'
+            ? [
+                `The imported project (${pkgPath}) is missing the "workspaces" field in package.json.`,
+                `Add "${pkgPath}" to workspaces run "${pmc.install}".`,
+              ]
+            : [
+                `The imported project (${pkgPath}) is missing the "packages" field in pnpm-workspaces.yaml.`,
+                `Add "${pkgPath}" to packages run "${pmc.install}".`,
+              ],
+      });
     }
   }
 }