Skip to content

Commit

Permalink
fix(core): add workspaces path if package path is not included
Browse files Browse the repository at this point in the history
  • Loading branch information
xiongemi committed Nov 8, 2024
1 parent 6882ad1 commit 0cf563d
Show file tree
Hide file tree
Showing 5 changed files with 128 additions and 52 deletions.
39 changes: 20 additions & 19 deletions e2e/nx/src/import.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ import {
newProject,
runCLI,
runCommand,
updateJson,
updateFile,
e2eCwd,
readJson,
readFile,
} from '@nx/e2e/utils';
import { writeFileSync, mkdirSync, rmdirSync } from 'fs';
import { execSync } from 'node:child_process';
Expand All @@ -21,24 +21,13 @@ describe('Nx Import', () => {
packages: ['@nx/js'],
});

if (getSelectedPackageManager() === 'pnpm') {
updateFile(
'pnpm-workspace.yaml',
`packages:
- 'projects/*'
`
);
} else {
updateJson('package.json', (json) => {
json.workspaces = ['projects/*'];
return json;
});
}

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 @@ -85,6 +74,14 @@ describe('Nx Import', () => {
}
);

if (getSelectedPackageManager() === 'pnpm') {
const workspaceYaml = readFile('pnpm-workspace.yaml');
expect(workspaceYaml).toMatch(/(projects\/vite-app)/);
} else {
const packageJson = readJson('package.json');
expect(packageJson.workspaces).toContain('projects/vite-app');
}

checkFilesExist(
'projects/vite-app/.gitignore',
'projects/vite-app/package.json',
Expand All @@ -110,9 +107,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
54 changes: 26 additions & 28 deletions packages/nx/src/command-line/import/import.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import { dirname, isAbsolute, join, relative, resolve } from 'path';
import { isAbsolute, join, relative, resolve } from 'path';
import { minimatch } from 'minimatch';
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 +12,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 +29,6 @@ import {
getPackagesInPackageManagerWorkspace,
needsInstall,
} from './utils/needs-install';
import { readPackageJson } from '../../project-graph/file-utils';

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,14 +396,18 @@ 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
) {
if (!isWorkspacesEnabled(pm, workspaceRoot)) {
addPackagePathToWorkspaces(pm, workspaceRoot, pkgPath);
await destinationGitClient.amendCommit();
output.warn({
title: `Missing workspaces in package.json`,
bodyLines:
Expand All @@ -411,53 +420,42 @@ async function warnOnMissingWorkspacesEntry(
: pm === 'yarn'
? [
`We recommend enabling Yarn workspaces to install dependencies for the imported project.`,
`Add \`"workspaces": ["${pkgPath}"]\` to package.json and run "${pmc.install}".`,
`Added \`"workspaces": ["${pkgPath}"]\` to package.json and run.`,
`See: https://yarnpkg.com/features/workspaces`,
]
: pm === 'bun'
? [
`We recommend enabling Bun workspaces to install dependencies for the imported project.`,
`Add \`"workspaces": ["${pkgPath}"]\` to package.json and run "${pmc.install}".`,
`Added \`"workspaces": ["${pkgPath}"]\` to package.json.`,
`See: https://bun.sh/docs/install/workspaces`,
]
: [
`We recommend enabling PNPM workspaces to install dependencies for the imported project.`,
`Add the following entry to to pnpm-workspace.yaml and run "${pmc.install}":`,
`Added the following entry to to pnpm-workspace.yaml:`,
chalk.bold(`packages:\n - '${pkgPath}'`),
`See: https://pnpm.io/workspaces`,
],
});
} 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[] | null = getPackageWorkspaces(pm, workspaceRoot);

if (workspaces) {
const isPkgIncluded = workspaces.some((w) => minimatch(pkgPath, w));
if (!isPkgIncluded) {
const pkgsDir = dirname(pkgPath);
addPackagePathToWorkspaces(pm, workspaceRoot, pkgPath);
await destinationGitClient.amendCommit();
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}".`,
`Added "${pkgPath}" to workspaces.`,
]
: [
`The imported project (${pkgPath}) is missing the "packages" field in pnpm-workspaces.yaml.`,
`Add "${pkgsDir}/*" to packages run "${pmc.install}".`,
`Added "${pkgPath}" to packages.`,
],
});
}
Expand Down
4 changes: 4 additions & 0 deletions packages/nx/src/command-line/init/init-v2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,10 @@ export async function detectPlugins(
}
}
}
// If both Vite and Remix are detected, use the Remix plugin
if (detectedPlugins.has('@nx/remix') && detectedPlugins.has('@nx/vite')) {
detectedPlugins.delete('@nx/vite');
}

let gradlewFiles = ['gradlew', 'gradlew.bat'].concat(
globWithWorkspaceContextSync(process.cwd(), [
Expand Down
78 changes: 77 additions & 1 deletion packages/nx/src/utils/package-manager.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
import { exec, execSync } from 'child_process';
import { copyFileSync, existsSync, writeFileSync } from 'fs';
import { copyFileSync, existsSync, writeFileSync, readFileSync } from 'fs';
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 { load as yamlLoad, dump as yamlDump } from '@zkochan/js-yaml';
import { minimatch } from 'minimatch';

import { readNxJson } from '../config/configuration';
import { readPackageJson } from '../project-graph/file-utils';
import { readFileIfExisting, readJsonFile, writeJsonFile } from './fileutils';
Expand Down Expand Up @@ -478,3 +481,76 @@ export async function packageRegistryPack(
const tarballPath = stdout.trim();
return { tarballPath };
}

/**
* This function returns the workspaces defined in the package manager configuration.
* @returns workspaces defined in the package manager configuration, null if not found
*/
export function getPackageWorkspaces(
packageManager: PackageManager = detectPackageManager(),
root: string = workspaceRoot
): string[] | null {
let workspaces: string[] | null = null;

if (
packageManager === 'npm' ||
packageManager === 'yarn' ||
packageManager === 'bun'
) {
const packageJson = readPackageJson();
workspaces = packageJson.workspaces;
} else if (packageManager === 'pnpm') {
const yamlPath = join(root, 'pnpm-workspace.yaml');
if (existsSync(yamlPath)) {
const yamlContent = readFileSync(yamlPath, 'utf-8');

const yaml = yamlLoad(yamlContent);
workspaces = yaml.packages;
}
}

return workspaces;
}

/**
* This function 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 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(
packageManager: PackageManager = detectPackageManager(),
root: string = workspaceRoot,
packagePath: string
): void {
let workspaces: string[] = getPackageWorkspaces(packageManager, root) ?? [];
const isPkgIncluded = workspaces.some((w) => minimatch(packagePath, w));
if (isPkgIncluded) {
return;
}

workspaces.push(packagePath);
if (
packageManager === 'npm' ||
packageManager === 'yarn' ||
packageManager === 'bun'
) {
const packageJson = readPackageJson();
const updatedPackageJson = {
...packageJson,
workspaces,
};
const packageJsonPath = join(root, 'package.json');
writeJsonFile(packageJsonPath, updatedPackageJson);
} else if (packageManager === 'pnpm') {
const yamlPath = join(root, 'pnpm-workspace.yaml');
let yaml: any = {};
if (existsSync(yamlPath)) {
const yamlContent = readFileSync(yamlPath, 'utf-8');
yaml = yamlLoad(yamlContent);
}
yaml.packages = workspaces;
writeFileSync(yamlPath, yamlDump(yaml));
}
}
5 changes: 1 addition & 4 deletions packages/remix/src/generators/init/init.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,7 @@ import {
runTasksInSerial,
createProjectGraphAsync,
} from '@nx/devkit';
import {
addPlugin,
generateCombinations,
} from '@nx/devkit/src/utils/add-plugin';
import { addPlugin } from '@nx/devkit/src/utils/add-plugin';
import { assertNotUsingTsSolutionSetup } from '@nx/js/src/utils/typescript/ts-solution-setup';
import { createNodesV2 } from '../../plugins/plugin';
import { nxVersion, remixVersion } from '../../utils/versions';
Expand Down

0 comments on commit 0cf563d

Please sign in to comment.