Skip to content

Commit d1e59a2

Browse files
devversionandrewseguin
authored andcommitted
fix(ng-add): do not add theme file if existing theme is set up (#13468)
* fix(ng-add): do not add theme file if existing theme is set up * No longer adds other theme files to the workspace configuration if a custom theme has been generated in a previous run already. * Instead of silently doing nothing if a prebuilt theme is being generated while another prebuilt theme is already defined, we now replace the previous prebuilt theme with the new theme. * Do not overwrite existing custom-theme files
1 parent 21de68e commit d1e59a2

File tree

2 files changed

+113
-11
lines changed

2 files changed

+113
-11
lines changed

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

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,4 +207,77 @@ describe('ng-add schematic', () => {
207207
'Expected the project app module to not import the "NoopAnimationsModule".');
208208
});
209209
});
210+
211+
describe('theme files', () => {
212+
213+
/** Path to the default prebuilt theme file that will be added when running ng-add. */
214+
const defaultPrebuiltThemePath =
215+
'./node_modules/@angular/material/prebuilt-themes/indigo-pink.css';
216+
217+
/** Writes a specific style file to the workspace in the given tree */
218+
function writeStyleFileToWorkspace(tree: Tree, stylePath: string) {
219+
const workspace = getWorkspace(tree);
220+
const project = getProjectFromWorkspace(workspace);
221+
const buildOptions = getProjectTargetOptions(project, 'build');
222+
223+
if (!buildOptions.styles) {
224+
buildOptions.styles = [stylePath];
225+
} else {
226+
buildOptions.styles.push(stylePath);
227+
}
228+
229+
tree.overwrite('/angular.json', JSON.stringify(workspace, null, 2));
230+
}
231+
232+
it('should replace existing prebuilt theme files', () => {
233+
const existingThemePath =
234+
'./node_modules/@angular/material/prebuilt-themes/purple-green.css';
235+
writeStyleFileToWorkspace(appTree, existingThemePath);
236+
237+
const tree = runner.runSchematic('ng-add-setup-project', {}, appTree);
238+
const workspace = getWorkspace(tree);
239+
const project = getProjectFromWorkspace(workspace);
240+
const styles = getProjectTargetOptions(project, 'build').styles;
241+
242+
expect(styles).not.toContain(existingThemePath,
243+
'Expected the existing prebuilt theme file to be removed.');
244+
expect(styles).toContain(defaultPrebuiltThemePath,
245+
'Expected the default prebuilt theme to be added.');
246+
});
247+
248+
it('should not replace existing custom theme files', () => {
249+
spyOn(console, 'warn');
250+
writeStyleFileToWorkspace(appTree, './projects/material/custom-theme.scss');
251+
252+
const tree = runner.runSchematic('ng-add-setup-project', {}, appTree);
253+
const workspace = getWorkspace(tree);
254+
const project = getProjectFromWorkspace(workspace);
255+
const styles = getProjectTargetOptions(project, 'build').styles;
256+
257+
expect(styles).not.toContain(defaultPrebuiltThemePath,
258+
'Expected the default prebuilt theme to be not configured.');
259+
expect(console.warn).toHaveBeenCalledWith(
260+
jasmine.stringMatching(/Cannot add.*already a custom theme/));
261+
});
262+
263+
it('should not add a theme file multiple times', () => {
264+
writeStyleFileToWorkspace(appTree, defaultPrebuiltThemePath);
265+
266+
const tree = runner.runSchematic('ng-add-setup-project', {}, appTree);
267+
const workspace = getWorkspace(tree);
268+
const project = getProjectFromWorkspace(workspace);
269+
const styles = getProjectTargetOptions(project, 'build').styles;
270+
271+
expect(styles).toEqual(['projects/material/src/styles.css', defaultPrebuiltThemePath],
272+
'Expected the "styles.css" file and default prebuilt theme to be the only styles');
273+
});
274+
275+
it('should not overwrite existing custom theme files', () => {
276+
appTree.create('/projects/material/custom-theme.scss', 'custom-theme');
277+
const tree = runner.runSchematic('ng-add-setup-project', {theme: 'custom'}, appTree);
278+
279+
expect(tree.readContent('/projects/material/custom-theme.scss')).toBe('custom-theme',
280+
'Expected the old custom theme content to be unchanged.');
281+
});
282+
});
210283
});

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

Lines changed: 40 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,13 @@ import {getWorkspace} from '@schematics/angular/utility/config';
1919
import {join} from 'path';
2020
import {Schema} from '../schema';
2121
import {createCustomTheme} from './custom-theme';
22+
import {red, bold} from 'chalk';
2223

24+
/** Path segment that can be found in paths that refer to a prebuilt theme. */
25+
const prebuiltThemePathSegment = '@angular/material/prebuilt-themes';
26+
27+
/** Default file name of the custom theme that can be generated. */
28+
const defaultCustomThemeFilename = 'custom-theme.scss';
2329

2430
/** Add pre-built styles to the main project style file. */
2531
export function addThemeToAppStyles(options: Schema): (host: Tree) => Tree {
@@ -60,11 +66,17 @@ function insertCustomTheme(project: WorkspaceProject, projectName: string, host:
6066

6167
// Normalize the path through the devkit utilities because we want to avoid having
6268
// unnecessary path segments and windows backslash delimiters.
63-
const customThemePath = normalize(join(project.sourceRoot, 'custom-theme.scss'));
69+
const customThemePath = normalize(join(project.sourceRoot, defaultCustomThemeFilename));
6470

65-
host.create(customThemePath, themeContent);
71+
if (host.exists(customThemePath)) {
72+
console.warn(red(`Cannot create a custom Angular Material theme because
73+
"${customThemePath}" already exists. Skipping custom theme generation.`));
74+
return;
75+
}
6676

67-
return addStyleToTarget(project, 'build', host, customThemePath, workspace);
77+
host.create(customThemePath, themeContent);
78+
addThemeStyleToTarget(project, 'build', host, customThemePath, workspace);
79+
return;
6880
}
6981

7082
const insertion = new InsertChange(stylesPath, 0, themeContent);
@@ -81,25 +93,42 @@ function insertPrebuiltTheme(project: WorkspaceProject, host: Tree, theme: strin
8193
// Path needs to be always relative to the `package.json` or workspace root.
8294
const themePath = `./node_modules/@angular/material/prebuilt-themes/${theme}.css`;
8395

84-
addStyleToTarget(project, 'build', host, themePath, workspace);
85-
addStyleToTarget(project, 'test', host, themePath, workspace);
96+
addThemeStyleToTarget(project, 'build', host, themePath, workspace);
97+
addThemeStyleToTarget(project, 'test', host, themePath, workspace);
8698
}
8799

88-
/** Adds a style entry to the given project target. */
89-
function addStyleToTarget(project: WorkspaceProject, targetName: string, host: Tree,
100+
/** Adds a theming style entry to the given project target options. */
101+
function addThemeStyleToTarget(project: WorkspaceProject, targetName: string, host: Tree,
90102
assetPath: string, workspace: WorkspaceSchema) {
103+
91104
const targetOptions = getProjectTargetOptions(project, targetName);
92105

93106
if (!targetOptions.styles) {
94107
targetOptions.styles = [assetPath];
95108
} else {
96109
const existingStyles = targetOptions.styles.map(s => typeof s === 'string' ? s : s.input);
97-
const hasGivenTheme = existingStyles.find(s => s.includes(assetPath));
98-
const hasOtherTheme = existingStyles.find(s => s.includes('material/prebuilt'));
99110

100-
if (!hasGivenTheme && !hasOtherTheme) {
101-
targetOptions.styles.unshift(assetPath);
111+
for (let [index, stylePath] of existingStyles.entries()) {
112+
// If the given asset is already specified in the styles, we don't need to do anything.
113+
if (stylePath === assetPath) {
114+
return;
115+
}
116+
117+
// In case a prebuilt theme is already set up, we can safely replace the theme with the new
118+
// theme file. If a custom theme is set up, we are not able to safely replace the custom
119+
// theme because these files can contain custom styles, while prebuilt themes are
120+
// always packaged and considered replaceable.
121+
if (stylePath.includes(defaultCustomThemeFilename)) {
122+
console.warn(red(`Cannot add "${bold(assetPath)} to the CLI project configuration ` +
123+
`because there is already a custom theme file referenced. Please manually add ` +
124+
`the "${bold(assetPath)}" style file to your configuration.`));
125+
return;
126+
} else if (stylePath.includes(prebuiltThemePathSegment)) {
127+
targetOptions.styles.splice(index, 1);
128+
}
102129
}
130+
131+
targetOptions.styles.unshift(assetPath);
103132
}
104133

105134
host.overwrite('angular.json', JSON.stringify(workspace, null, 2));

0 commit comments

Comments
 (0)