Skip to content

Commit

Permalink
feat(@schematics/angular): consistent naming of options and arguments…
Browse files Browse the repository at this point in the history
… that do the same thing

This aligns options that do the same thing:
1) `skipSpecs` and `spec` has been deprecated in favor of `skipTests`.
2) `styleext` has been deprecated in favor of `style` since the latest is two words.

Fixes #12784
  • Loading branch information
alan-agius4 authored and kyliau committed Dec 11, 2018
1 parent 1cdeccf commit a12a4e0
Show file tree
Hide file tree
Showing 23 changed files with 158 additions and 53 deletions.
42 changes: 31 additions & 11 deletions packages/angular/cli/lib/config/schema.json
Expand Up @@ -139,6 +139,11 @@
"type": "string",
"default": "css"
},
"style": {
"description": "The file extension to use for style files.",
"type": "string",
"default": "css"
},
"viewEncapsulation": {
"description": "Specifies the view encapsulation strategy.",
"enum": ["Emulated", "Native", "None", "ShadowDom"],
Expand Down Expand Up @@ -186,6 +191,11 @@
"type": "boolean",
"description": "Specifies if a spec file is generated.",
"default": true
},
"skipTests": {
"type": "boolean",
"description": "When true, does not create test files.",
"default": false
}
}
},
Expand All @@ -203,11 +213,6 @@
"description": "The scope for the generated routing.",
"default": "Child"
},
"spec": {
"type": "boolean",
"description": "Specifies if a spec file is generated.",
"default": true
},
"flat": {
"type": "boolean",
"description": "Flag to indicate if a directory is created.",
Expand Down Expand Up @@ -236,8 +241,13 @@
},
"spec": {
"type": "boolean",
"default": true,
"description": "Specifies if a spec file is generated."
"description": "Specifies if a spec file is generated.",
"default": true
},
"skipTests": {
"type": "boolean",
"description": "When true, does not create test files.",
"default": false
}
}
},
Expand All @@ -251,8 +261,13 @@
},
"spec": {
"type": "boolean",
"default": true,
"description": "Specifies if a spec file is generated."
"description": "Specifies if a spec file is generated.",
"default": true
},
"skipTests": {
"type": "boolean",
"description": "When true, does not create test files.",
"default": false
},
"skipImport": {
"type": "boolean",
Expand All @@ -277,8 +292,13 @@
"properties": {
"spec": {
"type": "boolean",
"default": true,
"description": "Specifies if a spec file is generated."
"description": "Specifies if a spec file is generated.",
"default": true
},
"skipTests": {
"type": "boolean",
"description": "When true, does not create test files.",
"default": false
}
}
}
Expand Down
12 changes: 6 additions & 6 deletions packages/schematics/angular/application/index.ts
Expand Up @@ -136,7 +136,7 @@ function addAppToWorkspaceFile(options: ApplicationOptions, workspace: Workspace
if (!(`@schematics/angular:${type}` in schematics)) {
schematics[`@schematics/angular:${type}`] = {};
}
(schematics[`@schematics/angular:${type}`] as JsonObject).spec = false;
(schematics[`@schematics/angular:${type}`] as JsonObject).skipTests = true;
});
}

Expand Down Expand Up @@ -264,15 +264,15 @@ export default function (options: ApplicationOptions): Rule {
{
inlineStyle: options.inlineStyle,
inlineTemplate: options.inlineTemplate,
spec: !options.skipTests,
styleext: options.style,
skipTests: options.skipTests,
style: options.style,
viewEncapsulation: options.viewEncapsulation,
} :
{
inlineStyle: true,
inlineTemplate: true,
spec: false,
styleext: options.style,
skipTests: true,
style: options.style,
};

const workspace = getWorkspace(host);
Expand Down Expand Up @@ -361,7 +361,7 @@ export default function (options: ApplicationOptions): Rule {
mergeWith(
apply(url('./other-files'), [
componentOptions.inlineTemplate ? filter(path => !path.endsWith('.html')) : noop(),
!componentOptions.spec ? filter(path => !path.endsWith('.spec.ts')) : noop(),
componentOptions.skipTests ? filter(path => !/[.|-]spec.ts$/.test(path)) : noop(),
template({
utils: strings,
...options as any, // tslint:disable-line:no-any
Expand Down
2 changes: 0 additions & 2 deletions packages/schematics/angular/application/index_spec.ts
Expand Up @@ -29,8 +29,6 @@ describe('Application Schematic', () => {
inlineStyle: false,
inlineTemplate: false,
routing: false,
style: 'css',
skipTests: false,
skipPackageJson: false,
};

Expand Down
Expand Up @@ -28,7 +28,7 @@ import { Component } from '@angular/core';
`,<% } else { %>
templateUrl: './app.component.html',<% } if(inlineStyle) { %>
styles: []<% } else { %>
styleUrls: ['./app.component.<%= styleext %>']<% } %>
styleUrls: ['./app.component.<%= style %>']<% } %>
})
export class AppComponent {
title = '<%= name %>';
Expand Down
5 changes: 4 additions & 1 deletion packages/schematics/angular/class/index.ts
Expand Up @@ -42,8 +42,11 @@ export default function (options: ClassOptions): Rule {
options.name = parsedPath.name;
options.path = parsedPath.path;

// todo remove these when we remove the deprecations
options.skipTests = options.skipTests || !options.spec;

const templateSource = apply(url('./files'), [
options.spec ? noop() : filter(path => !path.endsWith('.spec.ts')),
options.skipTests ? filter(path => !path.endsWith('.spec.ts')) : noop(),
template({
...strings,
...options,
Expand Down
8 changes: 7 additions & 1 deletion packages/schematics/angular/class/schema.json
Expand Up @@ -29,7 +29,13 @@
},
"spec": {
"type": "boolean",
"description": "When true, generates a \"spec.ts\" test file for the new class.",
"description": "When true (the default), generates a \"spec.ts\" test file for the new class.",
"default": true,
"x-deprecated": "Use \"skipTests\" instead."
},
"skipTests": {
"type": "boolean",
"description": "When true, does not create \"spec.ts\" test files for the new class.",
"default": false
},
"type": {
Expand Down
Expand Up @@ -9,7 +9,7 @@ import { Component, OnInit<% if(!!viewEncapsulation) { %>, ViewEncapsulation<% }
`,<% } else { %>
templateUrl: './<%= dasherize(name) %>.component.html',<% } if(inlineStyle) { %>
styles: []<% } else { %>
styleUrls: ['./<%= dasherize(name) %>.component.<%= styleext %>']<% } %><% if(!!viewEncapsulation) { %>,
styleUrls: ['./<%= dasherize(name) %>.component.<%= style %>']<% } %><% if(!!viewEncapsulation) { %>,
encapsulation: ViewEncapsulation.<%= viewEncapsulation %><% } if (changeDetection !== 'Default') { %>,
changeDetection: ChangeDetectionStrategy.<%= changeDetection %><% } %>
})
Expand Down
13 changes: 10 additions & 3 deletions packages/schematics/angular/component/index.ts
Expand Up @@ -125,7 +125,7 @@ function buildSelector(options: ComponentOptions, projectPrefix: string) {
}


export default function(options: ComponentOptions): Rule {
export default function (options: ComponentOptions): Rule {
return (host: Tree) => {
if (!options.project) {
throw new SchematicsException('Option (project) is required.');
Expand All @@ -143,12 +143,19 @@ export default function(options: ComponentOptions): Rule {
options.path = parsedPath.path;
options.selector = options.selector || buildSelector(options, project.prefix);

// todo remove these when we remove the deprecations
options.style = (
options.style && options.style !== 'css'
? options.style : options.styleext
) || 'css';
options.skipTests = options.skipTests || !options.spec;

validateName(options.name);
validateHtmlSelector(options.selector);

const templateSource = apply(url('./files'), [
options.spec ? noop() : filter(path => !path.endsWith('.spec.ts')),
options.inlineStyle ? filter(path => !path.endsWith('.__styleext__')) : noop(),
options.skipTests ? filter(path => !path.endsWith('.spec.ts')) : noop(),
options.inlineStyle ? filter(path => !path.endsWith('.__style__')) : noop(),
options.inlineTemplate ? filter(path => !path.endsWith('.html')) : noop(),
template({
...strings,
Expand Down
38 changes: 34 additions & 4 deletions packages/schematics/angular/component/index_spec.ts
Expand Up @@ -24,8 +24,8 @@ describe('Component Schematic', () => {
inlineStyle: false,
inlineTemplate: false,
changeDetection: ChangeDetection.Default,
styleext: 'css',
spec: true,
style: 'css',
skipTests: false,
module: undefined,
export: false,
project: 'bar',
Expand Down Expand Up @@ -249,8 +249,8 @@ describe('Component Schematic', () => {
expect(tree.files).not.toContain('/projects/bar/src/app/foo/foo.component.css');
});

it('should respect the styleext option', () => {
const options = { ...defaultOptions, styleext: 'scss' };
it('should respect the style option', () => {
const options = { ...defaultOptions, style: 'scss' };
const tree = schematicRunner.runSchematic('component', options, appTree);
const content = tree.readContent('/projects/bar/src/app/foo/foo.component.ts');
expect(content).toMatch(/styleUrls: \['.\/foo.component.scss/);
Expand Down Expand Up @@ -310,4 +310,34 @@ describe('Component Schematic', () => {
appTree = schematicRunner.runSchematic('component', defaultOptions, appTree);
expect(appTree.files).toContain('/projects/bar/custom/app/foo/foo.component.ts');
});

// testing deprecating options don't cause conflicts
it('should respect the deprecated styleext (scss) option', () => {
const options = { ...defaultOptions, style: undefined, styleext: 'scss' };
const tree = schematicRunner.runSchematic('component', options, appTree);
const files = tree.files;
expect(files).toContain('/projects/bar/src/app/foo/foo.component.scss');
});

it('should respect the deprecated styleext (css) option', () => {
const options = { ...defaultOptions, style: undefined, styleext: 'css' };
const tree = schematicRunner.runSchematic('component', options, appTree);
const files = tree.files;
expect(files).toContain('/projects/bar/src/app/foo/foo.component.css');
});

it('should respect the deprecated spec option when false', () => {
const options = { ...defaultOptions, skipTests: undefined, spec: false };
const tree = schematicRunner.runSchematic('component', options, appTree);
const files = tree.files;
expect(files).not.toContain('/projects/bar/src/app/foo/foo.component.spec.ts');
});

it('should respect the deprecated spec option when true', () => {
const options = { ...defaultOptions, skipTests: false, spec: true };
const tree = schematicRunner.runSchematic('component', options, appTree);
const files = tree.files;
expect(files).toContain('/projects/bar/src/app/foo/foo.component.spec.ts');
});

});
14 changes: 13 additions & 1 deletion packages/schematics/angular/component/schema.json
Expand Up @@ -67,14 +67,26 @@
]
},
"styleext": {
"description": "The file extension to use for style files.",
"type": "string",
"default": "css",
"x-deprecated": "Use \"style\" instead."
},
"style": {
"description": "The file extension to use for style files.",
"type": "string",
"default": "css"
},
"spec": {
"type": "boolean",
"description": "When true (the default), generates a \"spec.ts\" test file for the new component.",
"default": true
"default": true,
"x-deprecated": "Use \"skipTests\" instead."
},
"skipTests": {
"type": "boolean",
"description": "When true, does not create \"spec.ts\" test files for the new component.",
"default": false
},
"flat": {
"type": "boolean",
Expand Down
5 changes: 4 additions & 1 deletion packages/schematics/angular/directive/index.ts
Expand Up @@ -121,8 +121,11 @@ export default function (options: DirectiveOptions): Rule {

validateHtmlSelector(options.selector);

// todo remove these when we remove the deprecations
options.skipTests = options.skipTests || !options.spec;

const templateSource = apply(url('./files'), [
options.spec ? noop() : filter(path => !path.endsWith('.spec.ts')),
options.skipTests ? filter(path => !path.endsWith('.spec.ts')) : noop(),
template({
...strings,
'if-flat': (s: string) => options.flat ? '' : s,
Expand Down
2 changes: 0 additions & 2 deletions packages/schematics/angular/directive/index_spec.ts
Expand Up @@ -18,7 +18,6 @@ describe('Directive Schematic', () => {
);
const defaultOptions: DirectiveOptions = {
name: 'foo',
spec: true,
module: undefined,
export: false,
prefix: 'app',
Expand All @@ -37,7 +36,6 @@ describe('Directive Schematic', () => {
inlineStyle: false,
inlineTemplate: false,
routing: false,
style: 'css',
skipTests: false,
skipPackageJson: false,
};
Expand Down
12 changes: 9 additions & 3 deletions packages/schematics/angular/directive/schema.json
Expand Up @@ -44,7 +44,13 @@
"spec": {
"type": "boolean",
"description": "When true (the default), generates a \"spec.ts\" test file for the new directive.",
"default": true
"default": true,
"x-deprecated": "Use \"skipTests\" instead."
},
"skipTests": {
"type": "boolean",
"description": "When true, does not create \"spec.ts\" test files for the new class.",
"default": false
},
"skipImport": {
"type": "boolean",
Expand All @@ -61,7 +67,7 @@
"description": "When true (the default), creates the new files at the top level of the current project.",
"default": true
},
"module": {
"module": {
"type": "string",
"description": "The declaring NgModule.",
"alias": "m"
Expand All @@ -80,4 +86,4 @@
"required": [
"name"
]
}
}
5 changes: 4 additions & 1 deletion packages/schematics/angular/guard/index.ts
Expand Up @@ -41,8 +41,11 @@ export default function (options: GuardOptions): Rule {
options.name = parsedPath.name;
options.path = parsedPath.path;

// todo remove these when we remove the deprecations
options.skipTests = options.skipTests || !options.spec;

const templateSource = apply(url('./files'), [
options.spec ? noop() : filter(path => !path.endsWith('.spec.ts')),
options.skipTests ? filter(path => !path.endsWith('.spec.ts')) : noop(),
template({
...strings,
...options,
Expand Down
6 changes: 2 additions & 4 deletions packages/schematics/angular/guard/index_spec.ts
Expand Up @@ -18,7 +18,6 @@ describe('Guard Schematic', () => {
);
const defaultOptions: GuardOptions = {
name: 'foo',
spec: true,
flat: true,
project: 'bar',
};
Expand All @@ -33,7 +32,6 @@ describe('Guard Schematic', () => {
inlineStyle: false,
inlineTemplate: false,
routing: false,
style: 'css',
skipTests: false,
skipPackageJson: false,
};
Expand All @@ -50,8 +48,8 @@ describe('Guard Schematic', () => {
expect(files).toContain('/projects/bar/src/app/foo.guard.ts');
});

it('should respect the spec flag', () => {
const options = { ...defaultOptions, spec: false };
it('should respect the skipTests flag', () => {
const options = { ...defaultOptions, skipTests: true };

const tree = schematicRunner.runSchematic('guard', options, appTree);
const files = tree.files;
Expand Down

0 comments on commit a12a4e0

Please sign in to comment.