Skip to content

Commit

Permalink
feat(@schematics/angular): loosen project name validation
Browse files Browse the repository at this point in the history
With this change we update the validation of the libraries and application projects names to fully allow characters that make a valid NPM package name. http://json.schemastore.org/package has been used as reference.

We also remove validators that are no longer needed.

Closes #11051
  • Loading branch information
alan-agius4 authored and filipesilva committed Nov 16, 2021
1 parent 6d0f99a commit 21809e1
Show file tree
Hide file tree
Showing 12 changed files with 86 additions and 102 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ module.exports = function (config) {
suppressAll: true // removes the duplicated traces
},
coverageReporter: {
dir: require('path').join(__dirname, '<%= relativePathToWorkspaceRoot %>/coverage/<%= appName%>'),
dir: require('path').join(__dirname, '<%= relativePathToWorkspaceRoot %>/coverage/<%= folderName%>'),
subdir: '.',
reporters: [
{ type: 'html' },
Expand Down
28 changes: 16 additions & 12 deletions packages/schematics/angular/application/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import {
MergeStrategy,
Rule,
SchematicContext,
SchematicsException,
Tree,
apply,
applyTemplates,
Expand All @@ -28,7 +27,6 @@ import { Schema as ComponentOptions } from '../component/schema';
import { NodeDependencyType, addPackageJsonDependency } from '../utility/dependencies';
import { latestVersions } from '../utility/latest-versions';
import { relativePathToWorkspaceRoot } from '../utility/paths';
import { validateProjectName } from '../utility/validation';
import { getWorkspace, updateWorkspace } from '../utility/workspace';
import { Builders, ProjectType } from '../utility/workspace-models';
import { Schema as ApplicationOptions, Style } from './schema';
Expand Down Expand Up @@ -61,7 +59,11 @@ function addDependenciesToPackageJson(options: ApplicationOptions) {
};
}

function addAppToWorkspaceFile(options: ApplicationOptions, appDir: string): Rule {
function addAppToWorkspaceFile(
options: ApplicationOptions,
appDir: string,
folderName: string,
): Rule {
let projectRoot = appDir;
if (projectRoot) {
projectRoot += '/';
Expand Down Expand Up @@ -151,7 +153,7 @@ function addAppToWorkspaceFile(options: ApplicationOptions, appDir: string): Rul
builder: Builders.Browser,
defaultConfiguration: 'production',
options: {
outputPath: `dist/${options.name}`,
outputPath: `dist/${folderName}`,
index: `${sourceRoot}/index.html`,
main: `${sourceRoot}/main.ts`,
polyfills: `${sourceRoot}/polyfills.ts`,
Expand Down Expand Up @@ -238,12 +240,6 @@ function minimalPathFilter(path: string): boolean {

export default function (options: ApplicationOptions): Rule {
return async (host: Tree) => {
if (!options.name) {
throw new SchematicsException(`Invalid options, "name" is required.`);
}

validateProjectName(options.name);

const appRootSelector = `${options.prefix}-root`;
const componentOptions: Partial<ComponentOptions> = !options.minimal
? {
Expand All @@ -264,13 +260,20 @@ export default function (options: ApplicationOptions): Rule {
const workspace = await getWorkspace(host);
const newProjectRoot = (workspace.extensions.newProjectRoot as string | undefined) || '';
const isRootApp = options.projectRoot !== undefined;

// If scoped project (i.e. "@foo/bar"), convert dir to "foo/bar".
let folderName = options.name.startsWith('@') ? options.name.substr(1) : options.name;
if (/[A-Z]/.test(folderName)) {
folderName = strings.dasherize(folderName);
}

const appDir = isRootApp
? normalize(options.projectRoot || '')
: join(normalize(newProjectRoot), strings.dasherize(options.name));
: join(normalize(newProjectRoot), folderName);
const sourceDir = `${appDir}/src/app`;

return chain([
addAppToWorkspaceFile(options, appDir),
addAppToWorkspaceFile(options, appDir, folderName),
mergeWith(
apply(url('./files'), [
options.minimal ? filter(minimalPathFilter) : noop(),
Expand All @@ -280,6 +283,7 @@ export default function (options: ApplicationOptions): Rule {
relativePathToWorkspaceRoot: relativePathToWorkspaceRoot(appDir),
appName: options.name,
isRootApp,
folderName,
}),
move(appDir),
]),
Expand Down
44 changes: 44 additions & 0 deletions packages/schematics/angular/application/index_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -534,6 +534,15 @@ describe('Application Schematic', () => {
expect(exists).toBeTrue();
});

it(`should create scoped kebab-case project folder names with camelCase project name`, async () => {
const options: ApplicationOptions = { ...defaultOptions, name: '@foo/myCool' };
const tree = await schematicRunner
.runSchematicAsync('application', options, workspaceTree)
.toPromise();
const exists = tree.exists('/projects/foo/my-cool/.browserslistrc');
expect(exists).toBeTrue();
});

it(`should create kebab-case project folder names with PascalCase project name`, async () => {
const options: ApplicationOptions = { ...defaultOptions, name: 'MyCool' };
const tree = await schematicRunner
Expand All @@ -542,4 +551,39 @@ describe('Application Schematic', () => {
const exists = tree.exists('/projects/my-cool/.browserslistrc');
expect(exists).toBeTrue();
});

it(`should create scoped kebab-case project folder names with PascalCase project name`, async () => {
const options: ApplicationOptions = { ...defaultOptions, name: '@foo/MyCool' };
const tree = await schematicRunner
.runSchematicAsync('application', options, workspaceTree)
.toPromise();
const exists = tree.exists('/projects/foo/my-cool/.browserslistrc');
expect(exists).toBeTrue();
});

it('should support creating applications with `_` and `.` in name', async () => {
const options = { ...defaultOptions, name: 'foo.bar_buz' };
const tree = await schematicRunner
.runSchematicAsync('application', options, workspaceTree)
.toPromise();

const exists = tree.exists('/projects/foo.bar_buz/.browserslistrc');
expect(exists).toBeTrue();
});

it('should support creating scoped application', async () => {
const scopedName = '@myscope/myapp';
const options = { ...defaultOptions, name: scopedName };
const tree = await schematicRunner
.runSchematicAsync('application', options, workspaceTree)
.toPromise();

const cfg = JSON.parse(tree.readContent('/angular.json'));
expect(cfg.projects['@myscope/myapp']).toBeDefined();

const karmaConf = getFileContent(tree, '/projects/myscope/myapp/karma.conf.js');
expect(karmaConf).toContain(
`dir: require('path').join(__dirname, '../../../coverage/myscope/myapp')`,
);
});
});
1 change: 1 addition & 0 deletions packages/schematics/angular/application/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
"name": {
"description": "The name of the new app.",
"type": "string",
"pattern": "^(?:@[a-zA-Z0-9-*~][a-zA-Z0-9-*._~]*/)?[a-zA-Z0-9-~][a-zA-Z0-9-._~]*$",
"$default": {
"$source": "argv",
"index": 0
Expand Down
3 changes: 1 addition & 2 deletions packages/schematics/angular/component/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import { addDeclarationToModule, addExportToModule } from '../utility/ast-utils'
import { InsertChange } from '../utility/change';
import { buildRelativePath, findModuleFromOptions } from '../utility/find-module';
import { parseName } from '../utility/parse-name';
import { validateHtmlSelector, validateName } from '../utility/validation';
import { validateHtmlSelector } from '../utility/validation';
import { buildDefaultPath, getWorkspace } from '../utility/workspace';
import { Schema as ComponentOptions, Style } from './schema';

Expand Down Expand Up @@ -131,7 +131,6 @@ export default function (options: ComponentOptions): Rule {
options.selector =
options.selector || buildSelector(options, (project && project.prefix) || '');

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

const skipStyleFile = options.inlineStyle || options.style === Style.None;
Expand Down
8 changes: 8 additions & 0 deletions packages/schematics/angular/component/index_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,14 @@ describe('Component Schematic', () => {
expect(content).toMatch(/selector: 'pre-foo'/);
});

it('should error when name starts with a digit', async () => {
const options = { ...defaultOptions, name: '1-one' };

await expectAsync(
schematicRunner.runSchematicAsync('component', options, appTree).toPromise(),
).toBeRejectedWithError('Selector (app-1-one) is invalid.');
});

it('should use the default project prefix if none is passed', async () => {
const options = { ...defaultOptions, prefix: undefined };

Expand Down
29 changes: 11 additions & 18 deletions packages/schematics/angular/library/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import { join, normalize, strings } from '@angular-devkit/core';
import {
Rule,
SchematicContext,
SchematicsException,
Tree,
apply,
applyTemplates,
Expand All @@ -26,7 +25,6 @@ import { NodeDependencyType, addPackageJsonDependency } from '../utility/depende
import { JSONFile } from '../utility/json-file';
import { latestVersions } from '../utility/latest-versions';
import { relativePathToWorkspaceRoot } from '../utility/paths';
import { validateProjectName } from '../utility/validation';
import { getWorkspace, updateWorkspace } from '../utility/workspace';
import { Builders, ProjectType } from '../utility/workspace-models';
import { Schema as LibraryOptions } from './schema';
Expand Down Expand Up @@ -125,28 +123,23 @@ function addLibToWorkspaceFile(

export default function (options: LibraryOptions): Rule {
return async (host: Tree) => {
if (!options.name) {
throw new SchematicsException(`Invalid options, "name" is required.`);
}
const prefix = options.prefix;

validateProjectName(options.name);

// If scoped project (i.e. "@foo/bar"), convert projectDir to "foo/bar".
const projectName = options.name;
const packageName = strings.dasherize(projectName);
let scopeName = null;
const packageName = options.name;
if (/^@.*\/.*/.test(options.name)) {
const [scope, name] = options.name.split('/');
scopeName = scope.replace(/^@/, '');
const [, name] = options.name.split('/');
options.name = name;
}

const workspace = await getWorkspace(host);
const newProjectRoot = (workspace.extensions.newProjectRoot as string | undefined) || '';

const scopeFolder = scopeName ? strings.dasherize(scopeName) + '/' : '';
const folderName = `${scopeFolder}${strings.dasherize(options.name)}`;
let folderName = packageName.startsWith('@') ? packageName.substr(1) : packageName;
if (/[A-Z]/.test(folderName)) {
folderName = strings.dasherize(folderName);
}

const projectRoot = join(normalize(newProjectRoot), folderName);
const distRoot = `dist/${folderName}`;
const pathImportLib = `${distRoot}/${folderName.replace('/', '-')}`;
Expand All @@ -170,15 +163,15 @@ export default function (options: LibraryOptions): Rule {

return chain([
mergeWith(templateSource),
addLibToWorkspaceFile(options, projectRoot, projectName),
addLibToWorkspaceFile(options, projectRoot, packageName),
options.skipPackageJson ? noop() : addDependenciesToPackageJson(),
options.skipTsConfig ? noop() : updateTsConfig(packageName, pathImportLib, distRoot),
schematic('module', {
name: options.name,
commonModule: false,
flat: true,
path: sourceDir,
project: projectName,
project: packageName,
}),
schematic('component', {
name: options.name,
Expand All @@ -188,13 +181,13 @@ export default function (options: LibraryOptions): Rule {
flat: true,
path: sourceDir,
export: true,
project: projectName,
project: packageName,
}),
schematic('service', {
name: options.name,
flat: true,
path: sourceDir,
project: projectName,
project: packageName,
}),
(_tree: Tree, context: SchematicContext) => {
if (!options.skipPackageJson && !options.skipInstall) {
Expand Down
3 changes: 2 additions & 1 deletion packages/schematics/angular/library/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
"name": {
"type": "string",
"description": "The name of the library.",
"pattern": "^(?:@[a-zA-Z0-9-*~][a-zA-Z0-9-*._~]*/)?[a-zA-Z0-9-~][a-zA-Z0-9-._~]*$",
"$default": {
"$source": "argv",
"index": 0
Expand Down Expand Up @@ -45,5 +46,5 @@
"description": "Do not update \"tsconfig.json\" to add a path mapping for the new library. The path mapping is needed to use the library in an app, but can be disabled here to simplify development."
}
},
"required": []
"required": ["name"]
}
11 changes: 2 additions & 9 deletions packages/schematics/angular/ng-new/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
import {
Rule,
SchematicContext,
SchematicsException,
Tree,
apply,
chain,
Expand All @@ -25,19 +24,13 @@ import {
RepositoryInitializerTask,
} from '@angular-devkit/schematics/tasks';
import { Schema as ApplicationOptions } from '../application/schema';
import { validateProjectName } from '../utility/validation';
import { Schema as WorkspaceOptions } from '../workspace/schema';
import { Schema as NgNewOptions } from './schema';

export default function (options: NgNewOptions): Rule {
if (!options.name) {
throw new SchematicsException(`Invalid options, "name" is required.`);
}

validateProjectName(options.name);

if (!options.directory) {
options.directory = options.name;
// If scoped project (i.e. "@foo/bar"), convert directory to "foo/bar".
options.directory = options.name.startsWith('@') ? options.name.substr(1) : options.name;
}

const workspaceOptions: WorkspaceOptions = {
Expand Down
1 change: 0 additions & 1 deletion packages/schematics/angular/ng-new/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
"name": {
"description": "The name of the new workspace and initial project.",
"type": "string",
"format": "html-selector",
"$default": {
"$source": "argv",
"index": 0
Expand Down
57 changes: 0 additions & 57 deletions packages/schematics/angular/utility/validation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,6 @@
import { tags } from '@angular-devkit/core';
import { SchematicsException } from '@angular-devkit/schematics';

export function validateName(name: string): void {
if (name && /^\d/.test(name)) {
throw new SchematicsException(tags.oneLine`name (${name})
can not start with a digit.`);
}
}

// Must start with a letter, and must contain only alphanumeric characters or dashes.
// When adding a dash the segment after the dash must also start with a letter.
export const htmlSelectorRe = /^[a-zA-Z][.0-9a-zA-Z]*(:?-[a-zA-Z][.0-9a-zA-Z]*)*$/;
Expand All @@ -26,53 +19,3 @@ export function validateHtmlSelector(selector: string): void {
is invalid.`);
}
}

export function validateProjectName(projectName: string) {
const errorIndex = getRegExpFailPosition(projectName);
const unsupportedProjectNames: string[] = [];
const packageNameRegex = /^(?:@[a-zA-Z0-9_-]+\/)?[a-zA-Z0-9_-]+$/;
if (errorIndex !== null) {
const firstMessage = tags.oneLine`
Project name "${projectName}" is not valid. New project names must
start with a letter, and must contain only alphanumeric characters or dashes.
When adding a dash the segment after the dash must also start with a letter.
`;
const msg = tags.stripIndent`
${firstMessage}
${projectName}
${Array(errorIndex + 1).join(' ') + '^'}
`;
throw new SchematicsException(msg);
} else if (unsupportedProjectNames.indexOf(projectName) !== -1) {
throw new SchematicsException(
`Project name ${JSON.stringify(projectName)} is not a supported name.`,
);
} else if (!packageNameRegex.test(projectName)) {
throw new SchematicsException(`Project name ${JSON.stringify(projectName)} is invalid.`);
}
}

function getRegExpFailPosition(str: string): number | null {
const isScope = /^@.*\/.*/.test(str);
if (isScope) {
// Remove starting @
str = str.replace(/^@/, '');
// Change / to - for validation
str = str.replace(/\//g, '-');
}

const parts = str.indexOf('-') >= 0 ? str.split('-') : [str];
const matched: string[] = [];

const projectNameRegexp = /^[a-zA-Z][.0-9a-zA-Z]*(-[.0-9a-zA-Z]*)*$/;

parts.forEach((part) => {
if (part.match(projectNameRegexp)) {
matched.push(part);
}
});

const compare = matched.join('-');

return str !== compare ? compare.length : null;
}

0 comments on commit 21809e1

Please sign in to comment.