Skip to content

Commit

Permalink
Fix: eslint --init installs wrong dependencies of popular styles (fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
mysticatea authored and gyandeeps committed Jun 16, 2017
1 parent a82361b commit 46e73ee
Show file tree
Hide file tree
Showing 4 changed files with 87 additions and 28 deletions.
51 changes: 28 additions & 23 deletions lib/config/config-initializer.js
Expand Up @@ -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);
}

/**
Expand Down Expand Up @@ -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]) {
Expand Down
15 changes: 15 additions & 0 deletions lib/util/npm-util.js
Expand Up @@ -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.
*
Expand Down Expand Up @@ -140,6 +154,7 @@ function checkPackageJson(startDir) {

module.exports = {
installSyncSaveDev,
fetchPeerDependencies,
checkDeps,
checkDevDeps,
checkPackageJson
Expand Down
38 changes: 33 additions & 5 deletions tests/lib/config/config-initializer.js
Expand Up @@ -32,6 +32,7 @@ describe("configInitializer", () => {
let fixtureDir,
npmCheckStub,
npmInstallStub,
npmFetchPeerDependenciesStub,
init;

const log = {
Expand Down Expand Up @@ -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);
});

Expand All @@ -83,6 +92,7 @@ describe("configInitializer", () => {
log.error.reset();
npmInstallStub.restore();
npmCheckStub.restore();
npmFetchPeerDependenciesStub.restore();
});

after(() => {
Expand Down Expand Up @@ -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", () => {
Expand All @@ -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"
]
);
});
});

Expand Down
11 changes: 11 additions & 0 deletions tests/lib/util/npm-util.js
Expand Up @@ -187,4 +187,15 @@ describe("npmUtil", () => {
stub.restore();
});
});

describe("fetchPeerDependencies()", () => {
it("should execute 'npm show --json <packageName> 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();
});
});
});

0 comments on commit 46e73ee

Please sign in to comment.