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

feat(ng-add): respect project default style extension #12618

Merged
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
3 changes: 1 addition & 2 deletions src/lib/schematics/address-form/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,7 @@
},
"styleext": {
"description": "The file extension to be used for style files.",
"type": "string",
"default": "css"
"type": "string"
},
"spec": {
"type": "boolean",
Expand Down
24 changes: 17 additions & 7 deletions src/lib/schematics/dashboard/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,18 @@ import {Schema} from './schema';

describe('material-dashboard-schematic', () => {
let runner: SchematicTestRunner;
const options: Schema = {

const baseOptions: Schema = {
name: 'foo',
project: 'material',
changeDetection: 'Default',
styleext: 'css',
spec: true,
export: false,
};

beforeEach(() => {
runner = new SchematicTestRunner('schematics', collectionPath);
});

it('should create dashboard files and add them to module', () => {
const tree = runner.runSchematic('dashboard', { ...options }, createTestApp());
const tree = runner.runSchematic('dashboard', baseOptions, createTestApp());
const files = tree.files;

expect(files).toContain('/projects/material/src/app/foo/foo.component.css');
Expand All @@ -33,7 +30,7 @@ describe('material-dashboard-schematic', () => {
});

it('should add dashboard imports to module', () => {
const tree = runner.runSchematic('dashboard', { ...options }, createTestApp());
const tree = runner.runSchematic('dashboard', baseOptions, createTestApp());
const moduleContent = getFileContent(tree, '/projects/material/src/app/app.module.ts');

expect(moduleContent).toContain('MatGridListModule');
Expand All @@ -47,4 +44,17 @@ describe('material-dashboard-schematic', () => {
`import { MatGridListModule, MatCardModule, MatMenuModule, MatIconModule, MatButtonModule } from '@angular/material';`);
});

it('should support passing the style extension option', () => {
const tree = runner.runSchematic(
'dashboard', {styleext: 'scss', ...baseOptions}, createTestApp());

expect(tree.files).toContain('/projects/material/src/app/foo/foo.component.scss');
});

it('should fallback to the default angular:component style extension', () => {
const tree = runner.runSchematic('dashboard', baseOptions, createTestApp({style: 'less'}));

expect(tree.files).toContain('/projects/material/src/app/foo/foo.component.less');
});

});
3 changes: 1 addition & 2 deletions src/lib/schematics/dashboard/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,7 @@
},
"styleext": {
"description": "The file extension to be used for style files.",
"type": "string",
"default": "css"
"type": "string"
},
"spec": {
"type": "boolean",
Expand Down
6 changes: 5 additions & 1 deletion src/lib/schematics/install/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,11 @@ describe('material-install-schematic', () => {
'./node_modules/@angular/material/prebuilt-themes/indigo-pink.css');
});

it('should add custom theme', () => {
it('should support adding a custom theme', () => {
// TODO(devversion): currently a "custom" theme does only work for projects using SCSS.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the CLI not support mixing scss with other preprocessors?

Copy link
Member Author

@devversion devversion Aug 10, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does. The problem with the install schematic right now is that we also just add the custom theme to projects that don't use SCSS.

The schematic does not create a new scss file in a plain css project. It just looks for the default styles.(s)css file and puts the code in there. This is incorrect because the CSS files don't support the SASS syntax.

// 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);

const workspace = getWorkspace(tree);
Expand Down
21 changes: 14 additions & 7 deletions src/lib/schematics/nav/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,21 +6,17 @@ import {collectionPath, createTestApp} from '../test-setup/test-app';
describe('material-nav-schematic', () => {
let runner: SchematicTestRunner;

const options: Schema = {
const baseOptions: Schema = {
name: 'foo',
project: 'material',
changeDetection: 'Default',
styleext: 'css',
spec: true,
export: false,
};

beforeEach(() => {
runner = new SchematicTestRunner('schematics', collectionPath);
});

it('should create nav files and add them to module', () => {
const tree = runner.runSchematic('nav', { ...options }, createTestApp());
const tree = runner.runSchematic('nav', baseOptions, createTestApp());
const files = tree.files;

expect(files).toContain('/projects/material/src/app/foo/foo.component.css');
Expand All @@ -34,7 +30,7 @@ describe('material-nav-schematic', () => {
});

it('should add nav imports to module', () => {
const tree = runner.runSchematic('nav', { ...options }, createTestApp());
const tree = runner.runSchematic('nav', baseOptions, createTestApp());
const moduleContent = getFileContent(tree, '/projects/material/src/app/app.module.ts');

expect(moduleContent).toContain('LayoutModule');
Expand All @@ -50,4 +46,15 @@ describe('material-nav-schematic', () => {
`MatListModule } from '@angular/material';`);
});

it('should support passing the style extension option', () => {
const tree = runner.runSchematic('nav', {styleext: 'scss', ...baseOptions}, createTestApp());

expect(tree.files).toContain('/projects/material/src/app/foo/foo.component.scss');
});

it('should fallback to the default angular:component style extension', () => {
const tree = runner.runSchematic('nav', baseOptions, createTestApp({style: 'less'}));

expect(tree.files).toContain('/projects/material/src/app/foo/foo.component.less');
});
});
3 changes: 1 addition & 2 deletions src/lib/schematics/nav/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,7 @@
},
"styleext": {
"description": "The file extension to be used for style files.",
"type": "string",
"default": "css"
"type": "string"
},
"spec": {
"type": "boolean",
Expand Down
22 changes: 15 additions & 7 deletions src/lib/schematics/table/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,21 +6,17 @@ import {collectionPath, createTestApp} from '../test-setup/test-app';
describe('material-table-schematic', () => {
let runner: SchematicTestRunner;

const options: Schema = {
const baseOptions: Schema = {
name: 'foo',
project: 'material',
changeDetection: 'Default',
styleext: 'css',
spec: true,
export: false,
};

beforeEach(() => {
runner = new SchematicTestRunner('schematics', collectionPath);
});

it('should create table files and add them to module', () => {
const tree = runner.runSchematic('table', { ...options }, createTestApp());
const tree = runner.runSchematic('table', baseOptions, createTestApp());
const files = tree.files;

expect(files).toContain('/projects/material/src/app/foo/foo.component.css');
Expand All @@ -45,8 +41,20 @@ describe('material-table-schematic', () => {
expect(componentContent).toContain('FooDataSource');
});

it('should support passing the style extension option', () => {
const tree = runner.runSchematic('table', {styleext: 'scss', ...baseOptions}, createTestApp());

expect(tree.files).toContain('/projects/material/src/app/foo/foo.component.scss');
});

it('should fallback to the default angular:component style extension', () => {
const tree = runner.runSchematic('table', baseOptions, createTestApp({style: 'less'}));

expect(tree.files).toContain('/projects/material/src/app/foo/foo.component.less');
});

it('should add table imports to module', () => {
const tree = runner.runSchematic('table', { ...options }, createTestApp());
const tree = runner.runSchematic('table', baseOptions, createTestApp());
const moduleContent = getFileContent(tree, '/projects/material/src/app/app.module.ts');

expect(moduleContent).toContain('MatTableModule');
Expand Down
3 changes: 1 addition & 2 deletions src/lib/schematics/table/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,7 @@
},
"styleext": {
"description": "The file extension to be used for style files.",
"type": "string",
"default": "css"
"type": "string"
},
"spec": {
"type": "boolean",
Expand Down
12 changes: 3 additions & 9 deletions src/lib/schematics/test-setup/test-app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export const collectionPath = join(__dirname, '..', 'collection.json');
export const migrationCollection = join(__dirname, '..', 'migration.json');

/** Create a base app used for testing. */
export function createTestApp(): UnitTestTree {
export function createTestApp(appOptions = {}): UnitTestTree {
const baseRunner = new SchematicTestRunner('material-schematics', collectionPath);

const workspaceTree = baseRunner.runExternalSchematic('@schematics/angular', 'workspace', {
Expand All @@ -25,12 +25,6 @@ export function createTestApp(): UnitTestTree {
newProjectRoot: 'projects',
});

return baseRunner.runExternalSchematic('@schematics/angular', 'application', {
name: 'material',
inlineStyle: false,
inlineTemplate: false,
routing: false,
style: 'scss',
skipTests: false,
}, workspaceTree);
return baseRunner.runExternalSchematic('@schematics/angular', 'application',
{...appOptions, name: 'material'}, workspaceTree);
}
3 changes: 1 addition & 2 deletions src/lib/schematics/tree/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,7 @@
},
"styleext": {
"description": "The file extension to be used for style files.",
"type": "string",
"default": "css"
"type": "string"
},
"spec": {
"type": "boolean",
Expand Down
8 changes: 7 additions & 1 deletion src/lib/schematics/utils/build-component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import {validateHtmlSelector, validateName} from '@schematics/angular/utility/va
import {resolve, dirname, join} from 'path';
import {readFileSync} from 'fs';
import * as ts from 'typescript';
import {determineDefaultStyleExt} from './default-style-ext';

function readIntoSourceFile(host: Tree, modulePath: string): ts.SourceFile {
const text = host.read(modulePath);
Expand Down Expand Up @@ -137,7 +138,7 @@ function buildSelector(options: ComponentOptions, projectPrefix: string) {
*/
function indentTextContent(text: string, numSpaces: number): string {
// In the Material project there should be only LF line-endings, but the schematic files
// are not being linted and therefore there can be also use CRLF or just CR line-endings.
// are not being linted and therefore there can be also CRLF or just CR line-endings.
return text.replace(/(\r\n|\r|\n)/g, `$1${' '.repeat(numSpaces)}`);
}

Expand All @@ -160,13 +161,18 @@ export function buildComponent(options: ComponentOptions,
const schematicFilesPath = resolve(dirname(context.schematic.description.path),
schematicFilesUrl);

if (!options.styleext) {
options.styleext = determineDefaultStyleExt(project);
}

if (options.path === undefined) {
options.path = buildDefaultPath(project);
}

options.module = findModuleFromOptions(host, options);

const parsedPath = parseName(options.path, options.name);

options.name = parsedPath.name;
options.path = parsedPath.path;
options.selector = options.selector || buildSelector(options, project.prefix);
Expand Down
31 changes: 31 additions & 0 deletions src/lib/schematics/utils/default-style-ext.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/**
* @license
* Copyright Google LLC All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/

import {WorkspaceProject} from '@schematics/angular/utility/config';

/**
* Style extension that will be used if no default style extension for the CLI project
* could be determined.
*/
const fallbackStyleExtension = 'css';

/**
* Determines the default style extension in the Angular CLI project by looking for the default
* component schematic options. This is necessary for now because when creating CLI projects,
* the CLI only makes the default `--style` option available for the `angular:component` schematic.
*/
export function determineDefaultStyleExt(project: WorkspaceProject): string | null {
if (project.schematics &&
project.schematics['@schematics/angular:component'] &&
project.schematics['@schematics/angular:component']['styleext']) {

return project.schematics['@schematics/angular:component']['styleext'];
}

return fallbackStyleExtension;
}