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

<!-- Please make sure you have read the submission guidelines before
posting an PR -->
<!--
https://github.com/nrwl/nx/blob/master/CONTRIBUTING.md#-submitting-a-pr
-->

<!-- Please make sure that your commit message follows our format -->
<!-- Example: `fix(nx): must begin with lowercase` -->

<!-- If this is a particularly complex change or feature addition, you
can request a dedicated Nx release for this pull request branch. Mention
someone from the Nx team or the `@nrwl/nx-pipelines-reviewers` and they
will confirm if the PR warrants its own release for testing purposes,
and generate it for you if appropriate. -->

## Current Behavior
<!-- This is the behavior we have today -->
- when import to a path where is not in the workspaces, it currently
just shows a warning. however, it will cause an error like "module not
found" because there are packages not installed.

## Expected Behavior
<!-- This is the behavior we should expect with the changes in this PR
-->
- automatically add to the workspaces
![Screenshot 2024-11-08 at 12 58
42 AM](https://github.com/user-attachments/assets/d36dd093-a0ce-45c3-a783-97244741971f)



## Related Issue(s)
<!-- Please link the issue being fixed so it gets closed when this is
merged. -->

Fixes #
This commit is contained in:
Emily Xiong 2024-12-06 08:25:31 -08:00 committed by GitHub
parent 51cf34fc81
commit 902eef5320
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 687 additions and 326 deletions

View File

@ -5,9 +5,11 @@ import {
newProject,
runCLI,
runCommand,
updateJson,
updateFile,
e2eCwd,
readJson,
readFile,
updateFile,
updateJson,
} from '@nx/e2e/utils';
import { writeFileSync, mkdirSync, rmdirSync } from 'fs';
import { execSync } from 'node:child_process';
@ -38,7 +40,10 @@ describe('Nx Import', () => {
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`);
});
@ -110,9 +115,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 .`, {
@ -143,6 +152,16 @@ describe('Nx Import', () => {
}
);
if (getSelectedPackageManager() === 'pnpm') {
const workspaceYaml = readFile('pnpm-workspace.yaml');
expect(workspaceYaml).toMatch(/(packages\/a)/);
expect(workspaceYaml).toMatch(/(packages\/b)/);
} else {
const packageJson = readJson('package.json');
expect(packageJson.workspaces).toContain('packages/a');
expect(packageJson.workspaces).toContain('packages/b');
}
checkFilesExist('packages/a/README.md', 'packages/b/README.md');
});
});

View File

@ -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",
@ -315,6 +314,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"
},

View File

@ -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",

View File

@ -1,8 +1,6 @@
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';
@ -13,8 +11,10 @@ import { detectPlugins, installPlugins } 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 +28,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,6 +280,13 @@ 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) {
@ -325,8 +332,6 @@ export async function importHandler(options: ImportOptions) {
});
}
await warnOnMissingWorkspacesEntry(packageManager, pmc, relativeDestination);
if (source != destination) {
output.warn({
title: `Check configuration files`,
@ -391,13 +396,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 +435,30 @@ 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}".`,
],
});
}
}
addPackagePathToWorkspaces(pkgPath, pm, workspaces, workspaceRoot);
await destinationGitClient.amendCommit();
output.success({
title: `Project added in workspaces`,
bodyLines:
pm === 'npm' || pm === 'yarn' || pm === 'bun'
? [
`The imported project (${chalk.bold(
pkgPath
)}) is missing the "workspaces" field in package.json.`,
`Added "${chalk.bold(pkgPath)}" to workspaces.`,
]
: [
`The imported project (${chalk.bold(
pkgPath
)}) is missing the "packages" field in pnpm-workspaces.yaml.`,
`Added "${chalk.bold(pkgPath)}" to packages.`,
],
});
}
}

View File

@ -248,9 +248,10 @@ export function getGlobPatternsFromPackageManagerWorkspaces(
if (existsSync(join(root, 'pnpm-workspace.yaml'))) {
try {
const { packages } = readYamlFile<{ packages: string[] }>(
join(root, 'pnpm-workspace.yaml')
);
const { packages } =
readYamlFile<{ packages: string[] }>(
join(root, 'pnpm-workspace.yaml')
) ?? {};
patterns.push(...normalizePatterns(packages || []));
} catch (e: unknown) {
output.warn({
@ -262,7 +263,7 @@ export function getGlobPatternsFromPackageManagerWorkspaces(
if (existsSync(join(root, 'lerna.json'))) {
try {
const { packages } = readJson<any>('lerna.json');
const { packages } = readJson<any>('lerna.json') ?? {};
patterns.push(
...normalizePatterns(packages?.length > 0 ? packages : ['packages/*'])
);

View File

@ -168,9 +168,9 @@ export function defaultFileRead(filePath: string): string | null {
return readFileSync(join(workspaceRoot, filePath), 'utf-8');
}
export function readPackageJson(): any {
export function readPackageJson(root: string = workspaceRoot): any {
try {
return readJsonFile(`${workspaceRoot}/package.json`);
return readJsonFile(`${root}/package.json`);
} catch {
return {}; // if package.json doesn't exist
}

View File

@ -10,7 +10,7 @@ import {
statSync,
existsSync,
} from 'node:fs';
import { mkdir, readFile, writeFile } from 'node:fs/promises';
import { mkdir, writeFile } from 'node:fs/promises';
import { dirname } from 'path';
import * as tar from 'tar-stream';
import { createGunzip } from 'zlib';

View File

@ -1,14 +1,21 @@
import * as fs from 'fs';
import { existsSync, mkdirSync, readFileSync, rmSync, writeFileSync } from 'fs';
import { join } from 'path';
import * as childProcess from 'child_process';
import { tmpdir } from 'os';
import * as configModule from '../config/configuration';
import * as projectGraphFileUtils from '../project-graph/file-utils';
import * as fileUtils from '../utils/fileutils';
import {
addPackagePathToWorkspaces,
detectPackageManager,
getPackageManagerVersion,
getPackageWorkspaces,
isWorkspacesEnabled,
modifyYarnRcToFitNewDirectory,
modifyYarnRcYmlToFitNewDirectory,
PackageManager,
} from './package-manager';
describe('package-manager', () => {
@ -260,4 +267,221 @@ describe('package-manager', () => {
).toEqual('enableProgressBars false');
});
});
describe('getPackageWorkspaces', () => {
const tempWorkspace = join(tmpdir(), 'addPackagePathToWorkspaces');
beforeAll(() => {
if (!existsSync(tempWorkspace)) {
mkdirSync(tempWorkspace, { recursive: true });
}
});
describe.each(['npm', 'yarn', 'bun'])('%s workspaces', (packageManager) => {
it('should return workspaces from package.json', () => {
writeFileSync(
join(tempWorkspace, 'package.json'),
'{"workspaces": ["packages/*"]}'
);
const workspaces = getPackageWorkspaces(
packageManager as PackageManager,
tempWorkspace
);
expect(workspaces).toEqual(['packages/*']);
});
it('should return empty array if workspaces does not exist', () => {
writeFileSync(join(tempWorkspace, 'package.json'), '{}');
const workspaces = getPackageWorkspaces(
packageManager as PackageManager,
tempWorkspace
);
expect(workspaces).toEqual([]);
});
});
describe('pnpm workspaces', () => {
beforeEach(() => {
if (existsSync(join(tempWorkspace, 'pnpm-workspace.yaml'))) {
rmSync(join(tempWorkspace, 'pnpm-workspace.yaml'));
}
});
it('should return workspaces from package.json', () => {
writeFileSync(
join(tempWorkspace, 'pnpm-workspace.yaml'),
`packages:\n - apps/*`
);
const workspaces = getPackageWorkspaces('pnpm', tempWorkspace);
expect(workspaces).toEqual(['apps/*']);
});
it('should return empty array if workspaces is empty', () => {
writeFileSync(join(tempWorkspace, 'pnpm-workspace.yaml'), '');
const workspaces = getPackageWorkspaces('pnpm', tempWorkspace);
expect(workspaces).toEqual([]);
});
});
});
describe('addPackagePathToWorkspaces', () => {
const tempWorkspace = join(tmpdir(), 'addPackagePathToWorkspaces');
beforeAll(() => {
if (!existsSync(tempWorkspace)) {
mkdirSync(tempWorkspace, { recursive: true });
}
});
describe.each(['npm', 'yarn', 'bun'])('%s workspaces', (packageManager) => {
it('should add to workspaces if it is empty', () => {
writeFileSync(join(tempWorkspace, 'package.json'), '{}');
addPackagePathToWorkspaces(
'packages/app',
packageManager as PackageManager,
[],
tempWorkspace
);
expect(readFileSync(join(tempWorkspace, 'package.json'), 'utf-8'))
.toMatchInlineSnapshot(`
"{
"workspaces": [
"packages/app"
]
}"
`);
});
it('should add to workspaces if it is defined', () => {
writeFileSync(
join(tempWorkspace, 'package.json'),
'{"workspaces": ["test"]}'
);
addPackagePathToWorkspaces(
'packages/app',
packageManager as PackageManager,
['test'],
tempWorkspace
);
expect(readFileSync(join(tempWorkspace, 'package.json'), 'utf-8'))
.toMatchInlineSnapshot(`
"{
"workspaces": [
"test",
"packages/app"
]
}"
`);
});
it('should not add if package path is already in existing workspaces', () => {
writeFileSync(
join(tempWorkspace, 'package.json'),
'{"workspaces": ["packages/*"]}'
);
addPackagePathToWorkspaces(
'packages/app',
packageManager as PackageManager,
['packages/*'],
tempWorkspace
);
});
});
describe('pnpm workspaces', () => {
beforeEach(() => {
if (existsSync(join(tempWorkspace, 'pnpm-workspace.yaml'))) {
rmSync(join(tempWorkspace, 'pnpm-workspace.yaml'));
}
});
it('should create pnpm-workspace.yaml if it does not exist', () => {
addPackagePathToWorkspaces('packages/app', 'pnpm', [], tempWorkspace);
expect(
readFileSync(join(tempWorkspace, 'pnpm-workspace.yaml'), 'utf-8')
).toMatchInlineSnapshot(`
"packages:
- packages/app
"
`);
});
it('should add to packages if pnpm-workspace.yaml is empty', () => {
writeFileSync(join(tempWorkspace, 'pnpm-workspace.yaml'), '');
addPackagePathToWorkspaces('packages/app', 'pnpm', [], tempWorkspace);
expect(
readFileSync(join(tempWorkspace, 'pnpm-workspace.yaml'), 'utf-8')
).toMatchInlineSnapshot(`
"packages:
- packages/app
"
`);
});
it('should add to packages if packages is empty', () => {
writeFileSync(join(tempWorkspace, 'pnpm-workspace.yaml'), 'packages:');
addPackagePathToWorkspaces('packages/app', 'pnpm', [], tempWorkspace);
expect(
readFileSync(join(tempWorkspace, 'pnpm-workspace.yaml'), 'utf-8')
).toMatchInlineSnapshot(`
"packages:
- packages/app
"
`);
});
it('should add to pnpm workspace if there are packages defined', () => {
writeFileSync(
join(tempWorkspace, 'pnpm-workspace.yaml'),
`packages:\n - apps/*`
);
addPackagePathToWorkspaces('packages/app', 'pnpm', [], tempWorkspace);
expect(readFileSync(`${tempWorkspace}/pnpm-workspace.yaml`, 'utf-8'))
.toMatchInlineSnapshot(`
"packages:
- apps/*
- packages/app
"
`);
});
it('should not add to pnpm workspace if package path is already in', () => {
writeFileSync(
join(tempWorkspace, 'pnpm-workspace.yaml'),
`packages:\n - apps/*`
);
addPackagePathToWorkspaces('apps/app1', 'pnpm', [], tempWorkspace);
});
it('should preserve comments', () => {
writeFileSync(
join(tempWorkspace, 'pnpm-workspace.yaml'),
`packages:\n - apps/* # comment`
);
addPackagePathToWorkspaces('packages/app', 'pnpm', [], tempWorkspace);
expect(readFileSync(`${tempWorkspace}/pnpm-workspace.yaml`, 'utf-8'))
.toMatchInlineSnapshot(`
"packages:
- apps/* # comment
- packages/app
"
`);
});
it('should add packages key if it is not defined', () => {
writeFileSync(
join(tempWorkspace, 'pnpm-workspace.yaml'),
`something:\n - random/* # comment`
);
addPackagePathToWorkspaces('packages/app', 'pnpm', [], tempWorkspace);
expect(readFileSync(`${tempWorkspace}/pnpm-workspace.yaml`, 'utf-8'))
.toMatchInlineSnapshot(`
"something:
- random/* # comment
packages:
- packages/app
"
`);
});
});
});
});

View File

@ -1,13 +1,28 @@
import { exec, execSync } from 'child_process';
import { copyFileSync, existsSync, writeFileSync } from 'fs';
import {
Pair,
ParsedNode,
parseDocument,
stringify as YAMLStringify,
YAMLMap,
YAMLSeq,
Scalar,
} from 'yaml';
import { rm } from 'node:fs/promises';
import { dirname, join, relative } from 'path';
import { gte, lt } from 'semver';
import { dirSync } from 'tmp';
import { promisify } from 'util';
import { readNxJson } from '../config/configuration';
import { readPackageJson } from '../project-graph/file-utils';
import { readFileIfExisting, readJsonFile, writeJsonFile } from './fileutils';
import {
readFileIfExisting,
readJsonFile,
readYamlFile,
writeJsonFile,
} from './fileutils';
import { PackageJson, readModulePackageJson } from './package-json';
import { workspaceRoot } from './workspace-root';
@ -62,7 +77,7 @@ export function isWorkspacesEnabled(
}
// yarn and npm both use the same 'workspaces' property in package.json
const packageJson: PackageJson = readPackageJson();
const packageJson: PackageJson = readPackageJson(root);
return !!packageJson?.workspaces;
}
@ -478,3 +493,112 @@ export async function packageRegistryPack(
const tarballPath = stdout.trim();
return { tarballPath };
}
/**
* Gets the workspaces defined in the package manager configuration.
* @returns workspaces defined in the package manager configuration, empty array if none are defined
*/
export function getPackageWorkspaces(
packageManager: PackageManager = detectPackageManager(),
root: string = workspaceRoot
): string[] {
let workspaces: string[];
if (
packageManager === 'npm' ||
packageManager === 'yarn' ||
packageManager === 'bun'
) {
const packageJson = readPackageJson(root);
workspaces = packageJson.workspaces;
} else if (packageManager === 'pnpm') {
const pnpmWorkspacePath = join(root, 'pnpm-workspace.yaml');
if (existsSync(pnpmWorkspacePath)) {
const { packages } =
readYamlFile<{ packages: string[] }>(pnpmWorkspacePath) ?? {};
workspaces = packages;
}
}
return workspaces ?? [];
}
/**
* Adds a package to the workspaces defined in the package manager configuration.
* If the package is already included in the workspaces, it will not be added again.
* @param packageManager The package manager to use. If not provided, it will be detected based on the lock file.
* @param workspaces The workspaces to add the package to. Defaults to the workspaces defined in the package manager configuration.
* @param root The directory the commands will be ran inside of. Defaults to the current workspace's root.
* @param packagePath The path of the package to add to the workspaces
*/
export function addPackagePathToWorkspaces(
packagePath: string,
packageManager: PackageManager = detectPackageManager(),
workspaces: string[] = getPackageWorkspaces(packageManager),
root: string = workspaceRoot
): void {
if (
packageManager === 'npm' ||
packageManager === 'yarn' ||
packageManager === 'bun'
) {
workspaces.push(packagePath);
const packageJson = readPackageJson(root);
const updatedPackageJson = {
...packageJson,
workspaces,
};
const packageJsonPath = join(root, 'package.json');
writeJsonFile(packageJsonPath, updatedPackageJson);
} else if (packageManager === 'pnpm') {
const pnpmWorkspacePath = join(root, 'pnpm-workspace.yaml');
if (existsSync(pnpmWorkspacePath)) {
const pnpmWorkspaceDocument = parseDocument(
readFileIfExisting(pnpmWorkspacePath)
);
const pnpmWorkspaceContents: ParsedNode | null =
pnpmWorkspaceDocument.contents;
if (!pnpmWorkspaceContents) {
writeFileSync(
pnpmWorkspacePath,
YAMLStringify({
packages: [packagePath],
})
);
} else if (pnpmWorkspaceContents instanceof YAMLMap) {
const packages: Pair | undefined = pnpmWorkspaceContents.items.find(
(item: Pair) => {
return item.key instanceof Scalar
? item.key?.value === 'packages'
: item.key === 'packages';
}
);
if (packages) {
if (packages.value instanceof YAMLSeq === false) {
packages.value = new YAMLSeq();
}
(packages.value as YAMLSeq).items ??= [];
(packages.value as YAMLSeq).items.push(packagePath);
} else {
// if the 'packages' key doesn't exist, create it
const packagesSeq = new YAMLSeq();
packagesSeq.items ??= [];
packagesSeq.items.push(packagePath);
pnpmWorkspaceDocument.add(
pnpmWorkspaceDocument.createPair('packages', packagesSeq)
);
}
writeFileSync(pnpmWorkspacePath, YAMLStringify(pnpmWorkspaceContents));
}
} else {
// If the file doesn't exist, create it
writeFileSync(
pnpmWorkspacePath,
YAMLStringify({
packages: [packagePath],
})
);
}
}
}

512
pnpm-lock.yaml generated

File diff suppressed because it is too large Load Diff