From 17836f2a3a4708c5916d4361d3a278d17bdc2981 Mon Sep 17 00:00:00 2001 From: Julian Lam Date: Thu, 17 Feb 2022 14:10:24 -0500 Subject: [PATCH] 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 81fa2e22bc954a9c1878b0529cceee03f0f3a87b. * fix: regression caused by 94b79ce4024f72a3eee2cfa06b05d8f66898149f `./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/ --- src/cli/index.js | 33 +++++++++++---------------------- src/cli/package-install.js | 27 ++++++++------------------- test/package-install.js | 11 +++++++++-- 3 files changed, 28 insertions(+), 43 deletions(-) diff --git a/src/cli/index.js b/src/cli/index.js index 0ed6b449fd..7b5670dc75 100644 --- a/src/cli/index.js +++ b/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'); diff --git a/src/cli/package-install.js b/src/cli/package-install.js index d5e99b16d4..e9ca4a2e44 100644 --- a/src/cli/package-install.js +++ b/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)); }; diff --git a/test/package-install.js b/test/package-install.js index 0d31387df1..97d6041bae 100644 --- a/test/package-install.js +++ b/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);