Skip to content

Commit

Permalink
feat(@angular/cli): provide more detailed error for not found builder
Browse files Browse the repository at this point in the history
When a builder-based command is executed (build, serve, test, etc.) and the builder's node package cannot be found a more user-friendly error message is now displayed. In addition, when the builder's node package cannot be found, a check is performed to determine if the node packages for the workspace may have not been installed. Previously, a potentially long stacktrace was shown which did not provide much information regarding how to correct the issue.

Closes: #10536
  • Loading branch information
clydin authored and filipesilva committed Nov 17, 2021
1 parent 1e81b8d commit ecd9fb5
Show file tree
Hide file tree
Showing 2 changed files with 107 additions and 3 deletions.
77 changes: 74 additions & 3 deletions packages/angular/cli/models/architect-command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@
import { Architect, Target } from '@angular-devkit/architect';
import { WorkspaceNodeModulesArchitectHost } from '@angular-devkit/architect/node';
import { json, schema, tags } from '@angular-devkit/core';
import { existsSync } from 'fs';
import * as path from 'path';
import { parseJsonSchemaToOptions } from '../utilities/json-schema';
import { getPackageManager } from '../utilities/package-manager';
import { isPackageNameSafeForAnalytics } from './analytics';
import { BaseCommandOptions, Command } from './command';
import { Arguments, Option } from './interface';
Expand Down Expand Up @@ -115,7 +118,19 @@ export abstract class ArchitectCommand<
builderNames.add(builderName);
}

const builderDesc = await this._architectHost.resolveBuilder(builderName);
let builderDesc;
try {
builderDesc = await this._architectHost.resolveBuilder(builderName);
} catch (e) {
if (e.code === 'MODULE_NOT_FOUND') {
await this.warnOnMissingNodeModules(this.workspace.basePath);
this.logger.fatal(`Could not find the '${builderName}' builder's node package.`);

return 1;
}
throw e;
}

const optionDefs = await parseJsonSchemaToOptions(
this._registry,
builderDesc.optionSchema as json.JsonObject,
Expand Down Expand Up @@ -193,7 +208,19 @@ export abstract class ArchitectCommand<
project: projectName || (targetProjectNames.length > 0 ? targetProjectNames[0] : ''),
target: this.target,
});
const builderDesc = await this._architectHost.resolveBuilder(builderConf);

let builderDesc;
try {
builderDesc = await this._architectHost.resolveBuilder(builderConf);
} catch (e) {
if (e.code === 'MODULE_NOT_FOUND') {
await this.warnOnMissingNodeModules(this.workspace.basePath);
this.logger.fatal(`Could not find the '${builderConf}' builder's node package.`);

return 1;
}
throw e;
}

this.description.options.push(
...(await parseJsonSchemaToOptions(
Expand All @@ -210,6 +237,38 @@ export abstract class ArchitectCommand<
}
}

private async warnOnMissingNodeModules(basePath: string): Promise<void> {
// Check for a `node_modules` directory (npm, yarn non-PnP, etc.)
if (existsSync(path.resolve(basePath, 'node_modules'))) {
return;
}

// Check for yarn PnP files
if (
existsSync(path.resolve(basePath, '.pnp.js')) ||
existsSync(path.resolve(basePath, '.pnp.cjs')) ||
existsSync(path.resolve(basePath, '.pnp.mjs'))
) {
return;
}

const packageManager = await getPackageManager(basePath);
let installSuggestion = 'Try installing with ';
switch (packageManager) {
case 'npm':
installSuggestion += `'npm install'`;
break;
case 'yarn':
installSuggestion += `'yarn'`;
break;
default:
installSuggestion += `the project's package manager`;
break;
}

this.logger.warn(`Node packages may not be installed. ${installSuggestion}.`);
}

async run(options: ArchitectCommandOptions & Arguments) {
return await this.runArchitectTarget(options);
}
Expand All @@ -219,7 +278,19 @@ export abstract class ArchitectCommand<
// overrides separately (getting the configuration builds the whole project, including
// overrides).
const builderConf = await this._architectHost.getBuilderNameForTarget(target);
const builderDesc = await this._architectHost.resolveBuilder(builderConf);
let builderDesc;
try {
builderDesc = await this._architectHost.resolveBuilder(builderConf);
} catch (e) {
if (e.code === 'MODULE_NOT_FOUND') {
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
await this.warnOnMissingNodeModules(this.workspace!.basePath);
this.logger.fatal(`Could not find the '${builderConf}' builder's node package.`);

return 1;
}
throw e;
}
const targetOptionArray = await parseJsonSchemaToOptions(
this._registry,
builderDesc.optionSchema as json.JsonObject,
Expand Down
33 changes: 33 additions & 0 deletions tests/legacy-cli/e2e/tests/commands/builder-not-found.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import { moveFile } from '../../utils/fs';
import { installPackage, uninstallPackage } from '../../utils/packages';
import { execAndWaitForOutputToMatch, ng } from '../../utils/process';
import { expectToFail } from '../../utils/utils';

export default async function () {
try {
await uninstallPackage('@angular-devkit/build-angular');

await expectToFail(() => ng('build'));
await execAndWaitForOutputToMatch(
'ng',
['build'],
/Could not find the '@angular-devkit\/build-angular:browser' builder's node package\./,
);
await expectToFail(() =>
execAndWaitForOutputToMatch('ng', ['build'], /Node packages may not be installed\./),
);

await moveFile('node_modules', 'temp_node_modules');

await expectToFail(() => ng('build'));
await execAndWaitForOutputToMatch(
'ng',
['build'],
/Could not find the '@angular-devkit\/build-angular:browser' builder's node package\./,
);
await execAndWaitForOutputToMatch('ng', ['build'], /Node packages may not be installed\./);
} finally {
await moveFile('temp_node_modules', 'node_modules');
await installPackage('@angular-devkit/build-angular');
}
}

0 comments on commit ecd9fb5

Please sign in to comment.