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.

Sign up for GitHub

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

Jump to bottom

feat: Avoid dirname for built-in configs #70

Closed
wants to merge 6 commits into from
Closed

feat: Avoid dirname for built-in configs #70

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a1181ef
fix: Avoid dirname for built-in configs.
zddai Feb 13, 2022
a3bfe57
Fix reviews.
zddai Feb 16, 2022
646b3a8
fix: Avoid dirname for built-in configs.
zddai Feb 13, 2022
0829fee
Merge branch 'issue15575' of https://github.com/daidodo/eslintrc into…
zddai Feb 16, 2022
8259d84
fix: Avoid dirname for built-in configs.
zddai Feb 16, 2022
af4abc5
Merge branch 'issue15575' of https://github.com/daidodo/eslintrc into…
daidodo Feb 16, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
28 changes: 16 additions & 12 deletions lib/cascading-config-array-factory.js
View file
Open in desktop
Expand Up @@ -22,13 +22,18 @@
// Requirements
//------------------------------------------------------------------------------

import debugOrig from "debug";
import os from "os";
import path from "path";

import { ConfigArrayFactory } from "./config-array-factory.js";
import {
ConfigArray,
ConfigDependency,
IgnorePattern
} from "./config-array/index.js";
import ConfigValidator from "./shared/config-validator.js";
import { emitDeprecationWarning } from "./shared/deprecation-warnings.js";
import { ConfigArrayFactory } from "./config-array-factory.js";
import { ConfigArray, ConfigDependency, IgnorePattern } from "./config-array/index.js";
import debugOrig from "debug";

const debug = debugOrig("eslintrc:cascading-config-array-factory");

Expand Down Expand Up @@ -56,8 +61,8 @@ const debug = debugOrig("eslintrc:cascading-config-array-factory");
* @property {Function} loadRules The function to use to load rules.
* @property {Map<string,Rule>} builtInRules The rules that are built in to ESLint.
* @property {Object} [resolver=ModuleResolver] The module resolver object.
* @property {string} eslintAllPath The path to the definitions for eslint:all.
* @property {string} eslintRecommendedPath The path to the definitions for eslint:recommended.
* @property {ConfigData} eslintAllConfig The config data to the definitions for eslint:all.
* @property {ConfigData} eslintRecommendedConfig The config data to the definitions for eslint:recommended.
*/

/**
Expand All @@ -77,8 +82,8 @@ const debug = debugOrig("eslintrc:cascading-config-array-factory");
* @property {Function} loadRules The function to use to load rules.
* @property {Map<string,Rule>} builtInRules The rules that are built in to ESLint.
* @property {Object} [resolver=ModuleResolver] The module resolver object.
* @property {string} eslintAllPath The path to the definitions for eslint:all.
* @property {string} eslintRecommendedPath The path to the definitions for eslint:recommended.
* @property {ConfigData} eslintAllConfig The config data to the definitions for eslint:all.
* @property {ConfigData} eslintRecommendedConfig The config data to the definitions for eslint:recommended.
*/

/** @type {WeakMap<CascadingConfigArrayFactory, CascadingConfigArrayFactoryInternalSlots>} */
Expand Down Expand Up @@ -218,17 +223,17 @@ class CascadingConfigArrayFactory {
builtInRules = new Map(),
loadRules,
resolver,
eslintRecommendedPath,
eslintAllPath
eslintRecommendedConfig,
eslintAllConfig
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I marked this PR as "breaking" (and we'll have to update the commit tag) because this change is backward-incompatible, so we'd need to bump @eslint/eslintrc to 2.0 to avoid using it from ESLint v8.0.0-v8.9.0.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As an alternative, we could keep these string options, add new ones for objects, and support both paths and objects (if the objects are passed in, then use them; otherwise, use paths). That wouldn't be a breaking change for this package. Though it would make the code a bit more complicated, and I'm not sure if anyone other than eslint/eslint is using this API.

@nzakas thoughts about this, should we keep the existing API and just add new options?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I don’t want to make any breaking changes at this point. It probably makes sense to just add new properties and throw an error if both a path and an object are passed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the comments!

I've updated the code to keep the compatibility.

} = {}) {
const configArrayFactory = new ConfigArrayFactory({
additionalPluginPool,
cwd,
resolvePluginsRelativeTo,
builtInRules,
resolver,
eslintRecommendedPath,
eslintAllPath
eslintRecommendedConfig,
eslintAllConfig
});

internalSlotsMap.set(this, {
Expand Down Expand Up @@ -516,5 +521,4 @@ class CascadingConfigArrayFactory {
//------------------------------------------------------------------------------
// Public Interface
//------------------------------------------------------------------------------

export { CascadingConfigArrayFactory };
37 changes: 18 additions & 19 deletions lib/config-array-factory.js
View file
Open in desktop
Expand Up @@ -38,22 +38,23 @@
// Requirements
//------------------------------------------------------------------------------

import debugOrig from "debug";
import fs from "fs";
import path from "path";
import importFresh from "import-fresh";
import { createRequire } from "module";
import path from "path";
import stripComments from "strip-json-comments";
import ConfigValidator from "./shared/config-validator.js";
import * as naming from "./shared/naming.js";
import * as ModuleResolver from "./shared/relative-module-resolver.js";

import {
ConfigArray,
ConfigDependency,
IgnorePattern,
OverrideTester
} from "./config-array/index.js";
import debugOrig from "debug";
import ConfigValidator from "./shared/config-validator.js";
import * as naming from "./shared/naming.js";
import * as ModuleResolver from "./shared/relative-module-resolver.js";

import { createRequire } from "module";
const require = createRequire(import.meta.url);

const debug = debugOrig("eslintrc:config-array-factory");
Expand Down Expand Up @@ -89,8 +90,8 @@ const configFilenames = [
* @property {string} [resolvePluginsRelativeTo] A path to the directory that plugins should be resolved from. Defaults to `cwd`.
* @property {Map<string,Rule>} builtInRules The rules that are built in to ESLint.
* @property {Object} [resolver=ModuleResolver] The module resolver object.
* @property {string} eslintAllPath The path to the definitions for eslint:all.
* @property {string} eslintRecommendedPath The path to the definitions for eslint:recommended.
* @property {import("./shared/types").ConfigData} eslintAllConfig The config data to the definitions for eslint:all.
* @property {import("./shared/types").ConfigData} eslintRecommendedConfig The config data to the definitions for eslint:recommended.
*/

/**
Expand All @@ -100,8 +101,8 @@ const configFilenames = [
* @property {string | undefined} resolvePluginsRelativeTo An absolute path the the directory that plugins should be resolved from.
* @property {Map<string,Rule>} builtInRules The rules that are built in to ESLint.
* @property {Object} [resolver=ModuleResolver] The module resolver object.
* @property {string} eslintAllPath The path to the definitions for eslint:all.
* @property {string} eslintRecommendedPath The path to the definitions for eslint:recommended.
* @property {import("./shared/types").ConfigData} eslintAllConfig The config data to the definitions for eslint:all.
* @property {import("./shared/types").ConfigData} eslintRecommendedConfig The config data to the definitions for eslint:recommended.
*/

/**
Expand Down Expand Up @@ -427,8 +428,8 @@ class ConfigArrayFactory {
resolvePluginsRelativeTo,
builtInRules,
resolver = ModuleResolver,
eslintAllPath,
eslintRecommendedPath
eslintAllConfig,
eslintRecommendedConfig
} = {}) {
internalSlotsMap.set(this, {
additionalPluginPool,
Expand All @@ -438,8 +439,8 @@ class ConfigArrayFactory {
path.resolve(cwd, resolvePluginsRelativeTo),
builtInRules,
resolver,
eslintAllPath,
eslintRecommendedPath
eslintAllConfig,
eslintRecommendedConfig
});
}

Expand Down Expand Up @@ -797,19 +798,17 @@ class ConfigArrayFactory {
* @private
*/
_loadExtendedBuiltInConfig(extendName, ctx) {
const { eslintAllPath, eslintRecommendedPath } = internalSlotsMap.get(this);
const { eslintAllConfig, eslintRecommendedConfig } = internalSlotsMap.get(this);

if (extendName === "eslint:recommended") {
return this._loadConfigData({
return this._normalizeConfigData(eslintRecommendedConfig, {
...ctx,
filePath: eslintRecommendedPath,
name: `${ctx.name} » ${extendName}`
});
}
if (extendName === "eslint:all") {
return this._loadConfigData({
return this._normalizeConfigData(eslintAllConfig, {
...ctx,
filePath: eslintAllPath,
name: `${ctx.name} » ${extendName}`
});
}
Expand Down
13 changes: 6 additions & 7 deletions lib/flat-compat.js
View file
Open in desktop
Expand Up @@ -7,14 +7,13 @@
// Requirements
//-----------------------------------------------------------------------------

import path from "path";
import { fileURLToPath } from "url";
import createDebug from "debug";
import path from "path";

import { ConfigArrayFactory } from "./config-array-factory.js";
import environments from "../conf/environments.js";

const dirname = path.dirname(fileURLToPath(import.meta.url));
import eslintAllConfig from "../conf/eslint-all.cjs";
import eslintRecommendedConfig from "../conf/eslint-recommended.cjs";
import { ConfigArrayFactory } from "./config-array-factory.js";

//-----------------------------------------------------------------------------
// Helpers
Expand Down Expand Up @@ -225,8 +224,8 @@ class FlatCompat {
this[cafactory] = new ConfigArrayFactory({
cwd: baseDirectory,
resolvePluginsRelativeTo,
eslintAllPath: path.resolve(dirname, "../conf/eslint-all.cjs"),
eslintRecommendedPath: path.resolve(dirname, "../conf/eslint-recommended.cjs")
eslintAllConfig,
eslintRecommendedConfig
});
}

Expand Down
1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Expand Up @@ -50,6 +50,7 @@
},
"homepage": "https://github.com/eslint/eslintrc#readme",
"devDependencies": {
"@rollup/plugin-commonjs": "^21.0.1",
"c8": "^7.7.3",
"chai": "^4.3.4",
"eslint": "^7.31.0",
Expand Down
8 changes: 6 additions & 2 deletions rollup.config.js
View file
Open in desktop
@@ -1,3 +1,5 @@
import commonjs from "@rollup/plugin-commonjs";

export default [
{
input: "./lib/index.js",
Expand All @@ -12,7 +14,8 @@ export default [
file: "dist/eslintrc.cjs",
sourcemap: true,
freeze: false
}
},
plugins: [commonjs()]
},
{
input: "./lib/index-universal.js",
Expand All @@ -27,6 +30,7 @@ export default [
file: "dist/eslintrc-universal.cjs",
sourcemap: true,
freeze: false
}
},
plugins: [commonjs()]
}
];