Skip to content

Commit

Permalink
fix(core): add workspaces path if package path is not included (#28824)
Browse files Browse the repository at this point in the history
<!-- 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 #

(cherry picked from commit 902eef5)
  • Loading branch information
xiongemi authored and FrozenPandaz committed Dec 6, 2024
1 parent 6ed78d2 commit 046686f
Show file tree
Hide file tree
Showing 10 changed files with 687 additions and 326 deletions.
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

0 comments on commit 046686f

Please sign in to comment.