Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New: add --resolve-plugins-relative-to flag #11696

Merged
merged 2 commits into from May 11, 2019
Merged
Changes from 1 commit
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -355,6 +355,7 @@ The `CLIEngine` is a constructor, and you can create a new instance by passing i
* `parserOptions` - An object containing parser options (default: empty object). Corresponds to `--parser-options`.
* `plugins` - An array of plugins to load (default: empty array). Corresponds to `--plugin`.
* `reportUnusedDisableDirectives` - When set to `true`, adds reported errors for unused `eslint-disable` directives when no problems would be reported in the disabled area anyway (default: false). Corresponds to `--report-unused-disable-directives`.
* `resolvePluginsRelativeTo` - Determines the folder where plugins should be resolved from. Should be used when an integration installs plugins and uses those plugins to lint code on behalf of the end user. Corresponds to `--resolve-plugins-relative-to`.
* `rulePaths` - An array of directories to load custom rules from (default: empty array). Corresponds to `--rulesdir`.
* `rules` - An object of rules to use (default: null). Corresponds to `--rule`.
* `useEslintrc` - Set to false to disable use of `.eslintrc` files (default: true). Corresponds to `--no-eslintrc`.
@@ -37,6 +37,7 @@ Basic configuration:
--global [String] Define global variables
--parser String Specify the parser to be used
--parser-options Object Specify parser options
--resolve-plugins-relative-to path::String A folder where plugins should be resolved from, CWD by default
Specifying rules and plugins:
--rulesdir [path::String] Use additional rules from this directory
@@ -164,6 +165,13 @@ Examples:
echo '3 ** 4' | eslint --stdin --parser-options=ecmaVersion:6 # will fail with a parsing error
echo '3 ** 4' | eslint --stdin --parser-options=ecmaVersion:7 # succeeds, yay!

#### `--resolve-plugins-relative-to`

Changes the folder where plugins are resolved from. By default, plugins are resolved from the current working directory. This option should be used when plugins were installed by someone other than the end user. It should be set to the project directory of the project that has a dependency on the necessary plugins. For example:

* When using a config file that is located outside of the current project (with the `--config` flag), if the config uses plugins which are installed locally to itself, `--resolve-plugins-relative-to` should be set to the directory containing the config file.
* If an integration has dependencies on ESLint and a set of plugins, and the tool invokes ESLint on behalf of the user with a preset configuration, the tool should set `--resolve-plugins-relative-to` to the top-level directory of the tool.

### Specifying rules and plugins

#### `--rulesdir`
@@ -90,12 +90,12 @@ In rare cases (if you were relying on the previous behavior where `eslint:recomm

Previously, ESLint loaded plugins relative to the location of the ESLint package itself. As a result, we suggested that users with global ESLint installations should also install plugins globally, and users with local ESLint installations should install plugins locally. However, due to a design bug, this strategy caused ESLint to randomly fail to load plugins and shareable configs under certain circumstances, particularly when using package management tools like [`lerna`](https://github.com/lerna/lerna) and [Yarn Plug n' Play](https://yarnpkg.com/lang/en/docs/pnp/).

As a rule of thumb: With ESLint v6, plugins should always be installed locally, even if ESLint was installed globally. More precisely, ESLint v6 always resolves plugins relative to the end user's project, and always resolves shareable configs and parsers relative to the location of the config file that imports them.

<!-- FIXME: Update this section with workarounds for global eslint/global config file setups if https://github.com/eslint/rfcs/pull/18 is accepted -->
As a rule of thumb: With ESLint v6, plugins should always be installed locally, even if ESLint was installed globally. More precisely, ESLint v6 resolves plugins relative to the end user's project by default, and always resolves shareable configs and parsers relative to the location of the config file that imports them.

**To address:** If you use a global installation of ESLint (e.g. installed with `npm install eslint --global`) along with plugins, you should install those plugins locally in the projects where you run ESLint. If your config file extends shareable configs and/or parsers, you should ensure that those packages are installed as dependencies of the project containing the config file.

If you use a config file located outside of a local project (with the `--config` flag), consider installing the plugins as dependencies of that config file, and setting the [`--resolve-plugins-relative-to`](./command-line-interface#--resolve-plugins-relative-to) flag to the location of the config file.

**Related issue(s):** [eslint/eslint#10125](https://github.com/eslint/eslint/issues/10125), [eslint/rfcs#7](https://github.com/eslint/rfcs/pull/7)

## <a name="espree-validation"></a> The default parser now validates options more strictly
@@ -73,6 +73,7 @@ const validFixTypes = new Set(["problem", "suggestion", "layout"]);
* @property {string[]} rulePaths An array of directories to load custom rules from.
* @property {boolean} reportUnusedDisableDirectives `true` adds reports for unused eslint-disable directives
* @property {boolean} globInputPaths Set to false to skip glob resolution of input file paths to lint (default: true). If false, each input file paths is assumed to be a non-glob path to an existing file.
* @property {string} resolvePluginsRelativeTo The folder where plugins should be resolved from, defaulting to the CWD
*/

/**
@@ -540,6 +541,7 @@ class CLIEngine {
baseConfig: options.baseConfig || null,
cliConfig: createConfigDataFromOptions(options),
cwd: options.cwd,
resolvePluginsRelativeTo: options.resolvePluginsRelativeTo,
rulePaths: options.rulePaths,
specificConfigPath: options.configFile,
useEslintrc: options.useEslintrc
@@ -177,13 +177,15 @@ class CascadingConfigArrayFactory {
baseConfig: baseConfigData = null,
cliConfig: cliConfigData = null,
cwd = process.cwd(),
resolvePluginsRelativeTo = cwd,
rulePaths = [],
specificConfigPath = null,
useEslintrc = true
} = {}) {
const configArrayFactory = new ConfigArrayFactory({
additionalPluginPool,
cwd
cwd,
resolvePluginsRelativeTo
});

internalSlotsMap.set(this, {
@@ -71,12 +71,14 @@ const configFilenames = [
* @typedef {Object} ConfigArrayFactoryOptions
* @property {Map<string,Plugin>} [additionalPluginPool] The map for additional plugins.
* @property {string} [cwd] The path to the current working directory.
* @property {string} [resolvePluginsRelativeTo] A path to the directory that plugins should be resolved from. Defaults to `cwd`.
*/

/**
* @typedef {Object} ConfigArrayFactoryInternalSlots
* @property {Map<string,Plugin>} additionalPluginPool The map for additional plugins.
* @property {string} cwd The path to the current working directory.
* @property {string} resolvePluginsRelativeTo An absolute path the the directory that plugins should be resolved from.
*/

/** @type {WeakMap<ConfigArrayFactory, ConfigArrayFactoryInternalSlots>} */
@@ -340,9 +342,10 @@ class ConfigArrayFactory {
*/
constructor({
additionalPluginPool = new Map(),
cwd = process.cwd()
cwd = process.cwd(),
resolvePluginsRelativeTo = cwd
} = {}) {
internalSlotsMap.set(this, { additionalPluginPool, cwd });
internalSlotsMap.set(this, { additionalPluginPool, cwd, resolvePluginsRelativeTo: path.resolve(cwd, resolvePluginsRelativeTo) });
}

/**
@@ -791,7 +794,7 @@ class ConfigArrayFactory {
_loadPlugin(name, importerPath, importerName) {
debug("Loading plugin %j from %s", name, importerPath);

const { additionalPluginPool, cwd } = internalSlotsMap.get(this);
const { additionalPluginPool, resolvePluginsRelativeTo } = internalSlotsMap.get(this);
const request = naming.normalizePackageName(name, "eslint-plugin");
const id = naming.getShorthandName(request, "eslint-plugin");

@@ -829,8 +832,8 @@ class ConfigArrayFactory {

try {

// Resolve the plugin file relative to the project root.
const relativeTo = path.join(cwd, "__placeholder__.js");
// Resolve the plugin file
const relativeTo = path.join(resolvePluginsRelativeTo, "__placeholder__.js");
const filePath = ModuleResolver.resolve(request, relativeTo);

writeDebugLogForLoading(request, relativeTo, filePath);
@@ -849,7 +852,7 @@ class ConfigArrayFactory {
error.messageTemplate = "plugin-missing";
error.messageData = {
pluginName: request,
pluginRootPath: cwd,
resolvePluginsRelativeTo,
importerName
};
}
@@ -66,7 +66,8 @@ function translateOptions(cliOptions) {
fix: (cliOptions.fix || cliOptions.fixDryRun) && (cliOptions.quiet ? quietFixPredicate : true),
fixTypes: cliOptions.fixType,
allowInlineConfig: cliOptions.inlineConfig,
reportUnusedDisableDirectives: cliOptions.reportUnusedDisableDirectives
reportUnusedDisableDirectives: cliOptions.reportUnusedDisableDirectives,
resolvePluginsRelativeTo: cliOptions.resolvePluginsRelativeTo
};
}

@@ -64,6 +64,11 @@ module.exports = optionator({
type: "Object",
description: "Specify parser options"
},
{
option: "resolve-plugins-relative-to",
type: "path::String",
description: "A folder where plugins should be resolved from, CWD by default"
},
{
heading: "Specifying rules and plugins"
},
@@ -1,6 +1,6 @@
ESLint couldn't find the plugin "<%- pluginName %>".

(The package "<%- pluginName %>" was not found when loaded as a Node module from the directory "<%- pluginRootPath %>".)
(The package "<%- pluginName %>" was not found when loaded as a Node module from the directory "<%- resolvePluginsRelativeTo %>".)

It's likely that the plugin isn't installed correctly. Try reinstalling by running the following:

Some generated files are not rendered by default. Learn more.

@@ -2079,6 +2079,24 @@ describe("CLIEngine", () => {
assert.strictEqual(report.results[0].messages.length, 2);
assert.strictEqual(report.results[0].messages[0].ruleId, "test/example-rule");
});

it("should load plugins from the `loadPluginsRelativeTo` directory, if specified", () => {
engine = new CLIEngine({
resolvePluginsRelativeTo: getFixturePath("plugins"),
baseConfig: {
plugins: ["with-rules"],
rules: { "with-rules/rule1": "error" }
},
useEslintrc: false
});

const report = engine.executeOnText("foo");

assert.strictEqual(report.results.length, 1);
assert.strictEqual(report.results[0].messages.length, 1);
assert.strictEqual(report.results[0].messages[0].ruleId, "with-rules/rule1");
assert.strictEqual(report.results[0].messages[0].message, "Rule report from plugin");
});
});

describe("cache", () => {
@@ -1369,7 +1369,7 @@ describe("ConfigArrayFactory", () => {
assert.strictEqual(err.messageTemplate, "plugin-missing");
assert.deepStrictEqual(err.messageData, {
pluginName: "eslint-plugin-nonexistent-plugin",
pluginRootPath: process.cwd(),
resolvePluginsRelativeTo: process.cwd(),
importerName: "whatever"
});
return;
@@ -1386,7 +1386,7 @@ describe("ConfigArrayFactory", () => {
assert.strictEqual(err.messageTemplate, "plugin-missing");
assert.deepStrictEqual(err.messageData, {
pluginName: "eslint-plugin-nonexistent-plugin",
pluginRootPath: process.cwd(),
resolvePluginsRelativeTo: process.cwd(),
importerName: "whatever"
});
return;
@@ -2096,10 +2096,11 @@ describe("ConfigArrayFactory", () => {
/**
* Load a plugin.
* @param {string} request A request to load a plugin.
* @param {ConfigArrayFactory} [configArrayFactory] The factory to use
* @returns {Map<string,Object>} The loaded plugins.
*/
function load(request) {
const config = factory.create({ plugins: [request] });
function load(request, configArrayFactory = factory) {
const config = configArrayFactory.create({ plugins: [request] });

return new Map(
Object
@@ -2122,6 +2123,26 @@ describe("ConfigArrayFactory", () => {
);
});

it("should load a plugin when referenced by short name, even when using a custom loadPluginsRelativeTo value", () => {
const { ConfigArrayFactory: FactoryWithPluginsInSubdir } = defineConfigArrayFactoryWithInMemoryFileSystem({
cwd: () => tempDir,
files: {
"subdir/node_modules/@scope/eslint-plugin-example/index.js": "exports.configs = { name: '@scope/eslint-plugin-example' };",
"subdir/node_modules/eslint-plugin-example/index.js": "exports.configs = { name: 'eslint-plugin-example' };",
"subdir/node_modules/eslint-plugin-throws-on-load/index.js": "throw new Error('error thrown while loading this module')"
This conversation was marked as resolved by not-an-aardvark

This comment has been minimized.

Copy link
@mysticatea

mysticatea May 10, 2019

Member

Would you remove unused files?

}
});

const factoryWithCustomPluginPath = new FactoryWithPluginsInSubdir({ resolvePluginsRelativeTo: "subdir" });

const loadedPlugins = load("example", factoryWithCustomPluginPath);

assertPluginDefinition(
loadedPlugins.get("example"),
{ configs: { name: "eslint-plugin-example" } }
);
});

it("should load a plugin when referenced by long name", () => {
const loadedPlugins = load("eslint-plugin-example");

@@ -588,6 +588,16 @@ describe("cli", () => {

});

describe("when supplied with a plugin-loading path", () => {
it("should pass the option to CLIEngine", () => {
const examplePluginDirPath = "foo/bar";

verifyCLIEngineOpts(`--resolve-plugins-relative-to ${examplePluginDirPath} foo.js`, {
resolvePluginsRelativeTo: examplePluginDirPath
});
});
});

describe("when given an parser name", () => {
it("should exit with a fatal error if parser is invalid", () => {
const filePath = getFixturePath("passing.js");
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.