From 46e73eea69abc2ba80bb1397c6779b8789dbd385 Mon Sep 17 00:00:00 2001 From: Toru Nagashima Date: Sat, 17 Jun 2017 00:00:46 +0900 Subject: [PATCH] Fix: eslint --init installs wrong dependencies of popular styles (fixes #7338) (#8713) --- lib/config/config-initializer.js | 51 ++++++++++++++------------ lib/util/npm-util.js | 15 ++++++++ tests/lib/config/config-initializer.js | 38 ++++++++++++++++--- tests/lib/util/npm-util.js | 11 ++++++ 4 files changed, 87 insertions(+), 28 deletions(-) diff --git a/lib/config/config-initializer.js b/lib/config/config-initializer.js index ed4bde8757e..e87cb79b6db 100644 --- a/lib/config/config-initializer.js +++ b/lib/config/config-initializer.js @@ -62,42 +62,47 @@ function writeFile(config, format) { * @returns {void} */ function installModules(config) { - let modules = []; + const modules = {}; // Create a list of modules which should be installed based on config if (config.plugins) { - modules = modules.concat(config.plugins.map(name => `eslint-plugin-${name}`)); + for (const plugin of config.plugins) { + modules[`eslint-plugin-${plugin}`] = "latest"; + } } if (config.extends && config.extends.indexOf("eslint:") === -1) { - modules.push(`eslint-config-${config.extends}`); + const moduleName = `eslint-config-${config.extends}`; + + log.info(`Checking peerDependencies of ${moduleName}`); + modules[moduleName] = "latest"; + Object.assign( + modules, + npmUtil.fetchPeerDependencies(`${moduleName}@latest`) + ); } - // Determine which modules are already installed - if (modules.length === 0) { + // If no modules, do nothing. + if (Object.keys(modules).length === 0) { return; } // Add eslint to list in case user does not have it installed locally - modules.unshift("eslint"); + modules.eslint = modules.eslint || "latest"; - const installStatus = npmUtil.checkDevDeps(modules); + // Mark to show messages if it's new installation of eslint. + const installStatus = npmUtil.checkDevDeps(["eslint"]); - // Install packages which aren't already installed - const modulesToInstall = Object.keys(installStatus).filter(module => { - const notInstalled = installStatus[module] === false; + if (installStatus.eslint === false) { + log.info("Local ESLint installation not found."); + config.installedESLint = true; + } - if (module === "eslint" && notInstalled) { - log.info("Local ESLint installation not found."); - config.installedESLint = true; - } + // Install packages + const modulesToInstall = Object.keys(modules).map(name => `${name}@${modules[name]}`); - return notInstalled; - }); + log.info(`Installing ${modulesToInstall.join(", ")}`); - if (modulesToInstall.length > 0) { - log.info(`Installing ${modulesToInstall.join(", ")}`); - npmUtil.installSyncSaveDev(modulesToInstall); - } + npmUtil.installSyncSaveDev(modulesToInstall); } /** @@ -265,9 +270,9 @@ function processAnswers(answers) { function getConfigForStyleGuide(guide) { const guides = { google: { extends: "google" }, - airbnb: { extends: "airbnb", plugins: ["react", "jsx-a11y", "import"] }, - "airbnb-base": { extends: "airbnb-base", plugins: ["import"] }, - standard: { extends: "standard", plugins: ["standard", "promise"] } + airbnb: { extends: "airbnb" }, + "airbnb-base": { extends: "airbnb-base" }, + standard: { extends: "standard" } }; if (!guides[guide]) { diff --git a/lib/util/npm-util.js b/lib/util/npm-util.js index 4859fabc956..057dbfea4d0 100644 --- a/lib/util/npm-util.js +++ b/lib/util/npm-util.js @@ -56,6 +56,20 @@ function installSyncSaveDev(packages) { childProcess.execSync(`npm i --save-dev ${packages}`, { stdio: "inherit", encoding: "utf8" }); } +/** + * Fetch `peerDependencies` of the given package by `npm show` command. + * @param {string} packageName The package name to fetch peerDependencies. + * @returns {string[]} Gotten peerDependencies. + */ +function fetchPeerDependencies(packageName) { + const fetchedText = childProcess.execSync( + `npm show --json ${packageName} peerDependencies`, + { encoding: "utf8" } + ).trim(); + + return JSON.parse(fetchedText || "{}"); +} + /** * Check whether node modules are include in a project's package.json. * @@ -140,6 +154,7 @@ function checkPackageJson(startDir) { module.exports = { installSyncSaveDev, + fetchPeerDependencies, checkDeps, checkDevDeps, checkPackageJson diff --git a/tests/lib/config/config-initializer.js b/tests/lib/config/config-initializer.js index d313dca5f5b..1a7d0a90617 100644 --- a/tests/lib/config/config-initializer.js +++ b/tests/lib/config/config-initializer.js @@ -32,6 +32,7 @@ describe("configInitializer", () => { let fixtureDir, npmCheckStub, npmInstallStub, + npmFetchPeerDependenciesStub, init; const log = { @@ -75,6 +76,14 @@ describe("configInitializer", () => { status[pkg] = false; return status; }, {})); + npmFetchPeerDependenciesStub = sinon + .stub(npmUtil, "fetchPeerDependencies") + .returns({ + eslint: "^3.19.0", + "eslint-plugin-jsx-a11y": "^5.0.1", + "eslint-plugin-import": "^2.2.0", + "eslint-plugin-react": "^7.0.1" + }); init = proxyquire("../../../lib/config/config-initializer", requireStubs); }); @@ -83,6 +92,7 @@ describe("configInitializer", () => { log.error.reset(); npmInstallStub.restore(); npmCheckStub.restore(); + npmFetchPeerDependenciesStub.restore(); }); after(() => { @@ -185,19 +195,19 @@ describe("configInitializer", () => { it("should support the airbnb style guide", () => { const config = init.getConfigForStyleGuide("airbnb"); - assert.deepEqual(config, { extends: "airbnb", installedESLint: true, plugins: ["react", "jsx-a11y", "import"] }); + assert.deepEqual(config, { extends: "airbnb", installedESLint: true }); }); it("should support the airbnb base style guide", () => { const config = init.getConfigForStyleGuide("airbnb-base"); - assert.deepEqual(config, { extends: "airbnb-base", installedESLint: true, plugins: ["import"] }); + assert.deepEqual(config, { extends: "airbnb-base", installedESLint: true }); }); it("should support the standard style guide", () => { const config = init.getConfigForStyleGuide("standard"); - assert.deepEqual(config, { extends: "standard", installedESLint: true, plugins: ["standard", "promise"] }); + assert.deepEqual(config, { extends: "standard", installedESLint: true }); }); it("should throw when encountering an unsupported style guide", () => { @@ -209,13 +219,31 @@ describe("configInitializer", () => { it("should install required sharable config", () => { init.getConfigForStyleGuide("google"); assert(npmInstallStub.calledOnce); - assert.deepEqual(npmInstallStub.firstCall.args[0][1], "eslint-config-google"); + assert(npmInstallStub.firstCall.args[0].some(name => name.startsWith("eslint-config-google@"))); }); it("should install ESLint if not installed locally", () => { init.getConfigForStyleGuide("google"); assert(npmInstallStub.calledOnce); - assert.deepEqual(npmInstallStub.firstCall.args[0][0], "eslint"); + assert(npmInstallStub.firstCall.args[0].some(name => name.startsWith("eslint@"))); + }); + + it("should install peerDependencies of the sharable config", () => { + init.getConfigForStyleGuide("airbnb"); + + assert(npmFetchPeerDependenciesStub.calledOnce); + assert(npmFetchPeerDependenciesStub.firstCall.args[0] === "eslint-config-airbnb@latest"); + assert(npmInstallStub.calledOnce); + assert.deepEqual( + npmInstallStub.firstCall.args[0], + [ + "eslint-config-airbnb@latest", + "eslint@^3.19.0", + "eslint-plugin-jsx-a11y@^5.0.1", + "eslint-plugin-import@^2.2.0", + "eslint-plugin-react@^7.0.1" + ] + ); }); }); diff --git a/tests/lib/util/npm-util.js b/tests/lib/util/npm-util.js index 48a41aa5067..c8c7d8f2d28 100644 --- a/tests/lib/util/npm-util.js +++ b/tests/lib/util/npm-util.js @@ -187,4 +187,15 @@ describe("npmUtil", () => { stub.restore(); }); }); + + describe("fetchPeerDependencies()", () => { + it("should execute 'npm show --json peerDependencies' command", () => { + const stub = sandbox.stub(childProcess, "execSync").returns(""); + + npmUtil.fetchPeerDependencies("desired-package"); + assert(stub.calledOnce); + assert.equal(stub.firstCall.args[0], "npm show --json desired-package peerDependencies"); + stub.restore(); + }); + }); });