feat(core): include PR review notes

nrwl · Apr 4, 2023 · 381f049 · 381f049
1 parent 7fc2e8a
commit 381f049
Show file tree

Hide file tree

Showing 7 changed files with 36 additions and 90 deletions.
diff --git a/docs/generated/devkit/nrwl_devkit.md b/docs/generated/devkit/nrwl_devkit.md
@@ -1433,7 +1433,7 @@ Returns the list of outputs that will be cached.
 
 ### getPackageManagerCommand
 
-▸ **getPackageManagerCommand**(`packageManager?`, `root?`): `PackageManagerCommands`
+▸ **getPackageManagerCommand**(`packageManager?`): `PackageManagerCommands`
 
 Returns commands for the package manager used in the workspace.
 By default, the package manager is derived based on the lock file,
@@ -1447,10 +1447,9 @@ execSync(`${getPackageManagerCommand().addDev} my-dev-package`);
 
 #### Parameters
 
-| Name             | Type                                                                  | Default value   |
-| :--------------- | :-------------------------------------------------------------------- | :-------------- |
-| `packageManager` | [`PackageManager`](../../devkit/documents/nrwl_devkit#packagemanager) | `undefined`     |
-| `root`           | `string`                                                              | `workspaceRoot` |
+| Name             | Type                                                                  |
+| :--------------- | :-------------------------------------------------------------------- |
+| `packageManager` | [`PackageManager`](../../devkit/documents/nrwl_devkit#packagemanager) |
 
 #### Returns
 

diff --git a/docs/generated/packages/devkit/documents/nrwl_devkit.md b/docs/generated/packages/devkit/documents/nrwl_devkit.md
@@ -1433,7 +1433,7 @@ Returns the list of outputs that will be cached.
 
 ### getPackageManagerCommand
 
-▸ **getPackageManagerCommand**(`packageManager?`, `root?`): `PackageManagerCommands`
+▸ **getPackageManagerCommand**(`packageManager?`): `PackageManagerCommands`
 
 Returns commands for the package manager used in the workspace.
 By default, the package manager is derived based on the lock file,
@@ -1447,10 +1447,9 @@ execSync(`${getPackageManagerCommand().addDev} my-dev-package`);
 
 #### Parameters
 
-| Name             | Type                                                                  | Default value   |
-| :--------------- | :-------------------------------------------------------------------- | :-------------- |
-| `packageManager` | [`PackageManager`](../../devkit/documents/nrwl_devkit#packagemanager) | `undefined`     |
-| `root`           | `string`                                                              | `workspaceRoot` |
+| Name             | Type                                                                  |
+| :--------------- | :-------------------------------------------------------------------- |
+| `packageManager` | [`PackageManager`](../../devkit/documents/nrwl_devkit#packagemanager) |
 
 #### Returns
 

diff --git a/e2e/storybook/src/storybook-nested.test.ts b/e2e/storybook/src/storybook-nested.test.ts
@@ -17,7 +17,6 @@ describe('Storybook generators and executors for standalone workspaces - using R
 
   beforeAll(() => {
     // create a workspace with a single react app at the root
-    // TODO @meeroslav: we always need to pass in `packageManager` here, otherwise `runCreateWorkspace` will use `npm` by default
     runCreateWorkspace(wsName, {
       preset: 'react-standalone',
       appName,

diff --git a/e2e/workspace-create-npm/src/create-nx-workspace-npm.test.ts b/e2e/workspace-create-npm/src/create-nx-workspace-npm.test.ts
@@ -1,8 +1,6 @@
 import {
   checkFilesExist,
   cleanupProject,
-  e2eCwd,
-  getPackageManagerCommand,
   getSelectedPackageManager,
   packageInstall,
   readJson,
@@ -11,15 +9,16 @@ import {
   runCreateWorkspace,
   uniq,
 } from '@nrwl/e2e/utils';
-import { execSync } from 'child_process';
 
 describe('create-nx-workspace --preset=npm', () => {
   const wsName = uniq('npm');
 
-  let originalVerbose;
+  let originalVerbose, originalDaemon;
   beforeAll(() => {
     originalVerbose = process.env.NX_VERBOSE_LOGGING;
+    originalDaemon = process.env.NX_DAEMON;
     process.env.NX_VERBOSE_LOGGING = 'true';
+    process.env.NX_DAEMON = 'false';
     runCreateWorkspace(wsName, {
       preset: 'npm',
       packageManager: getSelectedPackageManager(),
@@ -28,14 +27,12 @@ describe('create-nx-workspace --preset=npm', () => {
 
   afterEach(() => {
     runCommand(`git reset --hard HEAD`);
-    execSync(`${getPackageManagerCommand().runNx} reset`, {
-      cwd: `${e2eCwd}/${wsName}`,
-      stdio: 'pipe',
-    });
+    runCommand(`git clean -fdx`);
   });
 
   afterAll(() => {
     process.env.NX_VERBOSE_LOGGING = originalVerbose;
+    process.env.NX_DAEMON = originalDaemon;
     cleanupProject({ skipReset: true });
   });
 

diff --git a/e2e/workspace-create/src/create-nx-workspace.test.ts b/e2e/workspace-create/src/create-nx-workspace.test.ts
@@ -19,25 +19,6 @@ describe('create-nx-workspace', () => {
 
   afterEach(() => cleanupProject());
 
-  it('should create a workspace with a single angular app at the root with routing', () => {
-    const wsName = uniq('angular');
-
-    runCreateWorkspace(wsName, {
-      preset: 'angular-standalone',
-      appName: wsName,
-      style: 'css',
-      packageManager,
-      standaloneApi: false,
-      routing: true,
-    });
-
-    checkFilesExist('package.json');
-    checkFilesExist('src/app/app.routes.ts');
-    checkFilesDoNotExist('tsconfig.base.json');
-    checkFilesExist('project.json');
-    expectCodeIsFormatted();
-  });
-
   it('should create a workspace with a single angular app at the root without routing', () => {
     const wsName = uniq('angular');
 
@@ -46,6 +27,7 @@ describe('create-nx-workspace', () => {
       appName: wsName,
       style: 'css',
       packageManager,
+      standaloneApi: false,
       routing: false,
     });
 
@@ -70,6 +52,7 @@ describe('create-nx-workspace', () => {
 
     checkFilesExist('package.json');
     checkFilesExist('project.json');
+    checkFilesExist('src/app/app.routes.ts');
     checkFilesDoNotExist('src/app/app.module.ts');
     expectCodeIsFormatted();
   });
@@ -113,7 +96,7 @@ describe('create-nx-workspace', () => {
   it('should be able to create an empty workspace built for apps', () => {
     const wsName = uniq('apps');
     runCreateWorkspace(wsName, {
-      preset: 'empty',
+      preset: 'apps',
       packageManager,
     });
 
@@ -123,9 +106,6 @@ describe('create-nx-workspace', () => {
       'apps/.gitkeep',
       'libs/.gitkeep'
     );
-    const foreignLockFiles = Object.keys(packageManagerLockFile)
-      .filter((pm) => pm !== packageManager)
-      .map((pm) => packageManagerLockFile[pm]);
 
     expectNoAngularDevkit();
   });
@@ -179,7 +159,7 @@ describe('create-nx-workspace', () => {
         appName,
         packageManager,
         standaloneApi: false,
-        routing: true,
+        routing: false,
       })
     ).toThrow();
   });
@@ -293,7 +273,7 @@ describe('create-nx-workspace', () => {
   it('should be able to create a workspace with a custom base branch and HEAD', () => {
     const wsName = uniq('branch');
     runCreateWorkspace(wsName, {
-      preset: 'empty',
+      preset: 'apps',
       base: 'main',
       packageManager,
     });
@@ -302,7 +282,7 @@ describe('create-nx-workspace', () => {
   it('should be able to create a workspace with custom commit information', () => {
     const wsName = uniq('branch');
     runCreateWorkspace(wsName, {
-      preset: 'empty',
+      preset: 'apps',
       extraArgs:
         '--commit.name="John Doe" --commit.email="[email protected]" --commit.message="Custom commit message!"',
       packageManager,
@@ -322,51 +302,24 @@ describe('create-nx-workspace', () => {
 
   it('should respect package manager preference', () => {
     const wsName = uniq('pm');
-    const appName = uniq('app');
-
-    process.env.YARN_REGISTRY = `http://localhost:4872`;
-    process.env.SELECTED_PM = 'npm';
-
-    runCreateWorkspace(wsName, {
-      preset: 'react-monorepo',
-      style: 'css',
-      appName,
-      packageManager: 'npm',
-      bundler: 'webpack',
-    });
-
-    checkFilesDoNotExist('yarn.lock');
-    checkFilesExist('package-lock.json');
-    expectCodeIsFormatted();
-    process.env.SELECTED_PM = packageManager;
-  });
-
-  it('should store package manager preference for angular', () => {
-    const wsName = uniq('pm');
-    const appName = uniq('app');
 
     process.env.YARN_REGISTRY = `http://localhost:4872`;
     process.env.SELECTED_PM = 'npm';
 
     runCreateWorkspace(wsName, {
-      preset: 'angular-monorepo',
-      appName,
-      style: 'css',
+      preset: 'apps',
       packageManager: 'npm',
-      routing: true,
-      standaloneApi: false,
     });
 
     checkFilesDoNotExist('yarn.lock');
     checkFilesExist('package-lock.json');
-    expectCodeIsFormatted();
     process.env.SELECTED_PM = packageManager;
   });
 
   it('should return error when ci workflow is selected but no cloud is set up', () => {
     const wsName = uniq('github');
-    const create = runCreateWorkspace(wsName, {
-      preset: 'npm',
+    runCreateWorkspace(wsName, {
+      preset: 'apps',
       packageManager,
       ci: 'circleci',
     });
@@ -378,7 +331,7 @@ describe('create-nx-workspace', () => {
     function setupProject(envPm: 'npm' | 'yarn' | 'pnpm') {
       process.env.SELECTED_PM = envPm;
       runCreateWorkspace(uniq('pm'), {
-        preset: 'empty',
+        preset: 'apps',
         packageManager: envPm,
         useDetectedPm: true,
       });
@@ -425,7 +378,7 @@ describe('create-nx-workspace', () => {
 
 describe('create-nx-workspace custom parent folder', () => {
   const tmpDir = `${e2eCwd}/${uniq('with space')}`;
-  const wsName = uniq('empty');
+  const wsName = uniq('parent');
   const packageManager = getSelectedPackageManager() || 'pnpm';
 
   afterEach(() => cleanupProject({ cwd: `${tmpDir}/${wsName}` }));

diff --git a/packages/devkit/src/utils/package-json.ts b/packages/devkit/src/utils/package-json.ts
@@ -453,16 +453,17 @@ export function ensurePackage<T extends any = any>(
   const tempDir = dirSync().name;
 
   console.log(`Fetching ${pkg}...`);
+  const packageManager = detectPackageManager();
+  let addCommand = getPackageManagerCommand(packageManager).addDev;
+  if (packageManager === 'pnpm') {
+    addCommand = 'pnpm add -D'; // we need to ensure that we are not using workspace command
+  }
+
   const isVerbose = process.env.NX_VERBOSE_LOGGING === 'true';
-  execSync(
-    `${
-      getPackageManagerCommand(detectPackageManager(), tempDir).addDev
-    } ${pkg}@${requiredVersion}`,
-    {
-      cwd: tempDir,
-      stdio: isVerbose ? 'inherit' : 'ignore',
-    }
-  );
+  execSync(`${addCommand} ${pkg}@${requiredVersion}`, {
+    cwd: tempDir,
+    stdio: isVerbose ? 'inherit' : 'ignore',
+  });
 
   addToNodePath(join(workspaceRoot, 'node_modules'));
   addToNodePath(join(tempDir, 'node_modules'));

diff --git a/packages/nx/src/utils/package-manager.ts b/packages/nx/src/utils/package-manager.ts
@@ -7,7 +7,6 @@ import { promisify } from 'util';
 import { writeJsonFile } from './fileutils';
 import { readModulePackageJson } from './package-json';
 import { gte, lt } from 'semver';
-import { workspaceRoot } from './workspace-root';
 
 const execAsync = promisify(exec);
 
@@ -47,8 +46,7 @@ export function detectPackageManager(dir: string = ''): PackageManager {
  * ```
  */
 export function getPackageManagerCommand(
-  packageManager: PackageManager = detectPackageManager(),
-  root: string = workspaceRoot
+  packageManager: PackageManager = detectPackageManager()
 ): PackageManagerCommands {
   const commands: { [pm in PackageManager]: () => PackageManagerCommands } = {
     yarn: () => {
@@ -72,7 +70,7 @@ export function getPackageManagerCommand(
       const pnpmVersion = getPackageManagerVersion('pnpm');
       const useExec = gte(pnpmVersion, '6.13.0');
       const includeDoubleDashBeforeArgs = lt(pnpmVersion, '7.0.0');
-      const isPnpmWorkspace = existsSync(join(root, 'pnpm-workspace.yaml'));
+      const isPnpmWorkspace = existsSync('pnpm-workspace.yaml');
 
       return {
         install: 'pnpm install --no-frozen-lockfile', // explicitly disable in case of CI