fix(ng-add): do not incorrectly insert custom-theme into CSS files (#…

…12711)

* Currently the schematics add the custom-theme SCSS code to any project style file (`styles.EXT`). This is incorrect because the SCSS needs to be placed inside of a SCSS file. Instead of throwing an error we just create a new SCSS file with the theme and add it to the workspace config.
* Aligns the global font-family statement with the current Angular Material typography font-family.
* Adds missing tests for the global app styles.
* Removes an accidentally committed `console.log()`.

src/lib/schematics/install/index.spec.ts

@@ -1,10 +1,11 @@
 import {Tree} from '@angular-devkit/schematics';
 import {SchematicTestRunner} from '@angular-devkit/schematics/testing';
+import {getProjectStyleFile} from '@angular/material/schematics/utils/project-style-file';
 import {getIndexHtmlPath} from './fonts/project-index-html';
 import {getProjectFromWorkspace} from '../utils/get-project';
 import {getFileContent} from '@schematics/angular/utility/test';
 import {collectionPath, createTestApp} from '../test-setup/test-app';
-import {getWorkspace} from '@schematics/angular/utility/config';
+import {getWorkspace, WorkspaceProject} from '@schematics/angular/utility/config';
 import {normalize} from '@angular-devkit/core';
 
 describe('material-install-schematic', () => {
@@ -16,6 +17,16 @@ describe('material-install-schematic', () => {
     runner = new SchematicTestRunner('schematics', collectionPath);
   });
 
+  /** Expects the given file to be in the styles of the specified workspace project. */
+  function expectProjectStyleFile(project: WorkspaceProject, filePath: string) {
+    const architect = project.architect!;
+
+    expect(architect!['build']).toBeTruthy();
+    expect(architect!['build']['options']).toBeTruthy();
+    expect(architect!['build']['options']['styles']).toContain(filePath,
+        `Expected "${filePath}" to be added to the project styles in the workspace.`);
+  }
+
   it('should update package.json', () => {
     const tree = runner.runSchematic('ng-add', {}, appTree);
     const packageJson = JSON.parse(getFileContent(tree, '/package.json'));
@@ -33,17 +44,11 @@ describe('material-install-schematic', () => {
     const workspace = getWorkspace(tree);
     const project = getProjectFromWorkspace(workspace);
 
-    console.log(tree.files);
-
-    expect(project.architect!['build']).toBeTruthy();
-    expect(project.architect!['build']['options']).toBeTruthy();
-    expect(project.architect!['build']['options']['styles']).toContain(
-      './node_modules/@angular/material/prebuilt-themes/indigo-pink.css');
+    expectProjectStyleFile(project,
+        './node_modules/@angular/material/prebuilt-themes/indigo-pink.css');
   });
 
   it('should support adding a custom theme', () => {
-    // TODO(devversion): currently a "custom" theme does only work for projects using SCSS.
-    // TODO(devversion): Throw an error if a custom theme is being installed in a CSS project.
     appTree = createTestApp({style: 'scss'});
 
     const tree = runner.runSchematic('ng-add', {theme: 'custom'}, appTree);
@@ -59,6 +64,18 @@ describe('material-install-schematic', () => {
     expect(src.indexOf(`$app-primary`)).toBeGreaterThan(-1);
   });
 
+  it('should create a custom theme file if no SCSS file could be found', () => {
+    appTree = createTestApp({style: 'css'});
+
+    const tree = runner.runSchematic('ng-add', {theme: 'custom'}, appTree);
+    const workspace = getWorkspace(tree);
+    const project = getProjectFromWorkspace(workspace);
+    const expectedStylesPath = normalize(`/${project.root}/src/custom-theme.scss`);
+
+    expect(tree.files).toContain(expectedStylesPath, 'Expected a custom theme file to be created');
+    expectProjectStyleFile(project, 'projects/material/src/custom-theme.scss');
+  });
+
   it('should add font links', () => {
     const tree = runner.runSchematic('ng-add', {}, appTree);
     const workspace = getWorkspace(tree);
@@ -76,4 +93,17 @@ describe('material-install-schematic', () => {
     expect(htmlContent).toContain(
       '  <link href="https://fonts.googleapis.com/css?family=Roboto:300,400,500"');
   });
+
+  it('should add material app styles', () => {
+    const tree = runner.runSchematic('ng-add', {}, appTree);
+    const workspace = getWorkspace(tree);
+    const project = getProjectFromWorkspace(workspace);
+
+    const defaultStylesPath = getProjectStyleFile(project);
+    const htmlContent = tree.read(defaultStylesPath)!.toString();
+
+    expect(htmlContent).toContain('html, body { height: 100%; }');
+    expect(htmlContent).toContain(
+        'body { margin: 0; font-family: Roboto, "Helvetica Neue", sans-serif; }');
+  });
 });

src/lib/schematics/install/index.ts

@@ -99,7 +99,7 @@ function addMaterialAppStyles(options: Schema) {
     const htmlContent = buffer.toString();
     const insertion = '\n' +
       `html, body { height: 100%; }\n` +
-      `body { margin: 0; font-family: 'Roboto', sans-serif; }\n`;
+      `body { margin: 0; font-family: Roboto, "Helvetica Neue", sans-serif; }\n`;
 
     if (htmlContent.includes(insertion)) {
       return;

src/lib/schematics/install/theming/theming.ts

@@ -6,9 +6,11 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
+import {normalize} from '@angular-devkit/core';
 import {SchematicsException, Tree} from '@angular-devkit/schematics';
 import {InsertChange} from '@schematics/angular/utility/change';
 import {getWorkspace, WorkspaceProject, WorkspaceSchema} from '@schematics/angular/utility/config';
+import {join} from 'path';
 import {getProjectFromWorkspace} from '../../utils/get-project';
 import {getProjectStyleFile} from '../../utils/project-style-file';
 import {Schema} from '../schema';
@@ -20,14 +22,14 @@ 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.
-    assertDefaultProjectConfig(project);
+    assertDefaultBuildersConfigured(project);
 
-    const themeName = options.theme || 'indigo-pink';
     if (themeName === 'custom') {
-      insertCustomTheme(project, options.project, host);
+      insertCustomTheme(project, options.project, host, workspace);
     } else {
       insertPrebuiltTheme(project, host, themeName, workspace, options.project);
     }
@@ -36,19 +38,31 @@ export function addThemeToAppStyles(options: Schema): (host: Tree) => Tree {
   };
 }
 
-/** Insert a custom theme to styles.scss file. */
-function insertCustomTheme(project: WorkspaceProject, projectName: string, host: Tree) {
-  const stylesPath = getProjectStyleFile(project);
-  const buffer = host.read(stylesPath);
+/**
+ * Insert a custom theme to project style file. If no valid style file could be found, a new
+ * Scss file for the custom theme will be created.
+ */
+function insertCustomTheme(project: WorkspaceProject, projectName: string, host: Tree,
+                           workspace: WorkspaceSchema) {
 
-  if (buffer) {
-    const insertion = new InsertChange(stylesPath, 0, createCustomTheme(projectName));
-    const recorder = host.beginUpdate(stylesPath);
-    recorder.insertLeft(insertion.pos, insertion.toAdd);
-    host.commitUpdate(recorder);
-  } else {
-    console.warn(`Skipped custom theme; could not find file: ${stylesPath}`);
+  const stylesPath = getProjectStyleFile(project, 'scss');
+  const themeContent = createCustomTheme(projectName);
+
+  if (!stylesPath) {
+    // Normalize the path through the devkit utilities because we want to avoid having
+    // unnecessary path segments and windows backslash delimiters.
+    const customThemePath = normalize(join(project.sourceRoot, 'custom-theme.scss'));
+
+    host.create(customThemePath, themeContent);
+    addStyleToTarget(project.architect['build'], host, customThemePath, workspace);
+    return;
   }
+
+  const insertion = new InsertChange(stylesPath, 0, themeContent);
+  const recorder = host.beginUpdate(stylesPath);
+
+  recorder.insertLeft(insertion.pos, insertion.toAdd);
+  host.commitUpdate(recorder);
 }
 
 /** Insert a pre-built theme into the angular.json file. */
@@ -87,23 +101,20 @@ function addStyleToTarget(target: any, host: Tree, asset: string, workspace: Wor
   host.overwrite('angular.json', JSON.stringify(workspace, null, 2));
 }
 
-/** Throws if the project is not using the default build and test config. */
-function assertDefaultProjectConfig(project: WorkspaceProject) {
-  if (!isProjectUsingDefaultConfig(project)) {
-    throw new SchematicsException('Your project is not using the default configuration for ' +
-     'build and test. The Angular Material schematics can only be used with the default ' +
-     'configuration');
-  }
-}
-
-/** Gets whether the Angular CLI project is using the default build configuration. */
-function isProjectUsingDefaultConfig(project: WorkspaceProject) {
+/** Throws if the project is not using the default Angular devkit builders. */
+function assertDefaultBuildersConfigured(project: WorkspaceProject) {
   const defaultBuilder = '@angular-devkit/build-angular:browser';
   const defaultTestBuilder = '@angular-devkit/build-angular:karma';
 
-  return project.architect &&
+  const hasDefaultBuilders = project.architect &&
       project.architect['build'] &&
       project.architect['build']['builder'] === defaultBuilder &&
       project.architect['test'] &&
       project.architect['test']['builder'] === defaultTestBuilder;
+
+  if (!hasDefaultBuilders) {
+    throw new SchematicsException(
+      'Your project is not using the default builders for build and test. The Angular Material ' +
+      'schematics can only be used if the original builders from the Angular CLI are configured.');
+  }
 }

src/lib/schematics/utils/project-style-file.ts

@@ -7,29 +7,43 @@
  */
 
 import {normalize} from '@angular-devkit/core';
-import {SchematicsException} from '@angular-devkit/schematics';
 import {WorkspaceProject} from '@schematics/angular/utility/config';
 
-/** Looks for the primary style file for a given project and returns its path. */
-export function getProjectStyleFile(project: WorkspaceProject): string {
+/** Regular expression that matches all possible Angular CLI default style files. */
+const defaultStyleFileRegex = /styles\.(c|le|sc)ss/;
+
+/** Regular expression that matches all files that have a proper stylesheet extension. */
+const validStyleFileRegex = /\.(c|le|sc)ss/;
+
+/**
+ * Gets a style file with the given extension in a project and returns its path. If no
+ * extension is specified, any style file with a valid extension will be returned.
+ */
+export function getProjectStyleFile(project: WorkspaceProject, extension?: string): string | null {
   const buildTarget = project.architect['build'];
 
   if (buildTarget.options && buildTarget.options.styles && buildTarget.options.styles.length) {
     const styles = buildTarget.options.styles.map(s => typeof s === 'string' ? s : s.input);
 
-    // First, see if any of the assets is called "styles.(le|sc|c)ss", which is the default
-    // "main" style sheet.
-    const defaultMainStylePath = styles.find(a => /styles\.(c|le|sc)ss/.test(a));
+    // Look for the default style file that is generated for new projects by the Angular CLI. This
+    // default style file is usually called `styles.ext` unless it has been changed explicitly.
+    const defaultMainStylePath = styles
+      .find(file => extension ? file === `styles.${extension}` : defaultStyleFileRegex.test(file));
+
     if (defaultMainStylePath) {
       return normalize(defaultMainStylePath);
     }
 
-    // If there was no obvious default file, use the first style asset.
-    const fallbackStylePath = styles.find(a => /\.(c|le|sc)ss/.test(a));
+    // If no default style file could be found, use the first style file that matches the given
+    // extension. If no extension specified explicitly, we look for any file with a valid style
+    // file extension.
+    const fallbackStylePath = styles
+      .find(file => extension ? file.endsWith(`.${extension}`) : validStyleFileRegex.test(file));
+
     if (fallbackStylePath) {
       return normalize(fallbackStylePath);
     }
   }
 
-  throw new SchematicsException('No style files could be found into which a theme could be added.');
+  return null;
 }