From 069b3e7a8e458bb69a4ab1d9deb0e653de071ed3 Mon Sep 17 00:00:00 2001 From: rosen-vladimirov Date: Sun, 11 Feb 2018 20:21:18 +0200 Subject: [PATCH 1/2] feat(extensions): Allow generation of help content for extension commands Each CLI extension may add new commands to CLI. Allow showing command help for these commands in case: 1. Commmand execution fails. 2. Users need more information for command - `tns --help` 3. Users need full html help for command - `tns help `. Each extension that wants to use this functionality will have to add `nativescript` key in its `package.json` and add `docs` key in it. The `docs` key must point to the directory where the help content (.md files) is located, relative to the root directory of the extension. When CLI needs to show command line help for extension's command, it will search the docs directories of all extensions. When CLI needs to show HTML help for the extension's commands, it will generate `html` directory right next to the docs dir in the extension. Move extensibility.d.ts from {N} to mobile-cli-lib just to ensure correct transpilation. --- lib/common | 2 +- lib/definitions/extensibility.d.ts | 56 --------------- lib/services/extensibility-service.ts | 47 +++++++++++-- test/services/extensibility-service.ts | 96 ++++++++++++++------------ 4 files changed, 94 insertions(+), 107 deletions(-) delete mode 100644 lib/definitions/extensibility.d.ts diff --git a/lib/common b/lib/common index 484ed26d05..e39698131f 160000 --- a/lib/common +++ b/lib/common @@ -1 +1 @@ -Subproject commit 484ed26d05081a37195a909ac0a401c5281ff872 +Subproject commit e39698131f2469322273467ab29e0f9c09dbb8ed diff --git a/lib/definitions/extensibility.d.ts b/lib/definitions/extensibility.d.ts deleted file mode 100644 index 90d2916ef9..0000000000 --- a/lib/definitions/extensibility.d.ts +++ /dev/null @@ -1,56 +0,0 @@ -/** - * Describes each extension. - */ -interface IExtensionData { - /** - * The name of the extension. - */ - extensionName: string; -} - -/** - * Defines methods for working with CLI's extensions. - */ -interface IExtensibilityService { - /** - * Installs a specified extension. - * @param {string} extensionName Name of the extension to be installed. It may contain version as well, i.e. myPackage, myPackage@1.0.0, - * myPackage.tgz, https://github.com/myOrganization/myPackage/tarball/master, https://github.com/myOrganization/myPackage, etc. - * @returns {Promise} Information about installed extensions. - */ - installExtension(extensionName: string): Promise; - - /** - * Uninstalls extension from the installation. - * @param {string} extensionName Name of the extension to be uninstalled. - * @returns {Promise} - */ - uninstallExtension(extensionName: string): Promise; - - /** - * Loads all extensions, so their methods and commands can be used from CLI. - * For each of the extensions, a new Promise is returned. It will be rejected in case the extension cannot be loaded. However other promises will not be reflected by this failure. - * In case a promise is rejected, the error will have additional property (extensionName) that shows which is the extension that cannot be loaded in the process. - * @returns {Promise[]} Array of promises, each is resolved with information about loaded extension. - */ - loadExtensions(): Promise[]; - - /** - * Loads a single extension, so its methods and commands can be used from CLI. - * @param {string} extensionName Name of the extension to be installed. It may contain version as well, i.e. myPackage, myPackage@1.0.0 - * A Promise is returned. It will be rejected in case the extension cannot be loaded. - * @returns {Promise} Promise, resolved with IExtensionData. - */ - loadExtension(extensionName: string): Promise; - - /** - * Gets information about installed dependencies - names and versions. - * @returns {IStringDictionary} - */ - getInstalledExtensions(): IStringDictionary; -} - -/** - * Describes the error that will be raised when a problem with extension is detected. - */ -interface IExtensionLoadingError extends Error, IExtensionData { } \ No newline at end of file diff --git a/lib/services/extensibility-service.ts b/lib/services/extensibility-service.ts index c96df2117a..e1ca659996 100644 --- a/lib/services/extensibility-service.ts +++ b/lib/services/extensibility-service.ts @@ -35,7 +35,16 @@ export class ExtensibilityService implements IExtensibilityService { const installResultInfo = await this.$npm.install(packageName, this.pathToExtensions, npmOpts); this.$logger.trace(`Finished installation of extension '${extensionName}'. Trying to load it now.`); - return { extensionName: installResultInfo.name }; + const packageJsonData = this.getExtensionPackageJsonData(installResultInfo.name); + + const pathToExtension = this.getPathToExtension(extensionName); + const docs = packageJsonData && packageJsonData.nativescript && packageJsonData.nativescript.docs && path.join(pathToExtension, packageJsonData.nativescript.docs); + return { + extensionName: installResultInfo.name, + version: installResultInfo.version, + docs, + pathToExtension + }; } @exported("extensibilityService") @@ -49,8 +58,15 @@ export class ExtensibilityService implements IExtensibilityService { this.$logger.trace(`Finished uninstallation of extension '${extensionName}'.`); } + public getInstalledExtensionsData(): IExtensionData[] { + const installedExtensions = this.getInstalledExtensions(); + return _.keys(installedExtensions).map(installedExtension => { + return this.getInstalledExtensionData(installedExtension); + }); + } + @exported("extensibilityService") - public loadExtensions(): Promise[] { + public loadExtensions(): Promise[] { this.$logger.trace("Loading extensions."); let dependencies: IStringDictionary = null; @@ -74,14 +90,26 @@ export class ExtensibilityService implements IExtensibilityService { return null; } + private getInstalledExtensionData(extensionName: string): IExtensionData { + const packageJsonData = this.getExtensionPackageJsonData(extensionName); + const pathToExtension = this.getPathToExtension(extensionName); + const docs = packageJsonData && packageJsonData.nativescript && packageJsonData.nativescript.docs && path.join(pathToExtension, packageJsonData.nativescript.docs); + return { + extensionName: packageJsonData.name, + version: packageJsonData.version, + docs, + pathToExtension + }; + } + @exported("extensibilityService") public async loadExtension(extensionName: string): Promise { try { await this.assertExtensionIsInstalled(extensionName); - const pathToExtension = path.join(this.pathToExtensions, constants.NODE_MODULES_FOLDER_NAME, extensionName); + const pathToExtension = this.getPathToExtension(extensionName); this.$requireService.require(pathToExtension); - return { extensionName }; + return this.getInstalledExtensionData(extensionName); } catch (error) { this.$logger.warn(`Error while loading ${extensionName} is: ${error.message}`); const err = new Error(`Unable to load extension ${extensionName}. You will not be able to use the functionality that it adds. Error: ${error.message}`); @@ -90,6 +118,17 @@ export class ExtensibilityService implements IExtensibilityService { } } + private getPathToExtension(extensionName: string): string { + return path.join(this.pathToExtensions, constants.NODE_MODULES_FOLDER_NAME, extensionName); + } + + private getExtensionPackageJsonData(extensionName: string): any { + const pathToExtension = this.getPathToExtension(extensionName); + const pathToPackageJson = path.join(pathToExtension, constants.PACKAGE_JSON_FILE_NAME); + const jsonData = this.$fs.readJson(pathToPackageJson); + return jsonData; + } + private async assertExtensionIsInstalled(extensionName: string): Promise { this.$logger.trace(`Asserting extension ${extensionName} is installed.`); const installedExtensions = this.$fs.readDirectory(path.join(this.pathToExtensions, constants.NODE_MODULES_FOLDER_NAME)); diff --git a/test/services/extensibility-service.ts b/test/services/extensibility-service.ts index 87978b60c3..b9c880b1fd 100644 --- a/test/services/extensibility-service.ts +++ b/test/services/extensibility-service.ts @@ -18,7 +18,9 @@ describe("extensibilityService", () => { const getTestInjector = (): IInjector => { const testInjector = new Yok(); - testInjector.register("fs", {}); + testInjector.register("fs", { + readJson: (pathToFile: string): any => ({}) + }); testInjector.register("logger", stubs.LoggerStub); testInjector.register("npm", {}); testInjector.register("settingsService", SettingsService); @@ -28,6 +30,33 @@ describe("extensibilityService", () => { return testInjector; }; + const getExpectedInstallationPathForExtension = (testInjector: IInjector, extensionName: string): string => { + const settingsService = testInjector.resolve("settingsService"); + const profileDir = settingsService.getProfileDir(); + + return path.join(profileDir, "extensions", "node_modules", extensionName); + }; + + const mockFsReadJson = (testInjector: IInjector, extensionNames: string[]): void => { + const fs = testInjector.resolve("fs"); + fs.readJson = (filename: string, encoding?: string): any => { + const extensionName = _.find(extensionNames, extName => filename.indexOf(extName) !== -1); + if (extensionName) { + return { + name: extensionName, + version: "1.0.0" + }; + } + + const dependencies: any = {}; + _.each(extensionNames, name => { + dependencies[name] = "1.0.0"; + }); + + return { dependencies }; + }; + }; + describe("installExtension", () => { describe("fails", () => { it("when extensions dir does not exist and trying to create it fails", async () => { @@ -133,17 +162,20 @@ describe("extensibilityService", () => { it("returns the name of the installed extension", async () => { const extensionName = "extension1"; const testInjector = getTestInjector(); + const fs: IFileSystem = testInjector.resolve("fs"); fs.exists = (pathToCheck: string): boolean => path.basename(pathToCheck) !== extensionName; fs.readDirectory = (dir: string): string[] => [extensionName]; + fs.readJson = () => ({ name: extensionName, version: "1.0.0" }); + const npm: INodePackageManager = testInjector.resolve("npm"); - npm.install = async (packageName: string, pathToSave: string, config?: any): Promise => ({ name: extensionName }); + npm.install = async (packageName: string, pathToSave: string, config?: any): Promise => ({ name: extensionName, version: "1.0.0" }); const extensibilityService: IExtensibilityService = testInjector.resolve(ExtensibilityService); const actualResult = await extensibilityService.installExtension(extensionName); - assert.deepEqual(actualResult, { extensionName }); + assert.deepEqual(actualResult, { extensionName, version: "1.0.0", docs: undefined, pathToExtension: getExpectedInstallationPathForExtension(testInjector, extensionName) }); }); }); @@ -160,16 +192,16 @@ describe("extensibilityService", () => { return extensionNames; }; - fs.readJson = (filename: string, encoding?: string): any => { - const dependencies: any = {}; - _.each(extensionNames, name => { - dependencies[name] = "1.0.0"; - }); - - return { dependencies }; - }; + mockFsReadJson(testInjector, extensionNames); - const expectedResults: IExtensionData[] = _.map(extensionNames, extensionName => ({ extensionName })); + const expectedResults: IExtensionData[] = _.map(extensionNames, extensionName => ( + { + extensionName, + version: "1.0.0", + pathToExtension: getExpectedInstallationPathForExtension(testInjector, extensionName), + docs: undefined + } + )); const extensibilityService: IExtensibilityService = testInjector.resolve(ExtensibilityService); const actualResult = await Promise.all(extensibilityService.loadExtensions()); @@ -194,14 +226,7 @@ describe("extensibilityService", () => { } }; - fs.readJson = (filename: string, encoding?: string): any => { - const dependencies: any = {}; - _.each(extensionNames, name => { - dependencies[name] = "1.0.0"; - }); - - return { dependencies }; - }; + mockFsReadJson(testInjector, extensionNames); let isNpmInstallCalled = false; const npm: INodePackageManager = testInjector.resolve("npm"); @@ -211,7 +236,7 @@ describe("extensibilityService", () => { return { name: packageName }; }; - const expectedResults: IExtensionData[] = _.map(extensionNames, extensionName => ({ extensionName })); + const expectedResults: IExtensionData[] = _.map(extensionNames, extensionName => ({ extensionName, version: "1.0.0", pathToExtension: getExpectedInstallationPathForExtension(testInjector, extensionName), docs: undefined })); const extensibilityService: IExtensibilityService = testInjector.resolve(ExtensibilityService); const actualResult = await Promise.all(extensibilityService.loadExtensions()); @@ -230,14 +255,7 @@ describe("extensibilityService", () => { return extensionNames; }; - fs.readJson = (filename: string, encoding?: string): any => { - const dependencies: any = {}; - _.each(extensionNames, name => { - dependencies[name] = "1.0.0"; - }); - - return { dependencies }; - }; + mockFsReadJson(testInjector, extensionNames); const requireService: IRequireService = testInjector.resolve("requireService"); requireService.require = (module: string) => { @@ -246,7 +264,7 @@ describe("extensibilityService", () => { } }; - const expectedResults: any[] = _.map(extensionNames, extensionName => ({ extensionName })); + const expectedResults: any[] = _.map(extensionNames, extensionName => ({ extensionName, version: "1.0.0", pathToExtension: getExpectedInstallationPathForExtension(testInjector, extensionName), docs: undefined })); expectedResults[0] = new Error("Unable to load extension extension1. You will not be able to use the functionality that it adds. Error: Unable to load module."); const extensibilityService: IExtensibilityService = testInjector.resolve(ExtensibilityService); const promises = extensibilityService.loadExtensions(); @@ -269,14 +287,7 @@ describe("extensibilityService", () => { const fs: IFileSystem = testInjector.resolve("fs"); const expectedErrorMessage = `Unable to read ${constants.NODE_MODULES_FOLDER_NAME} dir.`; fs.exists = (pathToCheck: string): boolean => path.basename(pathToCheck) === "extensions" || path.basename(pathToCheck) === constants.PACKAGE_JSON_FILE_NAME; - fs.readJson = (filename: string, encoding?: string): any => { - const dependencies: any = {}; - _.each(extensionNames, name => { - dependencies[name] = "1.0.0"; - }); - - return { dependencies }; - }; + mockFsReadJson(testInjector, extensionNames); let isReadDirCalled = false; fs.readDirectory = (dir: string): string[] => { @@ -310,14 +321,7 @@ describe("extensibilityService", () => { "expected 'extension3' to deeply equal 'extension1'"]; const fs: IFileSystem = testInjector.resolve("fs"); fs.exists = (pathToCheck: string): boolean => path.basename(pathToCheck) === "extensions" || path.basename(pathToCheck) === constants.PACKAGE_JSON_FILE_NAME; - fs.readJson = (filename: string, encoding?: string): any => { - const dependencies: any = {}; - _.each(extensionNames, name => { - dependencies[name] = "1.0.0"; - }); - - return { dependencies }; - }; + mockFsReadJson(testInjector, extensionNames); let isReadDirCalled = false; fs.readDirectory = (dir: string): string[] => { From e44029ff9e8156c953bd0abad2be9772ff616230 Mon Sep 17 00:00:00 2001 From: Rosen Vladimirov Date: Thu, 15 Feb 2018 00:53:27 +0200 Subject: [PATCH 2/2] refactor(help-service): Extract methods instead of duplicating code --- lib/common | 2 +- lib/services/extensibility-service.ts | 15 ++------------- 2 files changed, 3 insertions(+), 14 deletions(-) diff --git a/lib/common b/lib/common index e39698131f..c3d02f3f5f 160000 --- a/lib/common +++ b/lib/common @@ -1 +1 @@ -Subproject commit e39698131f2469322273467ab29e0f9c09dbb8ed +Subproject commit c3d02f3f5f3858b7647a751eb501010eabd609f9 diff --git a/lib/services/extensibility-service.ts b/lib/services/extensibility-service.ts index e1ca659996..babb682671 100644 --- a/lib/services/extensibility-service.ts +++ b/lib/services/extensibility-service.ts @@ -35,16 +35,7 @@ export class ExtensibilityService implements IExtensibilityService { const installResultInfo = await this.$npm.install(packageName, this.pathToExtensions, npmOpts); this.$logger.trace(`Finished installation of extension '${extensionName}'. Trying to load it now.`); - const packageJsonData = this.getExtensionPackageJsonData(installResultInfo.name); - - const pathToExtension = this.getPathToExtension(extensionName); - const docs = packageJsonData && packageJsonData.nativescript && packageJsonData.nativescript.docs && path.join(pathToExtension, packageJsonData.nativescript.docs); - return { - extensionName: installResultInfo.name, - version: installResultInfo.version, - docs, - pathToExtension - }; + return this.getInstalledExtensionData(installResultInfo.name); } @exported("extensibilityService") @@ -60,9 +51,7 @@ export class ExtensibilityService implements IExtensibilityService { public getInstalledExtensionsData(): IExtensionData[] { const installedExtensions = this.getInstalledExtensions(); - return _.keys(installedExtensions).map(installedExtension => { - return this.getInstalledExtensionData(installedExtension); - }); + return _.keys(installedExtensions).map(installedExtension => this.getInstalledExtensionData(installedExtension)); } @exported("extensibilityService")