Skip to content
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

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

Merged
merged 2 commits into from
Aug 21, 2018
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
48 changes: 39 additions & 9 deletions src/lib/schematics/install/index.spec.ts
Original file line number Diff line number Diff line change
@@ -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', () => {
Expand All @@ -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'));
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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; }');
});
});
2 changes: 1 addition & 1 deletion src/lib/schematics/install/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
63 changes: 37 additions & 26 deletions src/lib/schematics/install/theming/theming.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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);
}
Expand All @@ -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. */
Expand Down Expand Up @@ -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.');
}
}
32 changes: 23 additions & 9 deletions src/lib/schematics/utils/project-style-file.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}