From 51da6a680657b4ae9191c215f582715d61e407fb Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Tue, 21 Aug 2018 20:40:38 +0200 Subject: [PATCH] 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 | 48 +++++++++++--- src/lib/schematics/install/index.ts | 2 +- src/lib/schematics/install/theming/theming.ts | 63 +++++++++++-------- .../schematics/utils/project-style-file.ts | 32 +++++++--- 4 files changed, 100 insertions(+), 45 deletions(-) diff --git a/src/lib/schematics/install/index.spec.ts b/src/lib/schematics/install/index.spec.ts index 393bc0f273d..6047c561554 100644 --- a/src/lib/schematics/install/index.spec.ts +++ b/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( ' { + 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; }'); + }); }); diff --git a/src/lib/schematics/install/index.ts b/src/lib/schematics/install/index.ts index 2362cd8e054..3cd9f44e173 100644 --- a/src/lib/schematics/install/index.ts +++ b/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; diff --git a/src/lib/schematics/install/theming/theming.ts b/src/lib/schematics/install/theming/theming.ts index 3f10f3e4c0f..670a0da9fcb 100644 --- a/src/lib/schematics/install/theming/theming.ts +++ b/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.'); + } } diff --git a/src/lib/schematics/utils/project-style-file.ts b/src/lib/schematics/utils/project-style-file.ts index 3e73d6b263c..8a706be0b5f 100644 --- a/src/lib/schematics/utils/project-style-file.ts +++ b/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; }