Skip to content

fix(ng-add): do not throw if custom builder is used for "test" #14203

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
30 changes: 30 additions & 0 deletions src/lib/schematics/ng-add/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,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
Original file line number Diff line number Diff line change
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;
}