Re-introduce lodash into src/package-install.js (#10315)

* test: add failing test for if package.json is non-existant, fix tests' beforeEach method

* Revert "fix: #10289, remove lodash dependency in src/cli/package-install.js"

This reverts commit 81fa2e22bc.

* fix: regression caused by 94b79ce402

`./nodebb setup` was no longer able to be called without arguments or env vars

* fix: .updatePackageFile() throwing if no package.json

* fix: removing unneeded code in src/cli/index.js that seemed to be used to handle cases where package.json was missing (initial install)

... However, as .updatePackageFile() now handled cases where there is no package.json, it should be ok to remove this code

* fix: handle missing package.json or node_modules/

install/package.json

@@ -2,7 +2,7 @@
     "name": "nodebb",
     "license": "GPL-3.0",
     "description": "NodeBB Forum",
-    "version": "1.19.2",
+    "version": "1.19.3",
     "homepage": "http://www.nodebb.org",
     "repository": {
         "type": "git",
@@ -184,4 +184,4 @@
             "url": "https://github.com/barisusakli"
         }
     ]
-}
+}

src/cli/index.js

@@ -10,31 +10,18 @@ require('../../require-main');
 const packageInstall = require('./package-install');
 const { paths } = require('../constants');
 
-// check to make sure dependencies are installed
 try {
-	fs.accessSync(paths.currentPackage, fs.constants.R_OK);
-} catch (e) {
-	if (e.code === 'ENOENT') {
-		console.warn('package.json not found.');
-		console.log('Populating package.json...');
-
-		packageInstall.updatePackageFile();
-		packageInstall.preserveExtraneousPlugins();
-
-		try {
-			fs.accessSync(path.join(paths.nodeModules, 'chalk/package.json'), fs.constants.R_OK);
-
-			const chalk = require('chalk');
-			console.log(chalk.green('OK'));
-		} catch (e) {
-			console.log('OK');
+	fs.accessSync(paths.currentPackage, fs.constants.R_OK); // throw on missing package.json
+	try { // handle missing node_modules/ directory
+		fs.accessSync(paths.nodeModules, fs.constants.R_OK);
+	} catch (e) {
+		if (e.code === 'ENOENT') {
+			// run package installation just to sync up node_modules/ with existing package.json
+			packageInstall.installAll();
+		} else {
+			throw e;
 		}
-	} else {
-		throw e;
 	}
-}
-
-try {
 	fs.accessSync(path.join(paths.nodeModules, 'semver/package.json'), fs.constants.R_OK);
 
 	const semver = require('semver');
@@ -53,12 +40,14 @@ try {
 	checkVersion('async');
 	checkVersion('commander');
 	checkVersion('chalk');
+	checkVersion('lodash');
 } catch (e) {
 	if (['ENOENT', 'DEP_WRONG_VERSION', 'MODULE_NOT_FOUND'].includes(e.code)) {
 		console.warn('Dependencies outdated or not yet installed.');
 		console.log('Installing them now...\n');
 
 		packageInstall.updatePackageFile();
+		packageInstall.preserveExtraneousPlugins();
 		packageInstall.installAll();
 
 		const chalk = require('chalk');

src/cli/package-install.js

@@ -1,6 +1,7 @@
 'use strict';
 
 const path = require('path');
+
 const fs = require('fs');
 const cproc = require('child_process');
 
@@ -17,34 +18,22 @@ function sortDependencies(dependencies) {
 		}, {});
 }
 
-function merge(to, from) {
-	// Poor man's version of _.merge()
-	if (Object.values(from).every(val => typeof val !== 'object')) {
-		return Object.assign(to, from);
-	}
-
-	Object.keys(from).forEach((key) => {
-		if (Object.getPrototypeOf(from[key]) === Object.prototype) {
-			to[key] = merge(to[key], from[key]);
-		} else {
-			to[key] = from[key];
-		}
-	});
-
-	return to;
-}
-
 pkgInstall.updatePackageFile = () => {
-	let oldPackageContents = {};
+	let oldPackageContents;
 
 	try {
 		oldPackageContents = JSON.parse(fs.readFileSync(paths.currentPackage, 'utf8'));
 	} catch (e) {
 		if (e.code !== 'ENOENT') {
 			throw e;
+		} else {
+			// No local package.json, copy from install/package.json
+			fs.copyFileSync(paths.installPackage, paths.currentPackage);
+			return;
 		}
 	}
 
+	const _ = require('lodash');
 	const defaultPackageContents = JSON.parse(fs.readFileSync(paths.installPackage, 'utf8'));
 
 	let dependencies = {};
@@ -59,7 +48,7 @@ pkgInstall.updatePackageFile = () => {
 	// Sort dependencies alphabetically
 	dependencies = sortDependencies({ ...dependencies, ...defaultPackageContents.dependencies });
 
-	const packageContents = { ...merge(oldPackageContents, defaultPackageContents), dependencies, devDependencies };
+	const packageContents = { ..._.merge(oldPackageContents, defaultPackageContents), dependencies, devDependencies };
 	fs.writeFileSync(paths.currentPackage, JSON.stringify(packageContents, null, 4));
 };
 

src/install.js

@@ -47,7 +47,7 @@ questions.optional = [
 ];
 
 function checkSetupFlagEnv() {
-	let setupVal = install.values || {};
+	let setupVal = install.values;
 
 	const envConfMap = {
 		NODEBB_URL: 'url',
@@ -65,17 +65,21 @@ function checkSetupFlagEnv() {
 	};
 
 	// Set setup values from env vars (if set)
-	winston.info('[install/checkSetupFlagEnv] checking env vars for setup info...');
+	const envKeys = Object.keys(process.env);
+	if (Object.keys(envConfMap).some(key => envKeys.includes(key))) {
+		winston.info('[install/checkSetupFlagEnv] checking env vars for setup info...');
+		setupVal = setupVal || {};
 
-	Object.entries(process.env).forEach(([evName, evValue]) => { // get setup values from env
-		if (evName.startsWith('NODEBB_DB_')) {
-			setupVal[`${process.env.NODEBB_DB}:${envConfMap[evName]}`] = evValue;
-		} else if (evName.startsWith('NODEBB_')) {
-			setupVal[envConfMap[evName]] = evValue;
-		}
-	});
+		Object.entries(process.env).forEach(([evName, evValue]) => { // get setup values from env
+			if (evName.startsWith('NODEBB_DB_')) {
+				setupVal[`${process.env.NODEBB_DB}:${envConfMap[evName]}`] = evValue;
+			} else if (evName.startsWith('NODEBB_')) {
+				setupVal[envConfMap[evName]] = evValue;
+			}
+		});
 
-	setupVal['admin:password:confirm'] = setupVal['admin:password'];
+		setupVal['admin:password:confirm'] = setupVal['admin:password'];
+	}
 
 	// try to get setup values from json, if successful this overwrites all values set by env
 	// TODO: better behaviour would be to support overrides per value, i.e. in order of priority (generic pattern):

test/package-install.js

@@ -23,12 +23,11 @@ describe('Package install lib', () => {
 			// Move `install/package.json` and `package.json` out of the way for now
 			await fs.copyFile(sourcePackagePath, path.resolve(__dirname, '../install/package.json.bak')); // safekeeping
 			await fs.copyFile(packageFilePath, path.resolve(__dirname, '../package.json.bak')); // safekeeping
-			await fs.copyFile(sourcePackagePath, packageFilePath); // match files for testing
 		});
 
 		beforeEach(async () => {
 			await fs.copyFile(path.resolve(__dirname, '../install/package.json.bak'), sourcePackagePath);
-			await fs.copyFile(path.resolve(__dirname, '../package.json.bak'), packageFilePath);
+			await fs.copyFile(sourcePackagePath, packageFilePath); // match files for testing
 			source = JSON.parse(await fs.readFile(sourcePackagePath));
 			current = JSON.parse(await fs.readFile(packageFilePath));
 		});
@@ -95,6 +94,14 @@ describe('Package install lib', () => {
 			assert.strictEqual(updated.devDependencies.hasOwnProperty('expect'), false);
 		});
 
+		it('should handle if there is no package.json', async () => {
+			await fs.unlink(packageFilePath);
+
+			pkgInstall.updatePackageFile();
+			const updated = JSON.parse(await fs.readFile(packageFilePath, 'utf8'));
+			assert.deepStrictEqual(updated, source);
+		});
+
 		after(async () => {
 			// Clean up
 			await fs.rename(path.resolve(__dirname, '../install/package.json.bak'), sourcePackagePath);