Skip to content

Commit e02bb82

Browse files
devversionjelbourn
authored andcommitted
fix(schematics): do not depend on external dependency for colors (#17788)
Currently we assume that `chalk` always is installed. This has worked without any issues for a long time because most CLI projects had chalk installed. This could potentially change in the projects, or the verison of chalk could accidentally be older/more recent than what our schematics expect. We just remove colors and depend on the devkit logger for colors based on specific logging levels.
1 parent 3f6a1fd commit e02bb82

File tree

10 files changed

+85
-74
lines changed

10 files changed

+85
-74
lines changed

src/cdk/schematics/BUILD.bazel

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@ ts_library(
3636
"@npm//glob",
3737
"@npm//parse5",
3838
"@npm//typescript",
39-
"@npm//chalk",
4039
],
4140
)
4241

src/cdk/schematics/ng-update/index.ts

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,7 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import {Rule} from '@angular-devkit/schematics';
10-
import chalk from 'chalk';
9+
import {Rule, SchematicContext} from '@angular-devkit/schematics';
1110
import {TargetVersion} from '../update-tool/target-version';
1211
import {cdkUpgradeData} from './upgrade-data';
1312
import {createUpgradeRule} from './upgrade-rules';
@@ -33,14 +32,15 @@ export function updateToV9(): Rule {
3332
}
3433

3534
/** Function that will be called when the migration completed. */
36-
function onMigrationComplete(targetVersion: TargetVersion, hasFailures: boolean) {
37-
console.log();
38-
console.log(chalk.green(` ✓ Updated Angular CDK to ${targetVersion}`));
39-
console.log();
35+
function onMigrationComplete(context: SchematicContext, targetVersion: TargetVersion,
36+
hasFailures: boolean) {
37+
context.logger.info('');
38+
context.logger.info(` ✓ Updated Angular CDK to ${targetVersion}`);
39+
context.logger.info('');
4040

4141
if (hasFailures) {
42-
console.log(chalk.yellow(
42+
context.logger.warn(
4343
' ⚠ Some issues were detected but could not be fixed automatically. Please check the ' +
44-
'output above and fix these issues manually.'));
44+
'output above and fix these issues manually.');
4545
}
4646
}

src/cdk/schematics/ng-update/upgrade-rules/class-inheritance-rule.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import chalk from 'chalk';
109
import * as ts from 'typescript';
1110
import {MigrationRule} from '../../update-tool/migration-rule';
1211
import {PropertyNameUpgradeData} from '../data/property-names';
@@ -54,9 +53,9 @@ export class ClassInheritanceRule extends MigrationRule<RuleUpgradeData> {
5453
if (data) {
5554
this.createFailureAtNode(
5655
node,
57-
`Found class "${chalk.bold(className)}" which extends class ` +
58-
`"${chalk.bold(typeName)}". Please note that the base class property ` +
59-
`"${chalk.red(data.replace)}" has changed to "${chalk.green(data.replaceWith)}". ` +
56+
`Found class "${className}" which extends class ` +
57+
`"${typeName}". Please note that the base class property ` +
58+
`"${data.replace}" has changed to "${data.replaceWith}". ` +
6059
`You may need to update your class as well.`);
6160
}
6261
});

src/cdk/schematics/ng-update/upgrade-rules/index.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,13 +44,16 @@ export const cdkMigrationRules: MigrationRuleType<RuleUpgradeData>[] = [
4444

4545
type NullableMigrationRule = MigrationRuleType<RuleUpgradeData|null>;
4646

47+
type PostMigrationFn = (context: SchematicContext, targetVersion: TargetVersion,
48+
hasFailure: boolean) => void;
49+
4750
/**
4851
* Creates a Angular schematic rule that runs the upgrade for the
4952
* specified target version.
5053
*/
5154
export function createUpgradeRule(
5255
targetVersion: TargetVersion, extraRules: NullableMigrationRule[], upgradeData: RuleUpgradeData,
53-
onMigrationCompleteFn?: (targetVersion: TargetVersion, hasFailures: boolean) => void): Rule {
56+
onMigrationCompleteFn?: PostMigrationFn): Rule {
5457
return async (tree: Tree, context: SchematicContext) => {
5558
const logger = context.logger;
5659
const {buildPaths, testPaths} = getProjectTsConfigPaths(tree);
@@ -99,7 +102,7 @@ export function createUpgradeRule(
99102
}
100103

101104
if (onMigrationCompleteFn) {
102-
onMigrationCompleteFn(targetVersion, hasRuleFailures);
105+
onMigrationCompleteFn(context, targetVersion, hasRuleFailures);
103106
}
104107
};
105108
}

src/material/schematics/BUILD.bazel

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ ts_library(
3434
"@npm//@types/node",
3535
"@npm//tslint",
3636
"@npm//typescript",
37-
"@npm//chalk",
3837
],
3938
)
4039

src/material/schematics/ng-add/index.spec.ts

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,22 @@ import {getWorkspace} from '@schematics/angular/utility/config';
1414
describe('ng-add schematic', () => {
1515
let runner: SchematicTestRunner;
1616
let appTree: Tree;
17+
let errorOutput: string[];
18+
let warnOutput: string[];
1719

1820
beforeEach(async () => {
1921
runner = new SchematicTestRunner('schematics', require.resolve('../collection.json'));
2022
appTree = await createTestApp(runner);
23+
24+
errorOutput = [];
25+
warnOutput = [];
26+
runner.logger.subscribe(e => {
27+
if (e.level === 'error') {
28+
errorOutput.push(e.message);
29+
} else if (e.level === 'warn') {
30+
warnOutput.push(e.message);
31+
}
32+
});
2133
});
2234

2335
/** Expects the given file to be in the styles of the specified workspace project. */
@@ -164,12 +176,10 @@ describe('ng-add schematic', () => {
164176
addModuleImportToRootModule(
165177
appTree, 'NoopAnimationsModule', '@angular/platform-browser/animations', project);
166178

167-
spyOn(console, 'warn');
168179
await runner.runSchematicAsync('ng-add-setup-project', {}, appTree).toPromise();
169180

170-
expect(console.warn)
171-
.toHaveBeenCalledWith(
172-
jasmine.stringMatching(/Could not set up "BrowserAnimationsModule"/));
181+
expect(errorOutput.length).toBe(1);
182+
expect(errorOutput[0]).toMatch(/Could not set up "BrowserAnimationsModule"/);
173183
});
174184
});
175185

@@ -231,12 +241,12 @@ describe('ng-add schematic', () => {
231241
it('should warn if the "test" target has been changed', async () => {
232242
overwriteTargetBuilder(appTree, 'test', 'thirdparty-test-builder');
233243

234-
spyOn(console, 'warn');
235244
await runner.runSchematicAsync('ng-add-setup-project', {}, appTree).toPromise();
236245

237-
expect(console.warn)
238-
.toHaveBeenCalledWith(jasmine.stringMatching(
239-
/not using the default builders.*cannot add the configured theme/));
246+
expect(errorOutput.length).toBe(0);
247+
expect(warnOutput.length).toBe(1);
248+
expect(warnOutput[0]).toMatch(
249+
/not using the default builders.*cannot add the configured theme/);
240250
});
241251
});
242252

@@ -276,7 +286,6 @@ describe('ng-add schematic', () => {
276286
});
277287

278288
it('should not replace existing custom theme files', async () => {
279-
spyOn(console, 'warn');
280289
writeStyleFileToWorkspace(appTree, './projects/material/custom-theme.scss');
281290

282291
const tree = await runner.runSchematicAsync('ng-add-setup-project', {}, appTree).toPromise();
@@ -286,8 +295,8 @@ describe('ng-add schematic', () => {
286295

287296
expect(styles).not.toContain(
288297
defaultPrebuiltThemePath, 'Expected the default prebuilt theme to be not configured.');
289-
expect(console.warn)
290-
.toHaveBeenCalledWith(jasmine.stringMatching(/Could not add the selected theme/));
298+
expect(errorOutput.length).toBe(1);
299+
expect(errorOutput[0]).toMatch(/Could not add the selected theme/);
291300
});
292301

293302
it('should not add a theme file multiple times', async () => {

src/material/schematics/ng-add/setup-project.ts

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,14 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import {chain, Rule, Tree} from '@angular-devkit/schematics';
9+
import {chain, Rule, SchematicContext, Tree} from '@angular-devkit/schematics';
1010
import {
1111
addModuleImportToRootModule,
1212
getProjectFromWorkspace,
1313
getProjectMainFile,
1414
getProjectStyleFile,
1515
hasNgModuleImport,
1616
} from '@angular/cdk/schematics';
17-
import chalk from 'chalk';
1817
import {getWorkspace} from '@schematics/angular/utility/config';
1918
import {getAppModulePath} from '@schematics/angular/utility/ng-ast-utils';
2019
import {addFontsToIndex} from './fonts/material-fonts';
@@ -49,7 +48,7 @@ export default function(options: Schema): Rule {
4948
* components of Angular Material will throw an exception.
5049
*/
5150
function addAnimationsModule(options: Schema) {
52-
return (host: Tree) => {
51+
return (host: Tree, context: SchematicContext) => {
5352
const workspace = getWorkspace(host);
5453
const project = getProjectFromWorkspace(workspace, options.project);
5554
const appModulePath = getAppModulePath(host, getProjectMainFile(project));
@@ -60,10 +59,11 @@ function addAnimationsModule(options: Schema) {
6059
// animations. If we would add the BrowserAnimationsModule while the NoopAnimationsModule
6160
// is already configured, we would cause unexpected behavior and runtime exceptions.
6261
if (hasNgModuleImport(host, appModulePath, noopAnimationsModuleName)) {
63-
return console.warn(chalk.red(
64-
`Could not set up "${chalk.bold(browserAnimationsModuleName)}" ` +
65-
`because "${chalk.bold(noopAnimationsModuleName)}" is already imported. Please ` +
66-
`manually set up browser animations.`));
62+
context.logger.error(
63+
`Could not set up "${browserAnimationsModuleName}" ` +
64+
`because "${noopAnimationsModuleName}" is already imported.`);
65+
context.logger.info(`Please manually set up browser animations.`);
66+
return;
6767
}
6868

6969
addModuleImportToRootModule(host, browserAnimationsModuleName,
@@ -84,23 +84,24 @@ function addAnimationsModule(options: Schema) {
8484
* and reset the default browser body margin.
8585
*/
8686
function addMaterialAppStyles(options: Schema) {
87-
return (host: Tree) => {
87+
return (host: Tree, context: SchematicContext) => {
8888
const workspace = getWorkspace(host);
8989
const project = getProjectFromWorkspace(workspace, options.project);
9090
const styleFilePath = getProjectStyleFile(project);
91+
const logger = context.logger;
9192

9293
if (!styleFilePath) {
93-
console.warn(chalk.red(`Could not find the default style file for this project.`));
94-
console.warn(chalk.red(`Please consider manually setting up the Roboto font in your CSS.`));
94+
logger.error(`Could not find the default style file for this project.`);
95+
logger.info(`Please consider manually setting up the Roboto font in your CSS.`);
9596
return;
9697
}
9798

9899
const buffer = host.read(styleFilePath);
99100

100101
if (!buffer) {
101-
console.warn(chalk.red(`Could not read the default style file within the project ` +
102-
`(${chalk.italic(styleFilePath)})`));
103-
console.warn(chalk.red(`Please consider manually setting up the Robot font.`));
102+
logger.error(`Could not read the default style file within the project ` +
103+
`(${styleFilePath})`);
104+
logger.info(`Please consider manually setting up the Robot font.`);
104105
return;
105106
}
106107

src/material/schematics/ng-add/theming/theming.ts

Lines changed: 25 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,9 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import {normalize} from '@angular-devkit/core';
9+
import {normalize, logging} from '@angular-devkit/core';
1010
import {WorkspaceProject, WorkspaceSchema} from '@angular-devkit/core/src/experimental/workspace';
11-
import {SchematicsException, Tree} from '@angular-devkit/schematics';
11+
import {Rule, SchematicContext, SchematicsException, Tree} from '@angular-devkit/schematics';
1212
import {
1313
addBodyClass,
1414
defaultTargetBuilders,
@@ -19,7 +19,6 @@ import {
1919
} from '@angular/cdk/schematics';
2020
import {InsertChange} from '@schematics/angular/utility/change';
2121
import {getWorkspace} from '@schematics/angular/utility/config';
22-
import chalk from 'chalk';
2322
import {join} from 'path';
2423
import {Schema} from '../schema';
2524
import {createCustomTheme} from './create-custom-theme';
@@ -31,16 +30,16 @@ const prebuiltThemePathSegment = '@angular/material/prebuilt-themes';
3130
const defaultCustomThemeFilename = 'custom-theme.scss';
3231

3332
/** Add pre-built styles to the main project style file. */
34-
export function addThemeToAppStyles(options: Schema): (host: Tree) => Tree {
35-
return function(host: Tree): Tree {
33+
export function addThemeToAppStyles(options: Schema): Rule {
34+
return function(host: Tree, context: SchematicContext): Tree {
3635
const workspace = getWorkspace(host);
3736
const project = getProjectFromWorkspace(workspace, options.project);
3837
const themeName = options.theme || 'indigo-pink';
3938

4039
if (themeName === 'custom') {
41-
insertCustomTheme(project, options.project, host, workspace);
40+
insertCustomTheme(project, options.project, host, workspace, context.logger);
4241
} else {
43-
insertPrebuiltTheme(project, host, themeName, workspace);
42+
insertPrebuiltTheme(project, host, themeName, workspace, context.logger);
4443
}
4544

4645
return host;
@@ -69,7 +68,7 @@ export function addTypographyClass(options: Schema): (host: Tree) => Tree {
6968
* Scss file for the custom theme will be created.
7069
*/
7170
function insertCustomTheme(project: WorkspaceProject, projectName: string, host: Tree,
72-
workspace: WorkspaceSchema) {
71+
workspace: WorkspaceSchema, logger: logging.LoggerApi) {
7372

7473
const stylesPath = getProjectStyleFile(project, 'scss');
7574
const themeContent = createCustomTheme(projectName);
@@ -85,13 +84,13 @@ function insertCustomTheme(project: WorkspaceProject, projectName: string, host:
8584
const customThemePath = normalize(join(project.sourceRoot, defaultCustomThemeFilename));
8685

8786
if (host.exists(customThemePath)) {
88-
console.warn(chalk.yellow(`Cannot create a custom Angular Material theme because
89-
${chalk.bold(customThemePath)} already exists. Skipping custom theme generation.`));
87+
logger.warn(`Cannot create a custom Angular Material theme because
88+
${customThemePath} already exists. Skipping custom theme generation.`);
9089
return;
9190
}
9291

9392
host.create(customThemePath, themeContent);
94-
addThemeStyleToTarget(project, 'build', host, customThemePath, workspace);
93+
addThemeStyleToTarget(project, 'build', host, customThemePath, workspace, logger);
9594
return;
9695
}
9796

@@ -104,20 +103,21 @@ function insertCustomTheme(project: WorkspaceProject, projectName: string, host:
104103

105104
/** Insert a pre-built theme into the angular.json file. */
106105
function insertPrebuiltTheme(project: WorkspaceProject, host: Tree, theme: string,
107-
workspace: WorkspaceSchema) {
106+
workspace: WorkspaceSchema, logger: logging.LoggerApi) {
108107

109108
// Path needs to be always relative to the `package.json` or workspace root.
110109
const themePath = `./node_modules/@angular/material/prebuilt-themes/${theme}.css`;
111110

112-
addThemeStyleToTarget(project, 'build', host, themePath, workspace);
113-
addThemeStyleToTarget(project, 'test', host, themePath, workspace);
111+
addThemeStyleToTarget(project, 'build', host, themePath, workspace, logger);
112+
addThemeStyleToTarget(project, 'test', host, themePath, workspace, logger);
114113
}
115114

116115
/** Adds a theming style entry to the given project target options. */
117116
function addThemeStyleToTarget(project: WorkspaceProject, targetName: 'test' | 'build', host: Tree,
118-
assetPath: string, workspace: WorkspaceSchema) {
117+
assetPath: string, workspace: WorkspaceSchema,
118+
logger: logging.LoggerApi) {
119119
// Do not update the builder options in case the target does not use the default CLI builder.
120-
if (!validateDefaultTargetBuilder(project, targetName)) {
120+
if (!validateDefaultTargetBuilder(project, targetName, logger)) {
121121
return;
122122
}
123123

@@ -139,11 +139,10 @@ function addThemeStyleToTarget(project: WorkspaceProject, targetName: 'test' | '
139139
// theme because these files can contain custom styles, while prebuilt themes are
140140
// always packaged and considered replaceable.
141141
if (stylePath.includes(defaultCustomThemeFilename)) {
142-
console.warn(chalk.red(`Could not add the selected theme to the CLI project ` +
143-
`configuration because there is already a custom theme file referenced.`));
144-
console.warn(chalk.red(
145-
`Please manually add the following style file to your configuration:`));
146-
console.warn(chalk.yellow(` ${chalk.bold(assetPath)}`));
142+
logger.error(`Could not add the selected theme to the CLI project ` +
143+
`configuration because there is already a custom theme file referenced.`);
144+
logger.info(`Please manually add the following style file to your configuration:`);
145+
logger.info(` ${assetPath}`);
147146
return;
148147
} else if (stylePath.includes(prebuiltThemePathSegment)) {
149148
targetOptions.styles.splice(index, 1);
@@ -161,7 +160,8 @@ function addThemeStyleToTarget(project: WorkspaceProject, targetName: 'test' | '
161160
* provided by the Angular CLI. If the configured builder does not match the default builder,
162161
* this function can either throw or just show a warning.
163162
*/
164-
function validateDefaultTargetBuilder(project: WorkspaceProject, targetName: 'build' | 'test') {
163+
function validateDefaultTargetBuilder(project: WorkspaceProject, targetName: 'build' | 'test',
164+
logger: logging.LoggerApi) {
165165
const defaultBuilder = defaultTargetBuilders[targetName];
166166
const targetConfig = project.architect && project.architect[targetName] ||
167167
project.targets && project.targets[targetName];
@@ -178,7 +178,9 @@ function validateDefaultTargetBuilder(project: WorkspaceProject, targetName: 'bu
178178
`"${targetName}". The Angular Material schematics cannot add a theme to the workspace ` +
179179
`configuration if the builder has been changed.`);
180180
} else if (!isDefaultBuilder) {
181-
console.warn(`Your project is not using the default builders for "${targetName}". This ` +
181+
// for non-build targets we gracefully report the error without actually aborting the
182+
// setup schematic. This is because a theme is not mandatory for running tests.
183+
logger.warn(`Your project is not using the default builders for "${targetName}". This ` +
182184
`means that we cannot add the configured theme to the "${targetName}" target.`);
183185
}
184186

0 commit comments

Comments
 (0)