Skip to content

Commit

Permalink
Fix: wrong 'plugin-missing' error on Node.js 12 (fixes #11720)
Browse files Browse the repository at this point in the history
  • Loading branch information
mysticatea committed May 15, 2019
1 parent af81cb3 commit 4a87e6d
Show file tree
Hide file tree
Showing 13 changed files with 169 additions and 6 deletions.
58 changes: 54 additions & 4 deletions lib/cli-engine/config-array-factory.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
const fs = require("fs");
const path = require("path");
const importFresh = require("import-fresh");
const lodash = require("lodash");
const stripComments = require("strip-json-comments");
const { validateConfigSchema } = require("../config/config-validator");
const { ConfigArray, ConfigDependency, OverrideTester } = require("./config-array");
Expand All @@ -57,6 +58,15 @@ const configFilenames = [
".eslintrc",
"package.json"
];
const currentRequireStack = (() => {
const requireStack = [];

for (let cursor = module; cursor; cursor = cursor.parent) {
requireStack.push(cursor.filename || cursor.id);
}

return Object.freeze(requireStack);
})();

// Define types for VSCode IntelliSense.
/** @typedef {import("../util/types").ConfigData} ConfigData */
Expand Down Expand Up @@ -327,6 +337,46 @@ function normalizePlugin(plugin) {
};
}

/**
* Check if a given error is a `MODULE_NOT_FOUND` error.
* @param {any} error The thrown value to check.
* @param {string} request The requested module ID or file path.
* @returns {boolean} `true` if the error is the `MODULE_NOT_FOUND` of the `request`.
*/
function isModuleNotFoundError(error, request) {
if (
typeof error !== "object" ||
error === null ||
error.code !== "MODULE_NOT_FOUND" ||
typeof error.message !== "string"
) {
return false;
}

/*
* Before Node.js v12.0.0, it doesn't provide `error.requireStack`.
* Also, `Module.createRequire()` and `import-fresh` package don't provide
* the correct stack even after v12.0.0.
* In the case of either, check the error message.
*/
if (!error.requireStack || error.requireStack.length < currentRequireStack.length) {

// Remove the part `loadJSConfigFile()` prepended and the `requireStack` part.
const message = error.message.startsWith("Cannot read config file:")
? error.message.split("\n")[1]
: error.message.split("\n")[0];

return message.includes(request);
}

/*
* Since Node.js v12.0.0, the error message contains the `require` stack,
* so `error.message.includes(request)` doesn't work as expected.
* Check the `error.requireStack` is the expected `require` stack instead.
*/
return lodash.isEqual(error.requireStack, currentRequireStack);
}

//------------------------------------------------------------------------------
// Public Interface
//------------------------------------------------------------------------------
Expand Down Expand Up @@ -437,8 +487,8 @@ class ConfigArrayFactory {
} catch (error) {
if (
error.code !== "ENOENT" &&
error.code !== "MODULE_NOT_FOUND" &&
error.code !== "ESLINT_CONFIG_FIELD_NOT_FOUND"
error.code !== "ESLINT_CONFIG_FIELD_NOT_FOUND" &&
!isModuleNotFoundError(error, filePath)
) {
throw error;
}
Expand Down Expand Up @@ -703,7 +753,7 @@ class ConfigArrayFactory {
return this._loadConfigData(filePath, `${importerName} » ${request}`);
} catch (error) {
/* istanbul ignore next */
if (!error || error.code !== "MODULE_NOT_FOUND") {
if (!isModuleNotFoundError(error, request)) {
throw error;
}
}
Expand Down Expand Up @@ -848,7 +898,7 @@ class ConfigArrayFactory {
} catch (error) {
debug("Failed to load plugin '%s' declared in '%s'.", name, importerName);

if (error && error.code === "MODULE_NOT_FOUND" && error.message.includes(request)) {
if (isModuleNotFoundError(error, request)) {
error.messageTemplate = "plugin-missing";
error.messageData = {
pluginName: request,
Expand Down
10 changes: 8 additions & 2 deletions lib/util/relative-module-resolver.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,14 @@ module.exports = {
try {
return createRequire(relativeToPath).resolve(moduleName);
} catch (error) {
if (error && error.code === "MODULE_NOT_FOUND" && error.message.includes(moduleName)) {
error.message += ` relative to '${relativeToPath}'`;
if (
typeof error === "object" &&
error !== null &&
error.code === "MODULE_NOT_FOUND" &&
!error.requireStack &&
error.message.includes(moduleName)
) {
error.message += `\nRequire stack:\n- ${relativeToPath}`;
}
throw error;
}
Expand Down
1 change: 1 addition & 0 deletions tests/fixtures/module-not-found/.eslintrc.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
root: true
1 change: 1 addition & 0 deletions tests/fixtures/module-not-found/extends-js/.eslintrc.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
extends: nonexistent-config
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
extends: plugin:nonexistent-plugin/foo

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions tests/fixtures/module-not-found/plugins/.eslintrc.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
plugins: [nonexistent-plugin]
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
require("eslint/lib/util/glob-utils")
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
extends: throw
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
extends: plugin:throw/foo
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
plugins: [throw]
97 changes: 97 additions & 0 deletions tests/lib/cli-engine.js
Original file line number Diff line number Diff line change
Expand Up @@ -3031,6 +3031,103 @@ describe("CLIEngine", () => {
assert.deepStrictEqual(results[0].output, "fixed;");
});
});

describe("MODULE_NOT_FOUND error handling", () => {
const cwd = getFixturePath("module-not-found");

beforeEach(() => {
engine = new CLIEngine({ cwd });
});

it("should throw an error with a message template when 'extends' property has a non-existence JavaScript config.", () => {
try {
engine.executeOnText("test", "extends-js/test.js");
} catch (err) {
assert.strictEqual(err.messageTemplate, "extend-config-missing");
assert.deepStrictEqual(err.messageData, {
configName: "nonexistent-config"
});
return;
}
assert.fail("Expected to throw an error");
});

it("should throw an error with a message template when 'extends' property has a non-existence plugin config.", () => {
try {
engine.executeOnText("test", "extends-plugin/test.js");
} catch (err) {
assert.strictEqual(err.code, "MODULE_NOT_FOUND");
assert.strictEqual(err.messageTemplate, "plugin-missing");
assert.deepStrictEqual(err.messageData, {
importerName: `extends-plugin${path.sep}.eslintrc.yml`,
pluginName: "eslint-plugin-nonexistent-plugin",
resolvePluginsRelativeTo: cwd
});
return;
}
assert.fail("Expected to throw an error");
});

it("should throw an error with a message template when 'plugins' property has a non-existence plugin.", () => {
try {
engine.executeOnText("test", "plugins/test.js");
} catch (err) {
assert.strictEqual(err.code, "MODULE_NOT_FOUND");
assert.strictEqual(err.messageTemplate, "plugin-missing");
assert.deepStrictEqual(err.messageData, {
importerName: `plugins${path.sep}.eslintrc.yml`,
pluginName: "eslint-plugin-nonexistent-plugin",
resolvePluginsRelativeTo: cwd
});
return;
}
assert.fail("Expected to throw an error");
});

it("should throw an error with no message template when a JavaScript config threw a 'MODULE_NOT_FOUND' error.", () => {
try {
engine.executeOnText("test", "throw-in-config-itself/test.js");
} catch (err) {
assert.strictEqual(err.code, "MODULE_NOT_FOUND");
assert.strictEqual(err.messageTemplate, void 0);
return;
}
assert.fail("Expected to throw an error");
});

it("should throw an error with no message template when 'extends' property has a JavaScript config that throws a 'MODULE_NOT_FOUND' error.", () => {
try {
engine.executeOnText("test", "throw-in-extends-js/test.js");
} catch (err) {
assert.strictEqual(err.code, "MODULE_NOT_FOUND");
assert.strictEqual(err.messageTemplate, void 0);
return;
}
assert.fail("Expected to throw an error");
});

it("should throw an error with no message template when 'extends' property has a plugin config that throws a 'MODULE_NOT_FOUND' error.", () => {
try {
engine.executeOnText("test", "throw-in-extends-plugin/test.js");
} catch (err) {
assert.strictEqual(err.code, "MODULE_NOT_FOUND");
assert.strictEqual(err.messageTemplate, void 0);
return;
}
assert.fail("Expected to throw an error");
});

it("should throw an error with no message template when 'plugins' property has a plugin config that throws a 'MODULE_NOT_FOUND' error.", () => {
try {
engine.executeOnText("test", "throw-in-plugins/test.js");
} catch (err) {
assert.strictEqual(err.code, "MODULE_NOT_FOUND");
assert.strictEqual(err.messageTemplate, void 0);
return;
}
assert.fail("Expected to throw an error");
});
});
});

describe("getConfigForFile", () => {
Expand Down

0 comments on commit 4a87e6d

Please sign in to comment.