From b5d96d215f3c73cd84532091bed4473e52cbd47a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9mie=20Astori?= Date: Mon, 5 Feb 2018 01:30:57 -0500 Subject: [PATCH 1/3] Ensure packages loaded are directories --- src/plugins/packages/index.js | 44 ++++++++++++++++++++--------------- 1 file changed, 25 insertions(+), 19 deletions(-) diff --git a/src/plugins/packages/index.js b/src/plugins/packages/index.js index 080e1094..aefa2303 100644 --- a/src/plugins/packages/index.js +++ b/src/plugins/packages/index.js @@ -39,34 +39,40 @@ function loadPackages() { if (err) { return; } - packages.forEach((packageName) => { - const packageFile = getModuleInfo(packageName); - if (!packageFile) { - return; - } - packageMap.set(packageName, packageFile); - if (packageFile.type === "theme") { - themes.addTheme(packageName, packageFile); - } - if (packageFile.onServerStart) { - packageFile.onServerStart(packageApis(packageName)); - } + packages.forEach((packageName) => { + fs.stat(Helper.getPackageModulePath(packageName), function(err2, stat) { + if (err2 || !stat.isDirectory()) { + return; + } + + const packageFile = getModuleInfo(packageName); + + if (!packageFile) { + return; + } + + packageMap.set(packageName, packageFile); + + if (packageFile.type === "theme") { + themes.addTheme(packageName, packageFile); + } + + if (packageFile.onServerStart) { + packageFile.onServerStart(packageApis(packageName)); + } + }); }); }); } function getModuleInfo(packageName) { - let module; - try { - module = require(Helper.getPackageModulePath(packageName)); - } catch (e) { - log.warn(`Specified package ${colors.yellow(packageName)} is not installed in packages directory`); - return; - } + const module = require(Helper.getPackageModulePath(packageName)); + if (!module.thelounge) { log.warn(`Specified package ${colors.yellow(packageName)} doesn't have required information.`); return; } + return module.thelounge; } From 2c570fa9efe87c647c4bda69a931dc4b323f8eac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9mie=20Astori?= Date: Thu, 8 Feb 2018 00:19:44 -0500 Subject: [PATCH 2/3] Load packages from package.json, fix loading thelounge metadata from the wrong file, add tests --- src/command-line/install.js | 7 +- src/command-line/uninstall.js | 7 +- src/helper.js | 4 +- src/plugins/packages/index.js | 65 +++++++++---------- test/fixtures/.gitignore | 5 +- .../thelounge-package-foo/index.js | 7 ++ .../thelounge-package-foo/package.json | 12 ++++ test/fixtures/.lounge/packages/package.json | 6 ++ test/plugins/packages/indexTest.js | 61 +++++++++++++++++ test/util.js | 2 +- 10 files changed, 131 insertions(+), 45 deletions(-) create mode 100644 test/fixtures/.lounge/packages/node_modules/thelounge-package-foo/index.js create mode 100644 test/fixtures/.lounge/packages/node_modules/thelounge-package-foo/package.json create mode 100644 test/fixtures/.lounge/packages/package.json create mode 100644 test/plugins/packages/indexTest.js diff --git a/src/command-line/install.js b/src/command-line/install.js index 45aed91f..da98492c 100644 --- a/src/command-line/install.js +++ b/src/command-line/install.js @@ -35,11 +35,10 @@ program log.info(`Installing ${colors.green(packageName)}...`); const packagesPath = Helper.getPackagesPath(); - const packagesParent = path.dirname(packagesPath); - const packagesConfig = path.join(packagesParent, "package.json"); + const packagesConfig = path.join(packagesPath, "package.json"); // Create node_modules folder, otherwise npm will start walking upwards to find one - fsextra.ensureDirSync(packagesPath); + fsextra.ensureDirSync(path.join(packagesPath, "node_modules")); // Create package.json with private set to true to avoid npm warnings, if // it doesn't exist already @@ -61,7 +60,7 @@ program "--no-package-lock", "--no-progress", "--prefix", - packagesParent, + packagesPath, packageName, ], { diff --git a/src/command-line/uninstall.js b/src/command-line/uninstall.js index be797f88..32fe4d52 100644 --- a/src/command-line/uninstall.js +++ b/src/command-line/uninstall.js @@ -22,8 +22,7 @@ program log.info(`Uninstalling ${colors.green(packageName)}...`); const packagesPath = Helper.getPackagesPath(); - const packagesParent = path.dirname(packagesPath); - const packagesConfig = path.join(packagesParent, "package.json"); + const packagesConfig = path.join(packagesPath, "package.json"); const packageWasNotInstalled = `${colors.green(packageName)} was not installed.`; if (!fs.existsSync(packagesConfig)) { @@ -48,7 +47,7 @@ program "--depth", "0", "--prefix", - packagesParent, + packagesPath, packageName, ], { @@ -80,7 +79,7 @@ program "uninstall", "--no-progress", "--prefix", - packagesParent, + packagesPath, packageName, ], { diff --git a/src/helper.js b/src/helper.js index 12683c4f..94ef865a 100644 --- a/src/helper.js +++ b/src/helper.js @@ -75,7 +75,7 @@ function setHome(newPath) { configPath = path.join(homePath, "config.js"); usersPath = path.join(homePath, "users"); storagePath = path.join(homePath, "storage"); - packagesPath = path.join(homePath, "packages", "node_modules"); + packagesPath = path.join(homePath, "packages"); // Reload config from new home location if (fs.existsSync(configPath)) { @@ -144,7 +144,7 @@ function getPackagesPath() { } function getPackageModulePath(packageName) { - return path.join(Helper.getPackagesPath(), packageName); + return path.join(Helper.getPackagesPath(), "node_modules", packageName); } function ip2hex(address) { diff --git a/src/plugins/packages/index.js b/src/plugins/packages/index.js index aefa2303..daeaf29a 100644 --- a/src/plugins/packages/index.js +++ b/src/plugins/packages/index.js @@ -1,7 +1,7 @@ "use strict"; -const fs = require("fs"); const colors = require("colors/safe"); +const path = require("path"); const Helper = require("../../helper"); const themes = require("./themes"); const packageMap = new Map(); @@ -35,44 +35,43 @@ function getPackage(name) { } function loadPackages() { - fs.readdir(Helper.getPackagesPath(), (err, packages) => { - if (err) { + const packageJson = path.join(Helper.getPackagesPath(), "package.json"); + let packages; + + try { + packages = Object.keys(require(packageJson).dependencies); + } catch (e) { + packages = []; + } + + packages.forEach((packageName) => { + const errorMsg = `Package ${colors.bold(packageName)} could not be loaded`; + let packageInfo; + let packageFile; + + try { + packageInfo = require(path.join(Helper.getPackageModulePath(packageName), "package.json")); + packageFile = require(Helper.getPackageModulePath(packageName)); + } catch (e) { + log.warn(errorMsg); return; } - packages.forEach((packageName) => { - fs.stat(Helper.getPackageModulePath(packageName), function(err2, stat) { - if (err2 || !stat.isDirectory()) { - return; - } + if (!packageInfo.thelounge) { + log.warn(errorMsg); + return; + } - const packageFile = getModuleInfo(packageName); + packageMap.set(packageName, packageFile); - if (!packageFile) { - return; - } + if (packageInfo.type === "theme") { + themes.addTheme(packageName, packageInfo); + } - packageMap.set(packageName, packageFile); + if (packageFile.onServerStart) { + packageFile.onServerStart(packageApis(packageName)); + } - if (packageFile.type === "theme") { - themes.addTheme(packageName, packageFile); - } - - if (packageFile.onServerStart) { - packageFile.onServerStart(packageApis(packageName)); - } - }); - }); + log.info(`Package ${colors.bold(packageName)} loaded`); }); } - -function getModuleInfo(packageName) { - const module = require(Helper.getPackageModulePath(packageName)); - - if (!module.thelounge) { - log.warn(`Specified package ${colors.yellow(packageName)} doesn't have required information.`); - return; - } - - return module.thelounge; -} diff --git a/test/fixtures/.gitignore b/test/fixtures/.gitignore index 5708c99e..03ed0764 100644 --- a/test/fixtures/.gitignore +++ b/test/fixtures/.gitignore @@ -1,2 +1,5 @@ -# files that may be generated by tests +# Files that may be generated by tests .lounge/storage/ + +# Fixtures contain fake packages, stored in a fake node_modules folder +!.lounge/packages/node_modules/ diff --git a/test/fixtures/.lounge/packages/node_modules/thelounge-package-foo/index.js b/test/fixtures/.lounge/packages/node_modules/thelounge-package-foo/index.js new file mode 100644 index 00000000..34ae57a2 --- /dev/null +++ b/test/fixtures/.lounge/packages/node_modules/thelounge-package-foo/index.js @@ -0,0 +1,7 @@ +"use strict"; + +module.exports = { + onServerStart(apis) { + apis.Stylesheets.addFile("style.css"); + }, +}; diff --git a/test/fixtures/.lounge/packages/node_modules/thelounge-package-foo/package.json b/test/fixtures/.lounge/packages/node_modules/thelounge-package-foo/package.json new file mode 100644 index 00000000..455dc352 --- /dev/null +++ b/test/fixtures/.lounge/packages/node_modules/thelounge-package-foo/package.json @@ -0,0 +1,12 @@ +{ + "name": "thelounge-package-foo", + "private": true, + "main": "index.js", + "thelounge": { + "type": "package" + }, + "keywords": [ + "thelounge", + "thelounge-package" + ] +} diff --git a/test/fixtures/.lounge/packages/package.json b/test/fixtures/.lounge/packages/package.json new file mode 100644 index 00000000..8e5d9abc --- /dev/null +++ b/test/fixtures/.lounge/packages/package.json @@ -0,0 +1,6 @@ +{ + "private": true, + "dependencies": { + "thelounge-package-foo": "*" + } +} diff --git a/test/plugins/packages/indexTest.js b/test/plugins/packages/indexTest.js new file mode 100644 index 00000000..e5be96fa --- /dev/null +++ b/test/plugins/packages/indexTest.js @@ -0,0 +1,61 @@ +"use strict"; + +const expect = require("chai").expect; +const TestUtil = require("../../util"); + +let packages; + +describe("packages", function() { + let originalLogInfo; + + beforeEach(function() { + originalLogInfo = log.info; + log.info = () => {}; + + delete require.cache[require.resolve("../../../src/plugins/packages")]; + packages = require("../../../src/plugins/packages"); + }); + + afterEach(function() { + log.info = originalLogInfo; + }); + + describe(".getStylesheets", function() { + it("should contain no stylesheets before packages are loaded", function() { + expect(packages.getStylesheets()).to.be.empty; + }); + + it("should return the list of registered stylesheets for loaded packages", function() { + packages.loadPackages(); + + expect(packages.getStylesheets()).to.deep.equal([ + "thelounge-package-foo/style.css", + ]); + }); + }); + + describe(".getPackage", function() { + it("should contain no reference to packages before loading them", function() { + expect(packages.getPackage("thelounge-package-foo")).to.be.undefined; + }); + + it("should return details of a registered package after it was loaded", function() { + packages.loadPackages(); + + expect(packages.getPackage("thelounge-package-foo")) + .to.have.key("onServerStart"); + }); + }); + + describe(".loadPackages", function() { + it("should display report about loading packages", function() { + // Mock `log.info` to extract its effect into a string + let stdout = ""; + log.info = TestUtil.mockLogger((str) => stdout += str); + + packages.loadPackages(); + + expect(stdout).to.deep.equal("Package thelounge-package-foo loaded\n"); + }); + }); +}); diff --git a/test/util.js b/test/util.js index cf519f73..66e9987e 100644 --- a/test/util.js +++ b/test/util.js @@ -27,7 +27,7 @@ function mockLogger(callback) { return function() { // TODO: Use ...args with The Lounge v3: add `...args` as function argument // and replaced the next line with `args.join(", ")` - const stdout = Array.prototype.slice.call(arguments).join(", ") + const stdout = Array.prototype.slice.call(arguments).join(" ") .replace( // Removes ANSI colors. See https://stackoverflow.com/a/29497680 /[\u001b\u009b][[()#;?]*(?:[0-9]{1,4}(?:;[0-9]{0,4})*)?[0-9A-ORZcf-nqry=><]/g, "" From 6d931e8dcb29afd356071b88b4b66bd9d1afa949 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9mie=20Astori?= Date: Thu, 8 Feb 2018 01:37:51 -0500 Subject: [PATCH 3/3] Make sure packages are correctly removed from package.json when uninstalled --- src/command-line/uninstall.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/command-line/uninstall.js b/src/command-line/uninstall.js index 32fe4d52..e29f9b8b 100644 --- a/src/command-line/uninstall.js +++ b/src/command-line/uninstall.js @@ -77,6 +77,7 @@ program npm, [ "uninstall", + "--save", "--no-progress", "--prefix", packagesPath,