Skip to content

fix(schematics): do not depend on external dependency for colors #17788

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion src/cdk/schematics/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ ts_library(
"@npm//glob",
"@npm//parse5",
"@npm//typescript",
"@npm//chalk",
],
)

Expand Down
16 changes: 8 additions & 8 deletions src/cdk/schematics/ng-update/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

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

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

if (hasFailures) {
console.log(chalk.yellow(
context.logger.warn(
' ⚠ Some issues were detected but could not be fixed automatically. Please check the ' +
'output above and fix these issues manually.'));
'output above and fix these issues manually.');
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
* found in the LICENSE file at https://angular.io/license
*/

import chalk from 'chalk';
import * as ts from 'typescript';
import {MigrationRule} from '../../update-tool/migration-rule';
import {PropertyNameUpgradeData} from '../data/property-names';
Expand Down Expand Up @@ -54,9 +53,9 @@ export class ClassInheritanceRule extends MigrationRule<RuleUpgradeData> {
if (data) {
this.createFailureAtNode(
node,
`Found class "${chalk.bold(className)}" which extends class ` +
`"${chalk.bold(typeName)}". Please note that the base class property ` +
`"${chalk.red(data.replace)}" has changed to "${chalk.green(data.replaceWith)}". ` +
`Found class "${className}" which extends class ` +
`"${typeName}". Please note that the base class property ` +
`"${data.replace}" has changed to "${data.replaceWith}". ` +
`You may need to update your class as well.`);
}
});
Expand Down
7 changes: 5 additions & 2 deletions src/cdk/schematics/ng-update/upgrade-rules/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,16 @@ export const cdkMigrationRules: MigrationRuleType<RuleUpgradeData>[] = [

type NullableMigrationRule = MigrationRuleType<RuleUpgradeData|null>;

type PostMigrationFn = (context: SchematicContext, targetVersion: TargetVersion,
hasFailure: boolean) => void;

/**
* Creates a Angular schematic rule that runs the upgrade for the
* specified target version.
*/
export function createUpgradeRule(
targetVersion: TargetVersion, extraRules: NullableMigrationRule[], upgradeData: RuleUpgradeData,
onMigrationCompleteFn?: (targetVersion: TargetVersion, hasFailures: boolean) => void): Rule {
onMigrationCompleteFn?: PostMigrationFn): Rule {
return async (tree: Tree, context: SchematicContext) => {
const logger = context.logger;
const {buildPaths, testPaths} = getProjectTsConfigPaths(tree);
Expand Down Expand Up @@ -99,7 +102,7 @@ export function createUpgradeRule(
}

if (onMigrationCompleteFn) {
onMigrationCompleteFn(targetVersion, hasRuleFailures);
onMigrationCompleteFn(context, targetVersion, hasRuleFailures);
}
};
}
1 change: 0 additions & 1 deletion src/material/schematics/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ ts_library(
"@npm//@types/node",
"@npm//tslint",
"@npm//typescript",
"@npm//chalk",
],
)

Expand Down
31 changes: 20 additions & 11 deletions src/material/schematics/ng-add/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,22 @@ import {getWorkspace} from '@schematics/angular/utility/config';
describe('ng-add schematic', () => {
let runner: SchematicTestRunner;
let appTree: Tree;
let errorOutput: string[];
let warnOutput: string[];

beforeEach(async () => {
runner = new SchematicTestRunner('schematics', require.resolve('../collection.json'));
appTree = await createTestApp(runner);

errorOutput = [];
warnOutput = [];
runner.logger.subscribe(e => {
if (e.level === 'error') {
errorOutput.push(e.message);
} else if (e.level === 'warn') {
warnOutput.push(e.message);
}
});
});

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

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

expect(console.warn)
.toHaveBeenCalledWith(
jasmine.stringMatching(/Could not set up "BrowserAnimationsModule"/));
expect(errorOutput.length).toBe(1);
expect(errorOutput[0]).toMatch(/Could not set up "BrowserAnimationsModule"/);
});
});

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

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

expect(console.warn)
.toHaveBeenCalledWith(jasmine.stringMatching(
/not using the default builders.*cannot add the configured theme/));
expect(errorOutput.length).toBe(0);
expect(warnOutput.length).toBe(1);
expect(warnOutput[0]).toMatch(
/not using the default builders.*cannot add the configured theme/);
});
});

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

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

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

expect(styles).not.toContain(
defaultPrebuiltThemePath, 'Expected the default prebuilt theme to be not configured.');
expect(console.warn)
.toHaveBeenCalledWith(jasmine.stringMatching(/Could not add the selected theme/));
expect(errorOutput.length).toBe(1);
expect(errorOutput[0]).toMatch(/Could not add the selected theme/);
});

it('should not add a theme file multiple times', async () => {
Expand Down
27 changes: 14 additions & 13 deletions src/material/schematics/ng-add/setup-project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,14 @@
* found in the LICENSE file at https://angular.io/license
*/

import {chain, Rule, Tree} from '@angular-devkit/schematics';
import {chain, Rule, SchematicContext, Tree} from '@angular-devkit/schematics';
import {
addModuleImportToRootModule,
getProjectFromWorkspace,
getProjectMainFile,
getProjectStyleFile,
hasNgModuleImport,
} from '@angular/cdk/schematics';
import chalk from 'chalk';
import {getWorkspace} from '@schematics/angular/utility/config';
import {getAppModulePath} from '@schematics/angular/utility/ng-ast-utils';
import {addFontsToIndex} from './fonts/material-fonts';
Expand Down Expand Up @@ -49,7 +48,7 @@ export default function(options: Schema): Rule {
* components of Angular Material will throw an exception.
*/
function addAnimationsModule(options: Schema) {
return (host: Tree) => {
return (host: Tree, context: SchematicContext) => {
const workspace = getWorkspace(host);
const project = getProjectFromWorkspace(workspace, options.project);
const appModulePath = getAppModulePath(host, getProjectMainFile(project));
Expand All @@ -60,10 +59,11 @@ function addAnimationsModule(options: Schema) {
// animations. If we would add the BrowserAnimationsModule while the NoopAnimationsModule
// is already configured, we would cause unexpected behavior and runtime exceptions.
if (hasNgModuleImport(host, appModulePath, noopAnimationsModuleName)) {
return console.warn(chalk.red(
`Could not set up "${chalk.bold(browserAnimationsModuleName)}" ` +
`because "${chalk.bold(noopAnimationsModuleName)}" is already imported. Please ` +
`manually set up browser animations.`));
context.logger.error(
`Could not set up "${browserAnimationsModuleName}" ` +
`because "${noopAnimationsModuleName}" is already imported.`);
context.logger.info(`Please manually set up browser animations.`);
return;
}

addModuleImportToRootModule(host, browserAnimationsModuleName,
Expand All @@ -84,23 +84,24 @@ function addAnimationsModule(options: Schema) {
* and reset the default browser body margin.
*/
function addMaterialAppStyles(options: Schema) {
return (host: Tree) => {
return (host: Tree, context: SchematicContext) => {
const workspace = getWorkspace(host);
const project = getProjectFromWorkspace(workspace, options.project);
const styleFilePath = getProjectStyleFile(project);
const logger = context.logger;

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

const buffer = host.read(styleFilePath);

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

Expand Down
48 changes: 25 additions & 23 deletions src/material/schematics/ng-add/theming/theming.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@
* found in the LICENSE file at https://angular.io/license
*/

import {normalize} from '@angular-devkit/core';
import {normalize, logging} from '@angular-devkit/core';
import {WorkspaceProject, WorkspaceSchema} from '@angular-devkit/core/src/experimental/workspace';
import {SchematicsException, Tree} from '@angular-devkit/schematics';
import {Rule, SchematicContext, SchematicsException, Tree} from '@angular-devkit/schematics';
import {
addBodyClass,
defaultTargetBuilders,
Expand All @@ -19,7 +19,6 @@ import {
} from '@angular/cdk/schematics';
import {InsertChange} from '@schematics/angular/utility/change';
import {getWorkspace} from '@schematics/angular/utility/config';
import chalk from 'chalk';
import {join} from 'path';
import {Schema} from '../schema';
import {createCustomTheme} from './create-custom-theme';
Expand All @@ -31,16 +30,16 @@ const prebuiltThemePathSegment = '@angular/material/prebuilt-themes';
const defaultCustomThemeFilename = 'custom-theme.scss';

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

if (themeName === 'custom') {
insertCustomTheme(project, options.project, host, workspace);
insertCustomTheme(project, options.project, host, workspace, context.logger);
} else {
insertPrebuiltTheme(project, host, themeName, workspace);
insertPrebuiltTheme(project, host, themeName, workspace, context.logger);
}

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

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

if (host.exists(customThemePath)) {
console.warn(chalk.yellow(`Cannot create a custom Angular Material theme because
${chalk.bold(customThemePath)} already exists. Skipping custom theme generation.`));
logger.warn(`Cannot create a custom Angular Material theme because
${customThemePath} already exists. Skipping custom theme generation.`);
return;
}

host.create(customThemePath, themeContent);
addThemeStyleToTarget(project, 'build', host, customThemePath, workspace);
addThemeStyleToTarget(project, 'build', host, customThemePath, workspace, logger);
return;
}

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

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

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

addThemeStyleToTarget(project, 'build', host, themePath, workspace);
addThemeStyleToTarget(project, 'test', host, themePath, workspace);
addThemeStyleToTarget(project, 'build', host, themePath, workspace, logger);
addThemeStyleToTarget(project, 'test', host, themePath, workspace, logger);
}

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

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

Expand Down
Loading