From d330fa624bee80d52bf9b3e42d6472fbffda8af6 Mon Sep 17 00:00:00 2001 From: Miroslav Jonas Date: Mon, 3 Apr 2023 17:53:02 +0200 Subject: [PATCH] feat(core): include PR review notes --- docs/generated/devkit/nrwl_devkit.md | 9 +-- .../packages/devkit/documents/nrwl_devkit.md | 9 +-- e2e/storybook/src/storybook-nested.test.ts | 1 - .../src/create-nx-workspace-npm.test.ts | 76 ++++++------------- .../src/create-nx-workspace.test.ts | 69 +++-------------- packages/devkit/src/utils/package-json.ts | 19 ++--- packages/nx/src/utils/package-manager.ts | 6 +- 7 files changed, 53 insertions(+), 136 deletions(-) diff --git a/docs/generated/devkit/nrwl_devkit.md b/docs/generated/devkit/nrwl_devkit.md index 71c85e0c724a16..a253317955679d 100644 --- 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 index 71c85e0c724a16..a253317955679d 100644 --- 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 index 812b5f1ac81ab9..51e93312212fdb 100644 --- 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 index 2bda0d9fecd001..0ba9a1979a7591 100644 --- 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 originalDaemon; + beforeAll(() => { - originalVerbose = process.env.NX_VERBOSE_LOGGING; - process.env.NX_VERBOSE_LOGGING = 'true'; + originalDaemon = process.env.NX_DAEMON; + process.env.NX_DAEMON = 'false'; + runCreateWorkspace(wsName, { preset: 'npm', packageManager: getSelectedPackageManager(), @@ -28,14 +27,11 @@ describe('create-nx-workspace --preset=npm', () => { afterEach(() => { runCommand(`git reset --hard HEAD`); - execSync(`${getPackageManagerCommand().runNx} reset`, { - cwd: `${e2eCwd}/${wsName}`, - stdio: 'pipe', - }); + runCommand(`git clean -fd`); }); afterAll(() => { - process.env.NX_VERBOSE_LOGGING = originalVerbose; + process.env.NX_DAEMON = originalDaemon; cleanupProject({ skipReset: true }); }); @@ -44,9 +40,7 @@ describe('create-nx-workspace --preset=npm', () => { const appName = uniq('my-app'); expect(() => { - runCLI( - `generate @nrwl/angular:app ${appName} --skipPackageJson --no-interactive` - ); + runCLI(`generate @nrwl/angular:app ${appName} --no-interactive`); }).not.toThrowError(); checkFilesExist('tsconfig.base.json'); }, 1_000_000); @@ -56,9 +50,7 @@ describe('create-nx-workspace --preset=npm', () => { const libName = uniq('lib'); expect(() => { - runCLI( - `generate @nrwl/angular:lib ${libName} --skipPackageJson --no-interactive` - ); + runCLI(`generate @nrwl/angular:lib ${libName} --no-interactive`); }).not.toThrowError(); checkFilesExist('tsconfig.base.json'); const tsconfig = readJson(`tsconfig.base.json`); @@ -73,9 +65,7 @@ describe('create-nx-workspace --preset=npm', () => { const libName = uniq('lib'); expect(() => - runCLI( - `generate @nrwl/js:library ${libName} --skipPackageJson --no-interactive` - ) + runCLI(`generate @nrwl/js:library ${libName} --no-interactive`) ).not.toThrowError(); checkFilesExist('tsconfig.base.json'); const tsconfig = readJson(`tsconfig.base.json`); @@ -90,9 +80,7 @@ describe('create-nx-workspace --preset=npm', () => { const appName = uniq('my-app'); expect(() => - runCLI( - `generate @nrwl/web:app ${appName} --skipPackageJson --no-interactive` - ) + runCLI(`generate @nrwl/web:app ${appName} --no-interactive`) ).not.toThrowError(); checkFilesExist('tsconfig.base.json'); }); @@ -103,9 +91,7 @@ describe('create-nx-workspace --preset=npm', () => { const appName = uniq('my-app'); expect(() => { - runCLI( - `generate @nrwl/react:app ${appName} --skipPackageJson --no-interactive` - ); + runCLI(`generate @nrwl/react:app ${appName} --no-interactive`); }).not.toThrowError(); checkFilesExist('tsconfig.base.json'); }); @@ -116,9 +102,7 @@ describe('create-nx-workspace --preset=npm', () => { const libName = uniq('lib'); expect(() => { - runCLI( - `generate @nrwl/react:lib ${libName} --skipPackageJson --no-interactive` - ); + runCLI(`generate @nrwl/react:lib ${libName} --no-interactive`); }).not.toThrowError(); checkFilesExist('tsconfig.base.json'); const tsconfig = readJson(`tsconfig.base.json`); @@ -133,9 +117,7 @@ describe('create-nx-workspace --preset=npm', () => { const appName = uniq('my-app'); expect(() => { - runCLI( - `generate @nrwl/next:app ${appName} --skipPackageJson --no-interactive` - ); + runCLI(`generate @nrwl/next:app ${appName} --no-interactive`); }).not.toThrowError(); checkFilesExist('tsconfig.base.json'); }); @@ -146,9 +128,7 @@ describe('create-nx-workspace --preset=npm', () => { const libName = uniq('lib'); expect(() => { - runCLI( - `generate @nrwl/next:lib ${libName} --skipPackageJson --no-interactive` - ); + runCLI(`generate @nrwl/next:lib ${libName} --no-interactive`); }).not.toThrowError(); checkFilesExist('tsconfig.base.json'); const tsconfig = readJson(`tsconfig.base.json`); @@ -164,7 +144,7 @@ describe('create-nx-workspace --preset=npm', () => { expect(() => { runCLI( - `generate @nrwl/react-native:app ${appName} --install=false --skipPackageJson --no-interactive` + `generate @nrwl/react-native:app ${appName} --install=false --no-interactive` ); }).not.toThrowError(); checkFilesExist('tsconfig.base.json'); @@ -176,9 +156,7 @@ describe('create-nx-workspace --preset=npm', () => { const libName = uniq('lib'); expect(() => { - runCLI( - `generate @nrwl/react-native:lib ${libName} --skipPackageJson --no-interactive` - ); + runCLI(`generate @nrwl/react-native:lib ${libName} --no-interactive`); }).not.toThrowError(); checkFilesExist('tsconfig.base.json'); const tsconfig = readJson(`tsconfig.base.json`); @@ -193,9 +171,7 @@ describe('create-nx-workspace --preset=npm', () => { const appName = uniq('my-app'); expect(() => { - runCLI( - `generate @nrwl/node:app ${appName} --skipPackageJson --no-interactive` - ); + runCLI(`generate @nrwl/node:app ${appName} --no-interactive`); }).not.toThrowError(); checkFilesExist('tsconfig.base.json'); }); @@ -206,9 +182,7 @@ describe('create-nx-workspace --preset=npm', () => { const libName = uniq('lib'); expect(() => { - runCLI( - `generate @nrwl/node:lib ${libName} --skipPackageJson --no-interactive` - ); + runCLI(`generate @nrwl/node:lib ${libName} --no-interactive`); }).not.toThrowError(); checkFilesExist('tsconfig.base.json'); const tsconfig = readJson(`tsconfig.base.json`); @@ -223,9 +197,7 @@ describe('create-nx-workspace --preset=npm', () => { const appName = uniq('my-app'); expect(() => { - runCLI( - `generate @nrwl/nest:app ${appName} --skipPackageJson --no-interactive` - ); + runCLI(`generate @nrwl/nest:app ${appName} --no-interactive`); }).not.toThrowError(); checkFilesExist('tsconfig.base.json'); }); @@ -236,9 +208,7 @@ describe('create-nx-workspace --preset=npm', () => { const libName = uniq('lib'); expect(() => { - runCLI( - `generate @nrwl/nest:lib ${libName} --skipPackageJson --no-interactive` - ); + runCLI(`generate @nrwl/nest:lib ${libName} --no-interactive`); }).not.toThrowError(); checkFilesExist('tsconfig.base.json'); const tsconfig = readJson(`tsconfig.base.json`); @@ -253,9 +223,7 @@ describe('create-nx-workspace --preset=npm', () => { const appName = uniq('my-app'); expect(() => { - runCLI( - `generate @nrwl/express:app ${appName} --skipPackageJson --no-interactive` - ); + runCLI(`generate @nrwl/express:app ${appName} --no-interactive`); }).not.toThrowError(); checkFilesExist('tsconfig.base.json'); }); diff --git a/e2e/workspace-create/src/create-nx-workspace.test.ts b/e2e/workspace-create/src/create-nx-workspace.test.ts index 76fe6a4b90eca3..cd9a09a3763cc6 100644 --- 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="myemail@test.com" --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 index 8a427e627975c9..b2726a34f96092 100644 --- a/packages/devkit/src/utils/package-json.ts +++ b/packages/devkit/src/utils/package-json.ts @@ -453,16 +453,17 @@ export function ensurePackage( 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 index 6156b422de4901..c41165b979aee2 100644 --- 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