Skip to content

Commit ee45aab

Browse files
devversionjelbourn
authored andcommitted
fix(ng-add): do not throw if custom builder is used for "test" (#14203)
* 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
1 parent 57fa085 commit ee45aab

File tree

3 files changed

+67
-25
lines changed

3 files changed

+67
-25
lines changed

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

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,36 @@ describe('ng-add schematic', () => {
207207
});
208208
});
209209

210+
describe('custom project builders', () => {
211+
212+
/** Overwrites a target builder for the workspace in the given tree */
213+
function overwriteTargetBuilder(tree: Tree, targetName: string, newBuilder: string) {
214+
const workspace = getWorkspace(tree);
215+
const project = getProjectFromWorkspace(workspace);
216+
const targetConfig = project.architect && project.architect[targetName] ||
217+
project.targets && project.targets[targetName];
218+
targetConfig['builder'] = newBuilder;
219+
tree.overwrite('/angular.json', JSON.stringify(workspace, null, 2));
220+
}
221+
222+
it('should throw an error if the "build" target has been changed', () => {
223+
overwriteTargetBuilder(appTree, 'build', 'thirdparty-builder');
224+
225+
expect(() => runner.runSchematic('ng-add-setup-project', {}, appTree))
226+
.toThrowError(/not using the default builders.*build/);
227+
});
228+
229+
it('should warn if the "test" target has been changed', () => {
230+
overwriteTargetBuilder(appTree, 'test', 'thirdparty-test-builder');
231+
232+
spyOn(console, 'warn');
233+
runner.runSchematic('ng-add-setup-project', {}, appTree);
234+
235+
expect(console.warn).toHaveBeenCalledWith(
236+
jasmine.stringMatching(/not using the default builders.*cannot add the configured theme/));
237+
});
238+
});
239+
210240
describe('theme files', () => {
211241

212242
/** Path to the default prebuilt theme file that will be added when running ng-add. */

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

Lines changed: 37 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -8,36 +8,38 @@
88

99
import {normalize} from '@angular-devkit/core';
1010
import {WorkspaceProject, WorkspaceSchema} from '@angular-devkit/core/src/workspace';
11-
import {SchematicsException, Tree} from '@angular-devkit/schematics';
11+
import {Tree} from '@angular-devkit/schematics';
1212
import {
1313
getProjectFromWorkspace,
1414
getProjectStyleFile,
1515
getProjectTargetOptions,
1616
} from '@angular/cdk/schematics';
1717
import {InsertChange} from '@schematics/angular/utility/change';
1818
import {getWorkspace} from '@schematics/angular/utility/config';
19+
import {bold, red, yellow} from 'chalk';
1920
import {join} from 'path';
2021
import {Schema} from '../schema';
21-
import {createCustomTheme} from './custom-theme';
22-
import {red, bold, yellow} from 'chalk';
22+
import {createCustomTheme} from './create-custom-theme';
2323

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

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

30+
/** Object that maps a CLI target to its default builder name. */
31+
const defaultTargetBuilders = {
32+
build: '@angular-devkit/build-angular:browser',
33+
test: '@angular-devkit/build-angular:karma',
34+
};
35+
3036
/** Add pre-built styles to the main project style file. */
3137
export function addThemeToAppStyles(options: Schema): (host: Tree) => Tree {
3238
return function(host: Tree): Tree {
3339
const workspace = getWorkspace(host);
3440
const project = getProjectFromWorkspace(workspace, options.project);
3541
const themeName = options.theme || 'indigo-pink';
3642

37-
// Because the build setup for the Angular CLI can be changed so dramatically, we can't know
38-
// where to generate anything if the project is not using the default config for build and test.
39-
assertDefaultBuildersConfigured(project);
40-
4143
if (themeName === 'custom') {
4244
insertCustomTheme(project, options.project, host, workspace);
4345
} else {
@@ -98,8 +100,12 @@ function insertPrebuiltTheme(project: WorkspaceProject, host: Tree, theme: strin
98100
}
99101

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

104110
const targetOptions = getProjectTargetOptions(project, targetName);
105111

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

138-
/** Throws if the project is not using the default Angular devkit builders. */
139-
function assertDefaultBuildersConfigured(project: WorkspaceProject) {
140-
checkProjectTargetBuilder(project, 'build', '@angular-devkit/build-angular:browser');
141-
checkProjectTargetBuilder(project, 'test', '@angular-devkit/build-angular:karma');
142-
}
143-
144144
/**
145-
* Checks if the specified project target is configured with the default builders which are
146-
* provided by the Angular CLI.
145+
* Validates that the specified project target is configured with the default builders which are
146+
* provided by the Angular CLI. If the configured builder does not match the default builder,
147+
* this function can either throw or just show a warning.
147148
*/
148-
function checkProjectTargetBuilder(project: WorkspaceProject, targetName: string,
149-
defaultBuilder: string) {
150-
149+
function validateDefaultTargetBuilder(project: WorkspaceProject, targetName: 'build' | 'test') {
150+
const defaultBuilder = defaultTargetBuilders[targetName];
151151
const targetConfig = project.architect && project.architect[targetName] ||
152152
project.targets && project.targets[targetName];
153-
154-
if (!targetConfig || targetConfig['builder'] !== defaultBuilder) {
155-
throw new SchematicsException(
156-
`Your project is not using the default builders for "${targetName}". The Angular Material ` +
157-
'schematics can only be used if the original builders from the Angular CLI are configured.');
153+
const isDefaultBuilder = targetConfig && targetConfig['builder'] === defaultBuilder;
154+
155+
// Because the build setup for the Angular CLI can be customized by developers, we can't know
156+
// where to put the theme file in the workspace configuration if custom builders are being
157+
// used. In case the builder has been changed for the "build" target, we throw an error and
158+
// exit because setting up a theme is a primary goal of `ng-add`. Otherwise if just the "test"
159+
// builder has been changed, we warn because a theme is not mandatory for running tests
160+
// with Material. See: https://github.com/angular/material2/issues/14176
161+
if (!isDefaultBuilder && targetName === 'build') {
162+
throw new Error(`Your project is not using the default builders for "${targetName}". The ` +
163+
`Angular Material schematics cannot add a theme to the workspace configuration if the ` +
164+
`builder has been changed. Exiting..`);
165+
} else if (!isDefaultBuilder) {
166+
console.warn(`Your project is not using the default builders for "${targetName}". This ` +
167+
`means that we cannot add the configured theme to the "${targetName}" target.`);
158168
}
169+
170+
return isDefaultBuilder;
159171
}

0 commit comments

Comments
 (0)