Skip to content

Commit

Permalink
fix(ng-add): do not throw if custom builder is used for "test" (#14203)
Browse files Browse the repository at this point in the history
* No longer throws an exception if someone runs `ng-add` but uses a custom builder for the `test` target. Since theme files are not necessarily mandatory for running tests, we should just show a warning instead of exiting the whole `ng-add` process.

Fixes #14176
  • Loading branch information
devversion authored and jelbourn committed Dec 3, 2018
1 parent 4f7a2e6 commit 498a3d8
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 25 deletions.
30 changes: 30 additions & 0 deletions src/lib/schematics/ng-add/index.spec.ts
Expand Up @@ -207,6 +207,36 @@ describe('ng-add schematic', () => {
});
});

describe('custom project builders', () => {

/** Overwrites a target builder for the workspace in the given tree */
function overwriteTargetBuilder(tree: Tree, targetName: string, newBuilder: string) {
const workspace = getWorkspace(tree);
const project = getProjectFromWorkspace(workspace);
const targetConfig = project.architect && project.architect[targetName] ||
project.targets && project.targets[targetName];
targetConfig['builder'] = newBuilder;
tree.overwrite('/angular.json', JSON.stringify(workspace, null, 2));
}

it('should throw an error if the "build" target has been changed', () => {
overwriteTargetBuilder(appTree, 'build', 'thirdparty-builder');

expect(() => runner.runSchematic('ng-add-setup-project', {}, appTree))
.toThrowError(/not using the default builders.*build/);
});

it('should warn if the "test" target has been changed', () => {
overwriteTargetBuilder(appTree, 'test', 'thirdparty-test-builder');

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

expect(console.warn).toHaveBeenCalledWith(
jasmine.stringMatching(/not using the default builders.*cannot add the configured theme/));
});
});

describe('theme files', () => {

/** Path to the default prebuilt theme file that will be added when running ng-add. */
Expand Down
62 changes: 37 additions & 25 deletions src/lib/schematics/ng-add/theming/theming.ts
Expand Up @@ -8,36 +8,38 @@

import {normalize} from '@angular-devkit/core';
import {WorkspaceProject, WorkspaceSchema} from '@angular-devkit/core/src/workspace';
import {SchematicsException, Tree} from '@angular-devkit/schematics';
import {Tree} from '@angular-devkit/schematics';
import {
getProjectFromWorkspace,
getProjectStyleFile,
getProjectTargetOptions,
} from '@angular/cdk/schematics';
import {InsertChange} from '@schematics/angular/utility/change';
import {getWorkspace} from '@schematics/angular/utility/config';
import {bold, red, yellow} from 'chalk';
import {join} from 'path';
import {Schema} from '../schema';
import {createCustomTheme} from './custom-theme';
import {red, bold, yellow} from 'chalk';
import {createCustomTheme} from './create-custom-theme';

/** Path segment that can be found in paths that refer to a prebuilt theme. */
const prebuiltThemePathSegment = '@angular/material/prebuilt-themes';

/** Default file name of the custom theme that can be generated. */
const defaultCustomThemeFilename = 'custom-theme.scss';

/** Object that maps a CLI target to its default builder name. */
const defaultTargetBuilders = {
build: '@angular-devkit/build-angular:browser',
test: '@angular-devkit/build-angular:karma',
};

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

// Because the build setup for the Angular CLI can be changed so dramatically, we can't know
// where to generate anything if the project is not using the default config for build and test.
assertDefaultBuildersConfigured(project);

if (themeName === 'custom') {
insertCustomTheme(project, options.project, host, workspace);
} else {
Expand Down Expand Up @@ -98,8 +100,12 @@ function insertPrebuiltTheme(project: WorkspaceProject, host: Tree, theme: strin
}

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

const targetOptions = getProjectTargetOptions(project, targetName);

Expand Down Expand Up @@ -135,25 +141,31 @@ function addThemeStyleToTarget(project: WorkspaceProject, targetName: string, ho
host.overwrite('angular.json', JSON.stringify(workspace, null, 2));
}

/** Throws if the project is not using the default Angular devkit builders. */
function assertDefaultBuildersConfigured(project: WorkspaceProject) {
checkProjectTargetBuilder(project, 'build', '@angular-devkit/build-angular:browser');
checkProjectTargetBuilder(project, 'test', '@angular-devkit/build-angular:karma');
}

/**
* Checks if the specified project target is configured with the default builders which are
* provided by the Angular CLI.
* Validates that the specified project target is configured with the default builders which are
* 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 checkProjectTargetBuilder(project: WorkspaceProject, targetName: string,
defaultBuilder: string) {

function validateDefaultTargetBuilder(project: WorkspaceProject, targetName: 'build' | 'test') {
const defaultBuilder = defaultTargetBuilders[targetName];
const targetConfig = project.architect && project.architect[targetName] ||
project.targets && project.targets[targetName];

if (!targetConfig || targetConfig['builder'] !== defaultBuilder) {
throw new SchematicsException(
`Your project is not using the default builders for "${targetName}". The Angular Material ` +
'schematics can only be used if the original builders from the Angular CLI are configured.');
const isDefaultBuilder = targetConfig && targetConfig['builder'] === defaultBuilder;

// Because the build setup for the Angular CLI can be customized by developers, we can't know
// where to put the theme file in the workspace configuration if custom builders are being
// used. In case the builder has been changed for the "build" target, we throw an error and
// exit because setting up a theme is a primary goal of `ng-add`. Otherwise if just the "test"
// builder has been changed, we warn because a theme is not mandatory for running tests
// with Material. See: https://github.com/angular/material2/issues/14176
if (!isDefaultBuilder && targetName === 'build') {
throw new Error(`Your project is not using the default builders for "${targetName}". The ` +
`Angular Material schematics cannot add a theme to the workspace configuration if the ` +
`builder has been changed. Exiting..`);
} else if (!isDefaultBuilder) {
console.warn(`Your project is not using the default builders for "${targetName}". This ` +
`means that we cannot add the configured theme to the "${targetName}" target.`);
}

return isDefaultBuilder;
}

0 comments on commit 498a3d8

Please sign in to comment.