From 849d71f68f0cc542804534769998e9f4fb34fe99 Mon Sep 17 00:00:00 2001 From: Jon Thysell Date: Mon, 26 Aug 2024 11:43:04 -0700 Subject: [PATCH] Improve new project name(space) validation and cleaning (#13566) ## Description This PR moves all of the logic concerning validating and cleaning project names when creating new projects (via the `init-windows` CLI command or the soon to be deprecated `react-native-windows-init` command) into one shared location with unit tests. It also updates the cleaning logic to handle package scopes (i.e. `@org/package`) and better apply cleaning to packages with hyphens (i.e. `my-package`). Going forward, when creating (new or old arch) projects with `init-windows`, the flow will still to adhere the following rules: 1. Verify that a string specified with `--name` or `--namespace` is valid, or else error out. 2. If an item isn't specified, do our best to determine the value from `package.json`, etc., and clean that value if necessary. When using `react-native-windows-init`, (which still only lets you specify the `--namespace`, and always figures out name from `package.json`, etc.), the flow will be mostly the same as before, where both name and namespace will be cleaned automatically if invalid, rather than erroring out. (However the consolidated cleaning logic should mean an improvement when using tools other than the RN CLI for creating your initial RN project). ### Type of Change - Bug fix (non-breaking change which fixes an issue) ### Why The rules for what constitutes a "valid" name for a RN project has changed over the years with each of the different tools that are used to create RN projects. This PR is an attempt to both broaden the number of supported new project tools while also ensuring RNW still produces usable native code. Closes #13558 Closes #13426 ### What Moved all name(space) logic into a new `nameHelpers` module, exposed them to existing callers, and created unit tests. ## Screenshots N/A ## Testing Add new tests for nameHelpers. ## Changelog Should this change be included in the release notes: _yes_ Improve new project name(space) validation and cleaning --- ...-db04fef2-0cc5-477b-ac9a-1aa9f4e4c2ee.json | 7 +++ ...-ca4bf582-eec5-4ee4-8830-19a6d937a87b.json | 7 +++ .../src/commands/initWindows/initWindows.ts | 55 ++++++++++++------ .../cli/src/e2etest/initWindows.test.ts | 56 +++++++++++++++++++ .../cli/src/generator-windows/index.ts | 18 +++--- .../cli/src/utils/nameHelpers.ts | 38 +++++++++++++ .../telemetry/src/utils/errorUtils.ts | 1 + 7 files changed, 155 insertions(+), 27 deletions(-) create mode 100644 change/@react-native-windows-cli-db04fef2-0cc5-477b-ac9a-1aa9f4e4c2ee.json create mode 100644 change/@react-native-windows-telemetry-ca4bf582-eec5-4ee4-8830-19a6d937a87b.json create mode 100644 packages/@react-native-windows/cli/src/utils/nameHelpers.ts diff --git a/change/@react-native-windows-cli-db04fef2-0cc5-477b-ac9a-1aa9f4e4c2ee.json b/change/@react-native-windows-cli-db04fef2-0cc5-477b-ac9a-1aa9f4e4c2ee.json new file mode 100644 index 00000000000..c71a5b75a1c --- /dev/null +++ b/change/@react-native-windows-cli-db04fef2-0cc5-477b-ac9a-1aa9f4e4c2ee.json @@ -0,0 +1,7 @@ +{ + "type": "prerelease", + "comment": "Improve new project name(space) validation and cleaning", + "packageName": "@react-native-windows/cli", + "email": "jthysell@microsoft.com", + "dependentChangeType": "patch" +} diff --git a/change/@react-native-windows-telemetry-ca4bf582-eec5-4ee4-8830-19a6d937a87b.json b/change/@react-native-windows-telemetry-ca4bf582-eec5-4ee4-8830-19a6d937a87b.json new file mode 100644 index 00000000000..8355df4dccd --- /dev/null +++ b/change/@react-native-windows-telemetry-ca4bf582-eec5-4ee4-8830-19a6d937a87b.json @@ -0,0 +1,7 @@ +{ + "type": "prerelease", + "comment": "Improve new project name(space) validation and cleaning", + "packageName": "@react-native-windows/telemetry", + "email": "jthysell@microsoft.com", + "dependentChangeType": "patch" +} diff --git a/packages/@react-native-windows/cli/src/commands/initWindows/initWindows.ts b/packages/@react-native-windows/cli/src/commands/initWindows/initWindows.ts index ba64dabfb5f..7f25a8f0123 100644 --- a/packages/@react-native-windows/cli/src/commands/initWindows/initWindows.ts +++ b/packages/@react-native-windows/cli/src/commands/initWindows/initWindows.ts @@ -29,6 +29,7 @@ import { endTelemetrySession, } from '../../utils/telemetryHelpers'; import {copyAndReplaceWithChangedCallback} from '../../generator-common'; +import * as nameHelpers from '../../utils/nameHelpers'; import {InitOptions, initOptions} from './initWindowsOptions'; export interface TemplateFileMapping { @@ -94,18 +95,6 @@ export class InitWindows { ); } - protected pascalCase(str: string): string { - const camelCase = _.camelCase(str); - return camelCase[0].toUpperCase() + camelCase.substr(1); - } - - protected isValidProjectName(name: string): boolean { - if (name.match(/^[a-z][a-z0-9]*$/gi)) { - return true; - } - return false; - } - protected getReactNativeProjectName(projectDir: string): string { this.verboseMessage('Looking for project name in package.json...'); const pkgJsonPath = path.join(projectDir, 'package.json'); @@ -152,21 +141,53 @@ export class InitWindows { } const templateConfig = this.templates.get(this.options.template)!; - if (this.options.name && !this.isValidProjectName(this.options.name)) { + // Check if there's a passed-in project name and if it's valid + if (this.options.name && !nameHelpers.isValidProjectName(this.options.name)) { throw new CodedError( 'InvalidProjectName', - `The specified name is not a valid identifier`, + `The specified name '${this.options.name}' is not a valid identifier`, ); } + // If no project name is provided, calculate the name and clean if necessary if (!this.options.name) { const projectName = this.getReactNativeProjectName(this.config.root); - this.options.name = this.isValidProjectName(projectName) + this.options.name = nameHelpers.isValidProjectName(projectName) ? projectName - : this.pascalCase(projectName); + : nameHelpers.cleanName(projectName); + } + + // Final check that the project name is valid + if (!nameHelpers.isValidProjectName(this.options.name)) { + throw new CodedError( + 'InvalidProjectName', + `The name '${this.options.name}' is not a valid identifier`, + ); } - this.options.namespace ??= this.options.name; + // Check if there's a passed-in project namespace and if it's valid + if (this.options.namespace && !nameHelpers.isValidProjectNamespace(this.options.namespace)) { + throw new CodedError( + 'InvalidProjectNamespace', + `The specified namespace '${this.options.namespace}' is not a valid identifier`, + ); + } + + // If no project namespace is provided, use the project name and clean if necessary + if (!this.options.namespace) { + const namespace = this.options.name; + this.options.namespace = nameHelpers.isValidProjectNamespace(namespace) + ? namespace + : nameHelpers.cleanNamespace(namespace); + } + + // Final check that the project namespace is valid + if (!nameHelpers.isValidProjectNamespace(this.options.namespace)) { + throw new CodedError( + 'InvalidProjectNamespace', + `The namespace '${this.options.namespace}' is not a valid identifier`, + ); + } if (templateConfig.preInstall) { spinner.info(`Running ${this.options.template} preInstall()...`); diff --git a/packages/@react-native-windows/cli/src/e2etest/initWindows.test.ts b/packages/@react-native-windows/cli/src/e2etest/initWindows.test.ts index 89d6cb7026b..e69d8bd25fd 100644 --- a/packages/@react-native-windows/cli/src/e2etest/initWindows.test.ts +++ b/packages/@react-native-windows/cli/src/e2etest/initWindows.test.ts @@ -11,6 +11,8 @@ import { InitOptions, } from '../commands/initWindows/initWindowsOptions'; +import * as nameHelpers from '../utils/nameHelpers'; + function validateOptionName( name: string, optionName: keyof InitOptions, @@ -57,3 +59,57 @@ test('initOptions - validate options', () => { ).toBe(true); } }); + +test('nameHelpers - cleanName', () => { + expect(nameHelpers.cleanName('@scope/package')).toBe('Package'); + expect(nameHelpers.cleanName('@scope/package-name')).toBe('PackageName'); + expect(nameHelpers.cleanName('package')).toBe('Package'); + expect(nameHelpers.cleanName('package-name')).toBe('PackageName'); +}); + +test('nameHelpers - isValidProjectName', () => { + expect(nameHelpers.isValidProjectName('package')).toBe(true); + expect(nameHelpers.isValidProjectName('package-name')).toBe(false); + expect(nameHelpers.isValidProjectName('Package')).toBe(true); + expect(nameHelpers.isValidProjectName('Package-name')).toBe(false); + expect(nameHelpers.isValidProjectName('Package-Name')).toBe(false); + expect(nameHelpers.isValidProjectName('@scope/package')).toBe(false); + expect(nameHelpers.isValidProjectName('@scope/package-name')).toBe(false); +}); + +test('nameHelpers - cleanNamespace', () => { + expect(nameHelpers.cleanNamespace('@scope/package')).toBe('Package'); + expect(nameHelpers.cleanNamespace('@scope/package-name')).toBe('PackageName'); + expect(nameHelpers.cleanNamespace('package')).toBe('Package'); + expect(nameHelpers.cleanNamespace('package-name')).toBe('PackageName'); + expect(nameHelpers.cleanNamespace('com.company.app')).toBe('Com.Company.App'); + expect(nameHelpers.cleanNamespace('com.company.app-name')).toBe( + 'Com.Company.AppName', + ); + expect(nameHelpers.cleanNamespace('com.company.app-name.other')).toBe( + 'Com.Company.AppName.Other', + ); + expect(nameHelpers.cleanNamespace('com::company::app')).toBe('Com.Company.App'); + expect(nameHelpers.cleanNamespace('com::company::app-name')).toBe( + 'Com.Company.AppName', + ); + expect(nameHelpers.cleanNamespace('com::company::app-name::other')).toBe( + 'Com.Company.AppName.Other', + ); +}); + +test('nameHelpers - isValidProjectNamespace', () => { + expect(nameHelpers.isValidProjectNamespace('package')).toBe(true); + expect(nameHelpers.isValidProjectNamespace('package-name')).toBe(false); + expect(nameHelpers.isValidProjectNamespace('Package')).toBe(true); + expect(nameHelpers.isValidProjectNamespace('Package-name')).toBe(false); + expect(nameHelpers.isValidProjectNamespace('Package-Name')).toBe(false); + expect(nameHelpers.isValidProjectNamespace('@scope/package')).toBe(false); + expect(nameHelpers.isValidProjectNamespace('@scope/package-name')).toBe(false); + expect(nameHelpers.isValidProjectNamespace('com.company.app')).toBe(true); + expect(nameHelpers.isValidProjectNamespace('com.company.app-name')).toBe(false); + expect(nameHelpers.isValidProjectNamespace('com.company.app-name.other')).toBe(false); + expect(nameHelpers.isValidProjectNamespace('com::company::app')).toBe(false); + expect(nameHelpers.isValidProjectNamespace('com::company::app-name')).toBe(false); + expect(nameHelpers.isValidProjectNamespace('com::company::app-name::other')).toBe(false); +}); \ No newline at end of file diff --git a/packages/@react-native-windows/cli/src/generator-windows/index.ts b/packages/@react-native-windows/cli/src/generator-windows/index.ts index 92c1b085c9a..814dee47523 100644 --- a/packages/@react-native-windows/cli/src/generator-windows/index.ts +++ b/packages/@react-native-windows/cli/src/generator-windows/index.ts @@ -18,6 +18,7 @@ import { findPropertyValue, tryFindPropertyValueAsBoolean, } from '../commands/config/configUtils'; +import * as nameHelpers from '../utils/nameHelpers'; import { createDir, @@ -46,11 +47,6 @@ interface NugetPackage { privateAssets: boolean; } -function pascalCase(str: string) { - const camelCase = _.camelCase(str); - return camelCase[0].toUpperCase() + camelCase.substr(1); -} - function resolveRnwPath(subpath: string): string { return require.resolve(path.join('react-native-windows', subpath), { paths: [process.cwd()], @@ -91,14 +87,16 @@ export async function copyProjectTemplateAndReplace( const language = options.language; // @react-native-community/cli init only allows alphanumerics in project names, but other - // new project tools (like create-react-native-module) are less strict. - if (projectType === 'lib') { - newProjectName = pascalCase(newProjectName); + // new project tools (like expo and create-react-native-module) are less strict. + // The default (legacy) behavior of this flow is to clean the name rather than throw an error. + if (!nameHelpers.isValidProjectName(newProjectName)) { + newProjectName = nameHelpers.cleanName(newProjectName); } // Similar to the above, but we want to retain namespace separators - if (projectType === 'lib') { - namespace = namespace.split(/[.:]+/).map(pascalCase).join('.'); + // The default (legacy) behavior of this flow is to clean the name rather than throw an error. + if (!nameHelpers.isValidProjectNamespace(namespace)) { + namespace = nameHelpers.cleanNamespace(namespace); } // Checking if we're overwriting an existing project and re-uses their projectGUID diff --git a/packages/@react-native-windows/cli/src/utils/nameHelpers.ts b/packages/@react-native-windows/cli/src/utils/nameHelpers.ts new file mode 100644 index 00000000000..78ffadacbe2 --- /dev/null +++ b/packages/@react-native-windows/cli/src/utils/nameHelpers.ts @@ -0,0 +1,38 @@ +/** + * Copyright (c) Microsoft Corporation. + * Licensed under the MIT License. + * @format + */ + +import _ from 'lodash'; + +function pascalCase(str: string): string { + const camelCase = _.camelCase(str); + return camelCase[0].toUpperCase() + camelCase.substr(1); +} + +export function isValidProjectName(name: string): boolean { + if (name.match(/^[a-z][a-z0-9]*$/gi)) { + return true; + } + return false; +} + +export function cleanName(str: string): string { + str = str.replace('@', ''); // Remove '@' from package scope names + str = str.slice(str.lastIndexOf('/') + 1); // Remove package scope + str = pascalCase(str); // Convert to PascalCase + return str; +} + +export function isValidProjectNamespace(namespace: string): boolean { + if (namespace.split(/[.]+/).map(isValidProjectName).every(x => x)) { + // Validate that every part of the namespace is a valid project name + return true; + } + return false; +} + +export function cleanNamespace(str: string): string { + return str.split(/[.:]+/).map(cleanName).join('.'); +} \ No newline at end of file diff --git a/packages/@react-native-windows/telemetry/src/utils/errorUtils.ts b/packages/@react-native-windows/telemetry/src/utils/errorUtils.ts index 48b50a14274..1c3400df77e 100644 --- a/packages/@react-native-windows/telemetry/src/utils/errorUtils.ts +++ b/packages/@react-native-windows/telemetry/src/utils/errorUtils.ts @@ -78,6 +78,7 @@ export const CodedErrors = { InvalidTemplateName: 5002, NoProjectName: 5003, InvalidProjectName: 5004, + InvalidProjectNamespace: 5005, }; export type CodedErrorType = keyof typeof CodedErrors;