Skip to content

Commit

Permalink
feat(core): not exit when one plugin installation failed (#28684)
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 -->
it tries to install all plugins at once. if failed, it will stop the
remaining plugins from being installed.

## Expected Behavior
<!-- This is the behavior we should expect with the changes in this PR
-->
it will continue if one plugin failed.

success message:
<img width="301" alt="Screenshot 2024-12-11 at 11 36 14 AM"
src="https://github.com/user-attachments/assets/2bc389f0-4fda-4959-afab-57594c9d600b">
failed message:
<img width="894" alt="Screenshot 2024-12-12 at 2 58 17 PM"
src="https://github.com/user-attachments/assets/7b51d5e9-f308-48c3-9b7d-bc4219802acb"
/>


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

Fixes #
  • Loading branch information
xiongemi authored Dec 16, 2024
1 parent 67d0e33 commit 1d7465b
Show file tree
Hide file tree
Showing 9 changed files with 402 additions and 190 deletions.
2 changes: 1 addition & 1 deletion e2e/eslint/src/linter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -772,7 +772,7 @@ describe('Linter', () => {
const mylib = uniq('mylib');

runCLI(
`generate @nx/node:app --name=${myapp} --linter=eslint --directory="." --e2eTestRunner=jest --no-interactive`
`generate @nx/node:app --name=${myapp} --linter=eslint --directory="." --unitTestRunner=jest --e2eTestRunner=jest --no-interactive`
);
runCLI('reset', { env: { CI: 'false' } });
verifySuccessfulStandaloneSetup(myapp);
Expand Down
15 changes: 12 additions & 3 deletions packages/devkit/src/utils/add-plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@ import {
writeJson,
} from 'nx/src/devkit-exports';
import {
isProjectConfigurationsError,
isProjectsWithNoNameError,
LoadedNxPlugin,
ProjectConfigurationsError,
retrieveProjectConfigurations,
} from 'nx/src/devkit-internals';

Expand Down Expand Up @@ -130,8 +131,12 @@ async function _addPluginInternal<PluginOptions>(
);
} catch (e) {
// Errors are okay for this because we're only running 1 plugin
if (e instanceof ProjectConfigurationsError) {
if (isProjectConfigurationsError(e)) {
projConfigs = e.partialProjectConfigurationsResult;
// ignore errors from projects with no name
if (!e.errors.every(isProjectsWithNoNameError)) {
throw e;
}
} else {
throw e;
}
Expand Down Expand Up @@ -171,8 +176,12 @@ async function _addPluginInternal<PluginOptions>(
);
} catch (e) {
// Errors are okay for this because we're only running 1 plugin
if (e instanceof ProjectConfigurationsError) {
if (isProjectConfigurationsError(e)) {
projConfigs = e.partialProjectConfigurationsResult;
// ignore errors from projects with no name
if (!e.errors.every(isProjectsWithNoNameError)) {
throw e;
}
} else {
throw e;
}
Expand Down
4 changes: 3 additions & 1 deletion packages/js/src/utils/typescript/ts-solution-setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,9 @@ export function assertNotUsingTsSolutionSetup(
],
});

process.exit(1);
throw new Error(
`The ${artifactString} doesn't yet support the existing TypeScript setup. See the error above.`
);
}

export function findRuntimeTsConfigName(
Expand Down
96 changes: 36 additions & 60 deletions packages/nx/src/command-line/add/add.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import { exec } from 'child_process';
import { existsSync } from 'fs';
import * as ora from 'ora';
import { isAngularPluginInstalled } from '../../adapter/angular-json';
import type { GeneratorsJsonEntry } from '../../config/misc-interfaces';
import * as yargsParser from 'yargs-parser';
import { readNxJson, type NxJsonConfiguration } from '../../config/nx-json';
import { runNxAsync, runNxSync } from '../../utils/child-process';
import { runNxAsync } from '../../utils/child-process';
import { writeJsonFile } from '../../utils/fileutils';
import { logger } from '../../utils/logger';
import { output } from '../../utils/output';
Expand All @@ -14,12 +13,15 @@ import {
getPackageManagerVersion,
} from '../../utils/package-manager';
import { handleErrors } from '../../utils/handle-errors';
import { getPluginCapabilities } from '../../utils/plugins';
import { nxVersion } from '../../utils/versions';
import { workspaceRoot } from '../../utils/workspace-root';
import type { AddOptions } from './command-object';
import { normalizeVersionForNxJson } from '../init/implementation/dot-nx/add-nx-scripts';
import { gte } from 'semver';
import {
installPlugin,
getFailedToInstallPluginErrorMessages,
} from '../init/configure-plugins';

export function addHandler(options: AddOptions): Promise<number> {
return handleErrors(options.verbose, async () => {
Expand Down Expand Up @@ -109,80 +111,54 @@ async function initializePlugin(
options: AddOptions,
nxJson: NxJsonConfiguration
): Promise<void> {
const capabilities = await getPluginCapabilities(workspaceRoot, pkgName, {});
const generators = capabilities?.generators;
if (!generators) {
output.log({
title: `No generators found in ${pkgName}. Skipping initialization.`,
});
return;
}
const parsedCommandArgs: { [key: string]: any } = yargsParser(
options.__overrides_unparsed__,
{
configuration: {
'parse-numbers': false,
'parse-positional-numbers': false,
'dot-notation': false,
'camel-case-expansion': false,
},
}
);

const initGenerator = findInitGenerator(generators);
if (!initGenerator) {
output.log({
title: `No "init" generator found in ${pkgName}. Skipping initialization.`,
});
return;
if (coreNxPluginVersions.has(pkgName)) {
parsedCommandArgs.keepExistingVersions = true;

if (
options.updatePackageScripts ||
(options.updatePackageScripts === undefined &&
nxJson.useInferencePlugins !== false &&
process.env.NX_ADD_PLUGINS !== 'false')
) {
parsedCommandArgs.updatePackageScripts = true;
}
}

const spinner = ora(`Initializing ${pkgName}...`);
spinner.start();

try {
const args = [];
if (coreNxPluginVersions.has(pkgName)) {
args.push(`--keepExistingVersions`);

if (
options.updatePackageScripts ||
(options.updatePackageScripts === undefined &&
nxJson.useInferencePlugins !== false &&
process.env.NX_ADD_PLUGINS !== 'false')
) {
args.push(`--updatePackageScripts`);
}
}

if (options.__overrides_unparsed__.length) {
args.push(...options.__overrides_unparsed__);
}

runNxSync(`g ${pkgName}:${initGenerator} ${args.join(' ')}`, {
stdio: [0, 1, 2],
});
await installPlugin(
pkgName,
workspaceRoot,
options.verbose,
parsedCommandArgs
);
} catch (e) {
spinner.fail();
output.addNewline();
logger.error(e);
output.error({
title: `Failed to initialize ${pkgName}. Please check the error above for more details.`,
title: `Failed to initialize ${pkgName}`,
bodyLines: getFailedToInstallPluginErrorMessages(e),
});
process.exit(1);
}

spinner.succeed();
}

function findInitGenerator(
generators: Record<string, GeneratorsJsonEntry>
): string | undefined {
if (generators['init']) {
return 'init';
}

const angularPluginInstalled = isAngularPluginInstalled();
if (angularPluginInstalled && generators['ng-add']) {
return 'ng-add';
}

return Object.keys(generators).find(
(name) =>
generators[name].aliases?.includes('init') ||
(angularPluginInstalled && generators[name].aliases?.includes('ng-add'))
);
}

function parsePackageSpecifier(
packageSpecifier: string
): [pkgName: string, version: string] {
Expand Down
123 changes: 81 additions & 42 deletions packages/nx/src/command-line/import/import.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { tmpdir } from 'tmp';
import { prompt } from 'enquirer';
import { output } from '../../utils/output';
import * as createSpinner from 'ora';
import { detectPlugins, installPlugins } from '../init/init-v2';
import { detectPlugins } from '../init/init-v2';
import { readNxJson } from '../../config/nx-json';
import { workspaceRoot } from '../../utils/workspace-root';
import {
Expand All @@ -24,11 +24,11 @@ import { runInstall } from '../init/implementation/utils';
import { getBaseRef } from '../../utils/command-line-utils';
import { prepareSourceRepo } from './utils/prepare-source-repo';
import { mergeRemoteSource } from './utils/merge-remote-source';
import {
getPackagesInPackageManagerWorkspace,
needsInstall,
} from './utils/needs-install';
import { minimatch } from 'minimatch';
import {
configurePlugins,
runPackageManagerInstallPlugins,
} from '../init/configure-plugins';

const importRemoteName = '__tmp_nx_import__';

Expand Down Expand Up @@ -60,7 +60,7 @@ export interface ImportOptions {

export async function importHandler(options: ImportOptions) {
process.env.NX_RUNNING_NX_IMPORT = 'true';
let { sourceRepository, ref, source, destination } = options;
let { sourceRepository, ref, source, destination, verbose } = options;
const destinationGitClient = new GitRepository(process.cwd());

if (await destinationGitClient.hasUncommittedChanges()) {
Expand Down Expand Up @@ -219,11 +219,6 @@ export async function importHandler(options: ImportOptions) {
}

const packageManager = detectPackageManager(workspaceRoot);

const originalPackageWorkspaces = await getPackagesInPackageManagerWorkspace(
packageManager
);

const sourceIsNxWorkspace = existsSync(join(sourceGitClient.root, 'nx.json'));

const relativeDestination = relative(
Expand Down Expand Up @@ -287,42 +282,30 @@ export async function importHandler(options: ImportOptions) {
destinationGitClient
);

// If install fails, we should continue since the errors could be resolved later.
let installFailed = false;
if (plugins.length > 0) {
try {
output.log({ title: 'Installing Plugins' });
installPlugins(workspaceRoot, plugins, pmc, updatePackageScripts);

await destinationGitClient.amendCommit();
} catch (e) {
installFailed = true;
output.error({
title: `Install failed: ${e.message || 'Unknown error'}`,
bodyLines: [e.stack],
});
}
} else if (await needsInstall(packageManager, originalPackageWorkspaces)) {
try {
output.log({
title: 'Installing dependencies for imported code',
});

runInstall(workspaceRoot, getPackageManagerCommand(packageManager));

await destinationGitClient.amendCommit();
} catch (e) {
installFailed = true;
output.error({
title: `Install failed: ${e.message || 'Unknown error'}`,
bodyLines: [e.stack],
});
let installed = await runInstallDestinationRepo(
packageManager,
destinationGitClient
);

if (installed && plugins.length > 0) {
installed = await runPluginsInstall(plugins, pmc, destinationGitClient);
if (installed) {
const { succeededPlugins } = await configurePlugins(
plugins,
updatePackageScripts,
pmc,
workspaceRoot,
verbose
);
if (succeededPlugins.length > 0) {
await destinationGitClient.amendCommit();
}
}
}

console.log(await destinationGitClient.showStat());

if (installFailed) {
if (installed === false) {
const pmc = getPackageManagerCommand(packageManager);
output.warn({
title: `The import was successful, but the install failed`,
Expand Down Expand Up @@ -397,6 +380,62 @@ async function createTemporaryRemote(
}

/**
* Run install for the imported code and plugins
* @returns true if the install failed
*/
async function runInstallDestinationRepo(
packageManager: PackageManager,
destinationGitClient: GitRepository
): Promise<boolean> {
let installed = true;
try {
output.log({
title: 'Installing dependencies for imported code',
});
runInstall(workspaceRoot, getPackageManagerCommand(packageManager));
await destinationGitClient.amendCommit();
} catch (e) {
installed = false;
output.error({
title: `Install failed: ${e.message || 'Unknown error'}`,
bodyLines: [e.stack],
});
}
return installed;
}

async function runPluginsInstall(
plugins: string[],
pmc: PackageManagerCommands,
destinationGitClient: GitRepository
) {
let installed = true;
output.log({ title: 'Installing Plugins' });
try {
runPackageManagerInstallPlugins(workspaceRoot, pmc, plugins);
await destinationGitClient.amendCommit();
} catch (e) {
installed = false;
output.error({
title: `Install failed: ${e.message || 'Unknown error'}`,
bodyLines: [
'The following plugins were not installed:',
...plugins.map((p) => `- ${chalk.bold(p)}`),
e.stack,
],
});
output.error({
title: `To install the plugins manually`,
bodyLines: [
'You may need to run commands to install the plugins:',
...plugins.map((p) => `- ${chalk.bold(pmc.exec + ' nx add ' + p)}`),
],
});
}
return installed;
}

/*
* 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(
Expand Down
Loading

0 comments on commit 1d7465b

Please sign in to comment.