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.

Sign up for GitHub

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

[Telemetry] Address package name limitations #14252

Closed
wants to merge 4 commits into from
Closed

[Telemetry] Address package name limitations #14252

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
949ad2a
Moved nameHelpers to telemetry, updated references
danielayala94 Jan 7, 2025
a9f393f
Convert the package name
danielayala94 Jan 7, 2025
c4f0842
Added test cases
danielayala94 Jan 7, 2025
13d8eaa
Lint fixes blah blah
danielayala94 Jan 7, 2025
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
2 changes: 1 addition & 1 deletion packages/@react-native-windows/cli/src/commands/initWindows/initWindows.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import {
endTelemetrySession,
} from '../../utils/telemetryHelpers';
import {copyAndReplaceWithChangedCallback} from '../../generator-common';
import * as nameHelpers from '../../utils/nameHelpers';
import * as nameHelpers from '@react-native-windows/telemetry/src/utils/nameHelpers';
Copy link
Contributor

Choose a reason for hiding this comment

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

Look at telemetry/index.ts to see how to export the functions we need there (isValidProjectNamespace, isValidProjectName, cleanName, cleanNamespace). Then you can import those functions here with:

Suggested change
import * as nameHelpers from '@react-native-windows/telemetry/src/utils/nameHelpers';
import {isValidProjectNamespace, isValidProjectName, cleanName, cleanNamespace} from '@react-native-windows/telemetry';

import {InitOptions, initOptions} from './initWindowsOptions';

export interface TemplateFileMapping {
Expand Down
70 changes: 1 addition & 69 deletions packages/@react-native-windows/cli/src/e2etest/initWindows.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ import {
InitOptions,
} from '../commands/initWindows/initWindowsOptions';

import * as nameHelpers from '../utils/nameHelpers';

function validateOptionName(
name: string,
optionName: keyof InitOptions,
Expand Down Expand Up @@ -59,70 +57,4 @@ test('initOptions - validate options', () => {
validateOptionName(commandOption.name, optionName as keyof InitOptions),
).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);
});
});
2 changes: 1 addition & 1 deletion packages/@react-native-windows/cli/src/generator-windows/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import {
findPropertyValue,
tryFindPropertyValueAsBoolean,
} from '../commands/config/configUtils';
import * as nameHelpers from '../utils/nameHelpers';
import * as nameHelpers from '@react-native-windows/telemetry/src/utils/nameHelpers';
Copy link
Contributor

Choose a reason for hiding this comment

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

See the above comment for properly exporting the functions you need and consuming them here.


import {
createDir,
Expand Down
12 changes: 8 additions & 4 deletions packages/@react-native-windows/cli/tsconfig.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,14 @@
"DOM.Iterable",
"ES2019.String"
],

// Not clean
"strictFunctionTypes": false,
},
"include": ["src"],
"exclude": ["node_modules"]
}
"include": [
"src",
"../telemetry/src/utils/nameHelpers.ts"
Copy link
Contributor

Choose a reason for hiding this comment

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

You don't want to cross include packages, like this, don't include this here. You won't need it if you properly export the nameHelpers functions from the telemetry package, and remember to re-build the telemetry package so you can see them in this package.

],
"exclude": [
"node_modules"
]
}
8 changes: 6 additions & 2 deletions packages/@react-native-windows/telemetry/src/telemetry.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import * as basePropUtils from './utils/basePropUtils';
import * as versionUtils from './utils/versionUtils';
import * as errorUtils from './utils/errorUtils';
import * as projectUtils from './utils/projectUtils';
import * as nameHelpers from './utils/nameHelpers';

export interface TelemetryOptions {
setupString: string;
Expand Down Expand Up @@ -246,10 +247,13 @@ export class Telemetry {
return true;
}

if (forceRefresh === true || !Telemetry.versionsProp[name]) {
// Convert the package name to comply with the backend requirements
const packageName = nameHelpers.isValidTelemetryPackageName(name) ? name : nameHelpers.cleanTelemetryPackageName(name);

if (forceRefresh === true || !Telemetry.versionsProp[packageName]) {
const value = await getValue();
if (value) {
Telemetry.versionsProp[name] = value;
Telemetry.versionsProp[packageName] = value;
return true;
}
}
Expand Down
91 changes: 91 additions & 0 deletions packages/@react-native-windows/telemetry/src/test/nameHelpers.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
/**
* Copyright (c) Microsoft Corporation.
* Licensed under the MIT License.
*
* @format
*/

import * as nameHelpers from '../utils/nameHelpers';

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

test('Verify telemetry package name is valid', () => {
expect(nameHelpers.isValidTelemetryPackageName('package')).toBe(true);
expect(nameHelpers.isValidTelemetryPackageName('@react')).toBe(false);
expect(nameHelpers.isValidTelemetryPackageName('react-native')).toBe(false);
expect(nameHelpers.isValidTelemetryPackageName('react_native')).toBe(true);
expect(nameHelpers.isValidTelemetryPackageName('react_native/cli')).toBe(false);
});

test('Verify telemetry package name cleaning', () => {
expect(nameHelpers.cleanTelemetryPackageName('package')).toBe('package');
expect(nameHelpers.cleanTelemetryPackageName('@react')).toBe('_react');
expect(nameHelpers.cleanTelemetryPackageName('react-native')).toBe('react_native');
expect(nameHelpers.cleanTelemetryPackageName('react_native')).toBe('react_native');
expect(nameHelpers.cleanTelemetryPackageName('react_native/cli')).toBe('react_native_cli');
expect(nameHelpers.cleanTelemetryPackageName('@react-native-windows/cli')).toBe('_react_native_windows_cli');
});
13 changes: 13 additions & 0 deletions ...tive-windows/cli/src/utils/nameHelpers.ts → ...indows/telemetry/src/utils/nameHelpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,16 @@ export function isValidProjectNamespace(namespace: string): boolean {
export function cleanNamespace(str: string): string {
return str.split(/[.:]+/).map(cleanName).join('.');
}

export function isValidTelemetryPackageName(name: string): boolean {
// Accepted characters: alphanumeric, underscore, dot, starts with letter.
// Size: 1-100 characters.
if (name.match(/^[a-zA-Z][a-zA-Z0-9_.]{0,99}$/gi)) {
return true;
}
return false;
}

export function cleanTelemetryPackageName(str: string): string {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not that it affects any of the packages, but there is no check to trim the package name if it's over 100 characters,

Copy link
Contributor

Choose a reason for hiding this comment

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

Might be worth putting it in though, if it's an actual requirement.

return str.replace(/[^a-zA-Z0-9_.]/g, '_');
}
Loading