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): add workspaces path if package path is not included #28824

Merged
merged 1 commit into from
Dec 6, 2024
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
29 changes: 24 additions & 5 deletions e2e/nx/src/import.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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`);
});
Expand Down Expand Up @@ -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 .`, {
Expand Down Expand Up @@ -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');
});
});
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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"
},
Expand Down
19 changes: 10 additions & 9 deletions packages/nx/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
Expand Down
84 changes: 41 additions & 43 deletions packages/nx/src/command-line/import/import.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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,
Expand All @@ -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__';

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -325,8 +332,6 @@ export async function importHandler(options: ImportOptions) {
});
}

await warnOnMissingWorkspacesEntry(packageManager, pmc, relativeDestination);

if (source != destination) {
output.warn({
title: `Check configuration files`,
Expand Down Expand Up @@ -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`,
Expand Down Expand Up @@ -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.`,
],
});
}
}
9 changes: 5 additions & 4 deletions packages/nx/src/plugins/package-json/create-nodes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand All @@ -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/*'])
);
Expand Down
4 changes: 2 additions & 2 deletions packages/nx/src/project-graph/file-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion packages/nx/src/utils/fileutils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
Loading
Loading