Skip to content

Commit 937e050

Browse files
author
Fatme
authored
Merge pull request #4124 from NativeScript/tachev/app-creation-github
fix: fix project creation with github url passed as template
2 parents 5ac9442 + b2afcf7 commit 937e050

8 files changed

+274
-55
lines changed

‎lib/declarations.d.ts

+27
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,27 @@ interface INodePackageManager {
2525
*/
2626
view(packageName: string, config: Object): Promise<any>;
2727

28+
/**
29+
* Checks if the specified string is name of a packaged published in the NPM registry.
30+
* @param {string} packageName The string to be checked.
31+
* @return {Promise<boolean>} True if the specified string is a registered package name, false otherwise.
32+
*/
33+
isRegistered(packageName: string): Promise<boolean>;
34+
35+
/**
36+
* Separates the package name and version from a specified fullPackageName.
37+
* @param {string} fullPackageName The full name of the package like [email protected].
38+
* @return {INpmPackageNameParts} An object containing the separated package name and version.
39+
*/
40+
getPackageNameParts(fullPackageName: string): INpmPackageNameParts
41+
42+
/**
43+
* Returns the full name of an npm package based on the provided name and version.
44+
* @param {INpmPackageNameParts} packageNameParts An object containing the package name and version.
45+
* @return {string} The full name of the package like [email protected].
46+
*/
47+
getPackageFullName(packageNameParts: INpmPackageNameParts): string
48+
2849
/**
2950
* Searches for a package.
3051
* @param {string[]} filter Keywords with which to perform the search.
@@ -59,6 +80,7 @@ interface INpmInstallationManager {
5980
getLatestVersion(packageName: string): Promise<string>;
6081
getNextVersion(packageName: string): Promise<string>;
6182
getLatestCompatibleVersion(packageName: string, referenceVersion?: string): Promise<string>;
83+
getLatestCompatibleVersionSafe(packageName: string, referenceVersion?: string): Promise<string>;
6284
getInspectorFromCache(inspectorNpmPackageName: string, projectDir: string): Promise<string>;
6385
}
6486

@@ -353,6 +375,11 @@ interface INpmsPackageData {
353375
maintainers: INpmsUser[];
354376
}
355377

378+
interface INpmPackageNameParts {
379+
name: string;
380+
version: string;
381+
}
382+
356383
interface IUsername {
357384
username: string;
358385
}

‎lib/node-package-manager.ts

+53-3
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,46 @@ export class NodePackageManager implements INodePackageManager {
112112
return JSON.parse(viewResult);
113113
}
114114

115+
public async isRegistered(packageName: string): Promise<boolean> {
116+
if (this.isURL(packageName) || this.$fs.exists(packageName) || this.isTgz(packageName)) {
117+
return false;
118+
}
119+
120+
try {
121+
const viewResult = await this.view(packageName, { name: true });
122+
123+
// `npm view nonExistingPackageName` will return `nativescript`
124+
// if executed in the root dir of the CLI (npm 6.4.1)
125+
const packageNameRegex = new RegExp(packageName, "i");
126+
const isProperResult = packageNameRegex.test(viewResult);
127+
128+
return isProperResult;
129+
} catch (e) {
130+
return false;
131+
}
132+
}
133+
134+
public getPackageNameParts(fullPackageName: string): INpmPackageNameParts {
135+
// support <reserved_name>@<version> syntax, for example [email protected]
136+
// support <scoped_package_name>@<version> syntax, for example @nativescript/[email protected]
137+
const lastIndexOfAtSign = fullPackageName.lastIndexOf("@");
138+
let version = "";
139+
let templateName = "";
140+
if (lastIndexOfAtSign > 0) {
141+
templateName = fullPackageName.substr(0, lastIndexOfAtSign).toLowerCase();
142+
version = fullPackageName.substr(lastIndexOfAtSign + 1);
143+
}
144+
145+
return {
146+
name: templateName || fullPackageName,
147+
version: version
148+
};
149+
}
150+
151+
public getPackageFullName(packageNameParts: INpmPackageNameParts): string {
152+
return packageNameParts.version ? `${packageNameParts.name}@${packageNameParts.version}` : packageNameParts.name;
153+
}
154+
115155
public async searchNpms(keyword: string): Promise<INpmsResult> {
116156
// TODO: Fix the generation of url - in case it contains @ or / , the call may fail.
117157
const httpRequestResult = await this.$httpClient.httpRequest(`https://api.npms.io/v2/search?q=keywords:${keyword}`);
@@ -136,6 +176,16 @@ export class NodePackageManager implements INodePackageManager {
136176
return path.join(cachePath.trim(), CACACHE_DIRECTORY_NAME);
137177
}
138178

179+
private isTgz(packageName: string): boolean {
180+
return packageName.indexOf(".tgz") >= 0;
181+
}
182+
183+
private isURL(str: string): boolean {
184+
const urlRegex = '^(?!mailto:)(?:(?:http|https|ftp)://)(?:\\S+(?::\\S*)?@)?(?:(?:(?:[1-9]\\d?|1\\d\\d|2[01]\\d|22[0-3])(?:\\.(?:1?\\d{1,2}|2[0-4]\\d|25[0-5])){2}(?:\\.(?:[0-9]\\d?|1\\d\\d|2[0-4]\\d|25[0-4]))|(?:(?:[a-z\\u00a1-\\uffff0-9]+-?)*[a-z\\u00a1-\\uffff0-9]+)(?:\\.(?:[a-z\\u00a1-\\uffff0-9]+-?)*[a-z\\u00a1-\\uffff0-9]+)*(?:\\.(?:[a-z\\u00a1-\\uffff]{2,})))|localhost)(?::\\d{2,5})?(?:(/|\\?|#)[^\\s]*)?$';
185+
const url = new RegExp(urlRegex, 'i');
186+
return str.length < 2083 && url.test(str);
187+
}
188+
139189
private getNpmExecutableName(): string {
140190
let npmExecutableName = "npm";
141191

@@ -153,7 +203,7 @@ export class NodePackageManager implements INodePackageManager {
153203
array.push(`--${flag}`);
154204
array.push(`${config[flag]}`);
155205
} else if (config[flag]) {
156-
if (flag === "dist-tags" || flag === "versions") {
206+
if (flag === "dist-tags" || flag === "versions" || flag === "name") {
157207
array.push(` ${flag}`);
158208
continue;
159209
}
@@ -171,8 +221,8 @@ export class NodePackageManager implements INodePackageManager {
171221
// TODO: Add tests for this functionality
172222
try {
173223
const originalOutput: INpmInstallCLIResult | INpm5InstallCliResult = JSON.parse(npmDryRunInstallOutput);
174-
const npm5Output = <INpm5InstallCliResult> originalOutput;
175-
const npmOutput = <INpmInstallCLIResult> originalOutput;
224+
const npm5Output = <INpm5InstallCliResult>originalOutput;
225+
const npmOutput = <INpmInstallCLIResult>originalOutput;
176226
let name: string;
177227
_.forOwn(npmOutput.dependencies, (peerDependency: INpmPeerDependencyInfo, key: string) => {
178228
if (!peerDependency.required && !peerDependency.peerMissing) {

‎lib/npm-installation-manager.ts

+13-17
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,16 @@ export class NpmInstallationManager implements INpmInstallationManager {
3838
return maxSatisfying || latestVersion;
3939
}
4040

41+
public async getLatestCompatibleVersionSafe(packageName: string, referenceVersion?: string): Promise<string> {
42+
let version = "";
43+
const canGetVersionFromNpm = await this.$npm.isRegistered(packageName);
44+
if (canGetVersionFromNpm) {
45+
version = await this.getLatestCompatibleVersion(packageName, referenceVersion);
46+
}
47+
48+
return version;
49+
}
50+
4151
public async install(packageToInstall: string, projectDir: string, opts?: INpmInstallOptions): Promise<any> {
4252
try {
4353
const pathToSave = projectDir;
@@ -100,6 +110,7 @@ export class NpmInstallationManager implements INpmInstallationManager {
100110
if (this.$fs.exists(pathToInspector)) {
101111
return true;
102112
}
113+
103114
return false;
104115
}
105116

@@ -109,28 +120,13 @@ export class NpmInstallationManager implements INpmInstallationManager {
109120
packageName = possiblePackageName;
110121
}
111122

112-
// check if the packageName is url or local file and if it is, let npm install deal with the version
113-
if (this.isURL(packageName) || this.$fs.exists(packageName) || this.isTgz(packageName)) {
114-
version = null;
115-
} else {
116-
version = version || await this.getLatestCompatibleVersion(packageName);
117-
}
118-
123+
version = version || await this.getLatestCompatibleVersionSafe(packageName);
119124
const installResultInfo = await this.npmInstall(packageName, pathToSave, version, dependencyType);
120125
const installedPackageName = installResultInfo.name;
121126

122127
const pathToInstalledPackage = path.join(pathToSave, "node_modules", installedPackageName);
123-
return pathToInstalledPackage;
124-
}
125128

126-
private isTgz(packageName: string): boolean {
127-
return packageName.indexOf(".tgz") >= 0;
128-
}
129-
130-
private isURL(str: string): boolean {
131-
const urlRegex = '^(?!mailto:)(?:(?:http|https|ftp)://)(?:\\S+(?::\\S*)?@)?(?:(?:(?:[1-9]\\d?|1\\d\\d|2[01]\\d|22[0-3])(?:\\.(?:1?\\d{1,2}|2[0-4]\\d|25[0-5])){2}(?:\\.(?:[0-9]\\d?|1\\d\\d|2[0-4]\\d|25[0-4]))|(?:(?:[a-z\\u00a1-\\uffff0-9]+-?)*[a-z\\u00a1-\\uffff0-9]+)(?:\\.(?:[a-z\\u00a1-\\uffff0-9]+-?)*[a-z\\u00a1-\\uffff0-9]+)*(?:\\.(?:[a-z\\u00a1-\\uffff]{2,})))|localhost)(?::\\d{2,5})?(?:(/|\\?|#)[^\\s]*)?$';
132-
const url = new RegExp(urlRegex, 'i');
133-
return str.length < 2083 && url.test(str);
129+
return pathToInstalledPackage;
134130
}
135131

136132
private async npmInstall(packageName: string, pathToSave: string, version: string, dependencyType: string): Promise<INpmInstallResultInfo> {

‎lib/services/project-templates-service.ts

+13-22
Original file line numberDiff line numberDiff line change
@@ -12,42 +12,33 @@ export class ProjectTemplatesService implements IProjectTemplatesService {
1212
private $logger: ILogger,
1313
private $npmInstallationManager: INpmInstallationManager,
1414
private $pacoteService: IPacoteService,
15-
private $errors: IErrors) { }
15+
private $errors: IErrors,
16+
private $npm: INodePackageManager) { }
1617

17-
public async prepareTemplate(originalTemplateName: string, projectDir: string): Promise<ITemplateData> {
18-
if (!originalTemplateName) {
19-
originalTemplateName = constants.RESERVED_TEMPLATE_NAMES["default"];
18+
public async prepareTemplate(templateValue: string, projectDir: string): Promise<ITemplateData> {
19+
if (!templateValue) {
20+
templateValue = constants.RESERVED_TEMPLATE_NAMES["default"];
2021
}
2122

22-
// support <reserved_name>@<version> syntax, for example [email protected]
23-
// support <scoped_package_name>@<version> syntax, for example @nativescript/[email protected]
24-
const lastIndexOfAtSign = originalTemplateName.lastIndexOf("@");
25-
let name = originalTemplateName;
26-
let version = "";
27-
if (lastIndexOfAtSign > 0) {
28-
name = originalTemplateName.substr(0, lastIndexOfAtSign);
29-
version = originalTemplateName.substr(lastIndexOfAtSign + 1);
30-
}
23+
const templateNameParts = this.$npm.getPackageNameParts(templateValue);
24+
templateValue = constants.RESERVED_TEMPLATE_NAMES[templateNameParts.name] || templateNameParts.name;
3125

32-
const templateName = constants.RESERVED_TEMPLATE_NAMES[name.toLowerCase()] || name;
33-
if (!this.$fs.exists(templateName)) {
34-
version = version || await this.$npmInstallationManager.getLatestCompatibleVersion(templateName);
35-
}
26+
const version = templateNameParts.version || await this.$npmInstallationManager.getLatestCompatibleVersionSafe(templateValue);
27+
const fullTemplateName = this.$npm.getPackageFullName({ name: templateValue, version: version });
3628

37-
const fullTemplateName = version ? `${templateName}@${version}` : templateName;
3829
const templatePackageJsonContent = await this.getTemplatePackageJsonContent(fullTemplateName);
3930
const templateVersion = await this.getTemplateVersion(fullTemplateName);
4031

4132
let templatePath = null;
4233
if (templateVersion === constants.TemplateVersions.v1) {
43-
templatePath = await this.prepareNativeScriptTemplate(templateName, version, projectDir);
34+
templatePath = await this.prepareNativeScriptTemplate(templateValue, version, projectDir);
4435
// this removes dependencies from templates so they are not copied to app folder
4536
this.$fs.deleteDirectory(path.join(templatePath, constants.NODE_MODULES_FOLDER_NAME));
4637
}
4738

48-
await this.$analyticsService.track("Template used for project creation", templateName);
39+
await this.$analyticsService.track("Template used for project creation", templateValue);
4940

50-
const templateNameToBeTracked = this.getTemplateNameToBeTracked(templateName, templatePackageJsonContent);
41+
const templateNameToBeTracked = this.getTemplateNameToBeTracked(templateValue, templatePackageJsonContent);
5142
if (templateNameToBeTracked) {
5243
await this.$analyticsService.trackEventActionInGoogleAnalytics({
5344
action: constants.TrackActionNames.CreateProject,
@@ -61,7 +52,7 @@ export class ProjectTemplatesService implements IProjectTemplatesService {
6152
});
6253
}
6354

64-
return { templateName, templatePath, templateVersion, templatePackageJsonContent, version };
55+
return { templateName: templateValue, templatePath, templateVersion, templatePackageJsonContent, version };
6556
}
6657

6758
private async getTemplateVersion(templateName: string): Promise<string> {

‎test/node-package-manager.ts

+88
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
import { Yok } from "../lib/common/yok";
2+
import * as stubs from "./stubs";
3+
import { assert } from "chai";
4+
import { NodePackageManager } from "../lib/node-package-manager";
5+
6+
function createTestInjector(configuration: {
7+
} = {}): IInjector {
8+
const injector = new Yok();
9+
injector.register("hostInfo", {});
10+
injector.register("errors", stubs.ErrorsStub);
11+
injector.register("logger", stubs.LoggerStub);
12+
injector.register("childProcess", stubs.ChildProcessStub);
13+
injector.register("httpClient", {});
14+
injector.register("fs", stubs.FileSystemStub);
15+
injector.register("npm", NodePackageManager);
16+
17+
return injector;
18+
}
19+
20+
describe("node-package-manager", () => {
21+
22+
describe("getPackageNameParts", () => {
23+
[
24+
{
25+
name: "should return both name and version when valid fullName passed",
26+
templateFullName: "[email protected]",
27+
expectedVersion: "1.0.0",
28+
expectedName: "some-template",
29+
},
30+
{
31+
name: "should return both name and version when valid fullName with scope passed",
32+
templateFullName: "@nativescript/[email protected]",
33+
expectedVersion: "1.0.0",
34+
expectedName: "@nativescript/some-template",
35+
},
36+
{
37+
name: "should return only name when version is not specified and the template is scoped",
38+
templateFullName: "@nativescript/some-template",
39+
expectedVersion: "",
40+
expectedName: "@nativescript/some-template",
41+
},
42+
{
43+
name: "should return only name when version is not specified",
44+
templateFullName: "some-template",
45+
expectedVersion: "",
46+
expectedName: "some-template",
47+
}
48+
].forEach(testCase => {
49+
it(testCase.name, async () => {
50+
const testInjector = createTestInjector();
51+
const npm = testInjector.resolve<NodePackageManager>("npm");
52+
const templateNameParts = await npm.getPackageNameParts(testCase.templateFullName);
53+
assert.strictEqual(templateNameParts.name, testCase.expectedName);
54+
assert.strictEqual(templateNameParts.version, testCase.expectedVersion);
55+
});
56+
});
57+
});
58+
59+
describe("getPackageFullName", () => {
60+
[
61+
{
62+
name: "should return name and version when specified",
63+
templateName: "some-template",
64+
templateVersion: "1.0.0",
65+
expectedFullName: "[email protected]",
66+
},
67+
{
68+
name: "should return only the github url when no version specified",
69+
templateName: "https://github.com/NativeScript/template-drawer-navigation-ng#master",
70+
templateVersion: "",
71+
expectedFullName: "https://github.com/NativeScript/template-drawer-navigation-ng#master",
72+
},
73+
{
74+
name: "should return only the name when no version specified",
75+
templateName: "some-template",
76+
templateVersion: "",
77+
expectedFullName: "some-template",
78+
}
79+
].forEach(testCase => {
80+
it(testCase.name, async () => {
81+
const testInjector = createTestInjector();
82+
const npm = testInjector.resolve<NodePackageManager>("npm");
83+
const templateFullName = await npm.getPackageFullName({ name: testCase.templateName, version: testCase.templateVersion });
84+
assert.strictEqual(templateFullName, testCase.expectedFullName);
85+
});
86+
});
87+
});
88+
});

0 commit comments

Comments
 (0)