Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(core): ensure proper yarn version detection even for nested projects #18052

Merged
merged 7 commits into from
Jul 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion docs/generated/devkit/nx_devkit.md
Original file line number Diff line number Diff line change
Expand Up @@ -1397,7 +1397,7 @@ execSync(`${getPackageManagerCommand().addDev} my-dev-package`);

### getPackageManagerVersion

▸ **getPackageManagerVersion**(`packageManager?`): `string`
▸ **getPackageManagerVersion**(`packageManager?`, `cwd?`): `string`

Returns the version of the package manager used in the workspace.
By default, the package manager is derived based on the lock file,
Expand All @@ -1408,6 +1408,7 @@ but it can also be passed in explicitly.
| Name | Type |
| :--------------- | :------------------------------------------------------------------ |
| `packageManager` | [`PackageManager`](../../devkit/documents/nx_devkit#packagemanager) |
| `cwd` | `string` |

#### Returns

Expand Down
3 changes: 2 additions & 1 deletion docs/generated/packages/devkit/documents/nx_devkit.md
Original file line number Diff line number Diff line change
Expand Up @@ -1397,7 +1397,7 @@ execSync(`${getPackageManagerCommand().addDev} my-dev-package`);

### getPackageManagerVersion

▸ **getPackageManagerVersion**(`packageManager?`): `string`
▸ **getPackageManagerVersion**(`packageManager?`, `cwd?`): `string`

Returns the version of the package manager used in the workspace.
By default, the package manager is derived based on the lock file,
Expand All @@ -1408,6 +1408,7 @@ but it can also be passed in explicitly.
| Name | Type |
| :--------------- | :------------------------------------------------------------------ |
| `packageManager` | [`PackageManager`](../../devkit/documents/nx_devkit#packagemanager) |
| `cwd` | `string` |

#### Returns

Expand Down
62 changes: 60 additions & 2 deletions e2e/workspace-create/src/create-nx-workspace.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,12 @@ import {
getSelectedPackageManager,
packageManagerLockFile,
readJson,
runCommand,
runCreateWorkspace,
uniq,
} from '@nx/e2e/utils';
import { existsSync, mkdirSync } from 'fs-extra';
import { readFileSync } from 'fs';
import { existsSync, mkdirSync, rmSync } from 'fs-extra';

describe('create-nx-workspace', () => {
const packageManager = getSelectedPackageManager() || 'pnpm';
Expand Down Expand Up @@ -414,7 +416,7 @@ describe('create-nx-workspace', () => {
});
});

describe('create-nx-workspace custom parent folder', () => {
describe('create-nx-workspace parent folder', () => {
const tmpDir = `${e2eCwd}/${uniq('with space')}`;
const wsName = uniq('parent');
const packageManager = getSelectedPackageManager() || 'pnpm';
Expand All @@ -433,3 +435,59 @@ describe('create-nx-workspace custom parent folder', () => {
expect(existsSync(`${tmpDir}/${wsName}/package.json`)).toBeTruthy();
});
});

describe('create-nx-workspace yarn berry', () => {
const tmpDir = `${e2eCwd}/${uniq('yarn-berry')}`;
let wsName: string;

beforeAll(() => {
mkdirSync(tmpDir, { recursive: true });
runCommand('corepack prepare yarn@stable --activate', { cwd: tmpDir });
runCommand('yarn set version stable', { cwd: tmpDir });
// previous command creates a package.json file which we don't want
rmSync(`${tmpDir}/package.json`);
process.env.YARN_ENABLE_IMMUTABLE_INSTALLS = 'false';
});

afterEach(() => cleanupProject({ cwd: `${tmpDir}/${wsName}` }));

it('should create a workspace with yarn berry', () => {
wsName = uniq('apps');

runCreateWorkspace(wsName, {
preset: 'apps',
packageManager: 'yarn',
cwd: tmpDir,
});

expect(existsSync(`${tmpDir}/${wsName}/.yarnrc.yml`)).toBeTruthy();
expect(
readFileSync(`${tmpDir}/${wsName}/.yarnrc.yml`, { encoding: 'utf-8' })
).toMatchInlineSnapshot(`
"nodeLinker: node-modules

yarnPath: .yarn/releases/yarn-3.6.1.cjs
"
`);
});

it('should create a js workspace with yarn berry', () => {
wsName = uniq('ts');

runCreateWorkspace(wsName, {
preset: 'ts',
packageManager: 'yarn',
cwd: tmpDir,
});

expect(existsSync(`${tmpDir}/${wsName}/.yarnrc.yml`)).toBeTruthy();
expect(
readFileSync(`${tmpDir}/${wsName}/.yarnrc.yml`, { encoding: 'utf-8' })
).toMatchInlineSnapshot(`
"nodeLinker: node-modules

yarnPath: .yarn/releases/yarn-3.6.1.cjs
"
`);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,9 @@ export async function createEmptyWorkspace<T extends CreateWorkspaceOptions>(
/\s/.test(nxWorkspaceRoot) &&
packageManager === 'npm'
) {
const pmVersion = +getPackageManagerVersion(packageManager).split('.')[0];
const pmVersion = +getPackageManagerVersion(packageManager, tmpDir).split(
'.'
)[0];
if (pmVersion < 7) {
nxWorkspaceRoot = `\\"${nxWorkspaceRoot.slice(1, -1)}\\"`;
}
Expand Down
5 changes: 4 additions & 1 deletion packages/create-nx-workspace/src/create-preset.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,10 @@ export async function createPreset<T extends CreateWorkspaceOptions>(
/\s/.test(nxWorkspaceRoot) &&
packageManager === 'npm'
) {
const pmVersion = +getPackageManagerVersion(packageManager).split('.')[0];
const pmVersion = +getPackageManagerVersion(
packageManager,
workingDir
).split('.')[0];
if (pmVersion < 7) {
nxWorkspaceRoot = `\\"${nxWorkspaceRoot.slice(1, -1)}\\"`;
}
Expand Down
22 changes: 18 additions & 4 deletions packages/create-nx-workspace/src/utils/package-manager.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { execSync } from 'child_process';
import { existsSync, writeFileSync } from 'fs';
import { existsSync, readFileSync, writeFileSync } from 'fs';
import { join } from 'path';

/*
Expand Down Expand Up @@ -52,7 +52,8 @@ export function getPackageManagerCommand(
install: useBerry
? installCommand
: `${installCommand} --ignore-scripts`,
exec: 'yarn',
// using npx is necessary to avoid yarn classic manipulating the version detection when using berry
exec: useBerry ? 'npx' : 'yarn',
};

case 'pnpm':
Expand Down Expand Up @@ -85,15 +86,28 @@ export function generatePackageManagerFiles(
join(root, '.yarnrc.yml'),
'nodeLinker: node-modules\nenableScripts: false'
);
// avoids errors when using nested yarn projects
writeFileSync(join(root, 'yarn.lock'), '');
}
break;
}
}

const pmVersionCache = new Map<PackageManager, string>();

export function getPackageManagerVersion(
packageManager: PackageManager
packageManager: PackageManager,
cwd = process.cwd()
): string {
return execSync(`${packageManager} --version`).toString('utf-8').trim();
if (pmVersionCache.has(packageManager)) {
return pmVersionCache.get(packageManager) as string;
}
const version = execSync(`${packageManager} --version`, {
cwd,
encoding: 'utf-8',
}).trim();
pmVersionCache.set(packageManager, version);
return version;
}

/**
Expand Down
10 changes: 7 additions & 3 deletions packages/devkit/src/tasks/install-packages-task.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { execSync } from 'child_process';
import { type ExecSyncOptions, execSync } from 'child_process';
import { join } from 'path';
import { requireNx } from '../../nx';

Expand Down Expand Up @@ -38,9 +38,13 @@ export function installPackagesTask(
if (storedPackageJsonValue != packageJsonValue || alwaysRun) {
global['__packageJsonInstallCache__'] = packageJsonValue;
const pmc = getPackageManagerCommand(packageManager);
execSync(pmc.install, {
const execSyncOptions: ExecSyncOptions = {
cwd: join(tree.root, cwd),
stdio: process.env.NX_GENERATE_QUIET === 'true' ? 'ignore' : 'inherit',
});
};
if (pmc.preInstall) {
execSync(pmc.preInstall, execSyncOptions);
}
execSync(pmc.install, execSyncOptions);
}
}
12 changes: 8 additions & 4 deletions packages/nx/src/utils/package-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ export function getPackageManagerCommand(
): PackageManagerCommands {
const commands: { [pm in PackageManager]: () => PackageManagerCommands } = {
yarn: () => {
const yarnVersion = getPackageManagerVersion('yarn');
const yarnVersion = getPackageManagerVersion('yarn', root);
const useBerry = gte(yarnVersion, '2.0.0');

return {
Expand All @@ -81,7 +81,7 @@ export function getPackageManagerCommand(
};
},
pnpm: () => {
const pnpmVersion = getPackageManagerVersion('pnpm');
const pnpmVersion = getPackageManagerVersion('pnpm', root);
const useExec = gte(pnpmVersion, '6.13.0');
const includeDoubleDashBeforeArgs = lt(pnpmVersion, '7.0.0');
const isPnpmWorkspace = existsSync(join(root, 'pnpm-workspace.yaml'));
Expand Down Expand Up @@ -126,9 +126,13 @@ export function getPackageManagerCommand(
* but it can also be passed in explicitly.
*/
export function getPackageManagerVersion(
packageManager: PackageManager = detectPackageManager()
packageManager: PackageManager = detectPackageManager(),
cwd = process.cwd()
): string {
return execSync(`${packageManager} --version`).toString('utf-8').trim();
return execSync(`${packageManager} --version`, {
cwd,
encoding: 'utf-8',
}).trim();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ describe('@nx/workspace:generateWorkspaceFiles', () => {

beforeEach(() => {
tree = createTree();
// we need an actual path for the package manager version check
tree.root = process.cwd();
});

it('should create files', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
writeJson,
} from '@nx/devkit';
import { nxVersion } from '../../utils/versions';
import { join, join as pathJoin } from 'path';
import { join } from 'path';
import { Preset } from '../utils/presets';
import { deduceDefaultBase } from '../../utilities/default-base';
import { NormalizedSchema } from './new';
Expand All @@ -22,18 +22,26 @@ export async function generateWorkspaceFiles(
if (!options.name) {
throw new Error(`Invalid options, "name" is required.`);
}
// we need to check package manager version before the package.json is generated
// since it might influence the version report
const packageManagerVersion = getPackageManagerVersion(
options.packageManager as PackageManager,
tree.root
);
options = normalizeOptions(options);
createReadme(tree, options);
createFiles(tree, options);
createNxJson(tree, options);

const [packageMajor] = getPackageManagerVersion(
options.packageManager as PackageManager
).split('.');
const [packageMajor] = packageManagerVersion.split('.');
if (options.packageManager === 'pnpm' && +packageMajor >= 7) {
createNpmrc(tree, options);
} else if (options.packageManager === 'yarn' && +packageMajor >= 2) {
createYarnrcYml(tree, options);
} else if (options.packageManager === 'yarn') {
if (+packageMajor >= 2) {
createYarnrcYml(tree, options);
// avoids errors when using nested yarn projects
tree.write(join(options.directory, 'yarn.lock'), '');
}
}
setPresetProperty(tree, options);
addNpmScripts(tree, options);
Expand Down Expand Up @@ -133,7 +141,7 @@ function createFiles(tree: Tree, options: NormalizedSchema) {
: options.preset === Preset.NPM || options.preset === Preset.Core
? './files-package-based-repo'
: './files-integrated-repo';
generateFiles(tree, pathJoin(__dirname, filesDirName), options.directory, {
generateFiles(tree, join(__dirname, filesDirName), options.directory, {
formattedNames,
dot: '.',
tmpl: '',
Expand Down
2 changes: 2 additions & 0 deletions packages/workspace/src/generators/new/new.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ describe('new', () => {

beforeEach(() => {
tree = createTree();
// we need an actual path for the package manager version check
tree.root = process.cwd();
});

it('should generate an empty nx.json', async () => {
Expand Down