chore: update eslint-config-eslint exports

[mdjermanovic]

Prerequisites checklist

• I have read the contributing guidelines.

What is the purpose of this pull request? (put an "X" next to an item)

[ ] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofix to a rule
[ ] Add a CLI option
[ ] Add something to the core
[x] Other, please explain:

Refs discussion in eslint/espree#577 (comment).

Currently, eslint-config-eslint exports a single config array suitable for CommonJS projects. That's fine for this repo, but most of the other repos are now ESM. Usage in an ESM repo would look like in eslint/espree#577:

import eslintConfigEslint from "eslint-config-eslint";
import nodeRecommendedModule from "eslint-plugin-n/configs/recommended-module.js";
import nodeRecommendedScript from "eslint-plugin-n/configs/recommended-script.js";

export default [
    // eslintConfigEslint[3] is eslint-plugin-n's recommended-script config
    ...eslintConfigEslint.slice(0, 3),
    {
        files: ["**/*.js"],
        ...nodeRecommendedModule
    },
    {
        files: ["**/*.cjs"],
        ...nodeRecommendedScript
    },
    ...eslintConfigEslint.slice(4)
];

That doesn't look very convenient.

What changes did you make? (Give an overview)

This PR changes eslint-config-eslint to export an object with 3 configs: base (config array), commonjs (config object), esm (config object), so that the usage would look like this:

import eslintConfigEslint from "eslint-config-eslint";

export default [
    ...eslintConfigEslint.base,
    {
        files: ["**/*.js"],
        ...eslintConfigEslint.esm
    },
    {
        files: ["**/*.cjs"],
        ...eslintConfigEslint.commonjs
    }
];

Is there anything you'd like reviewers to focus on?

Do the names make sense?

[netlify]

✅ Deploy Preview for docs-eslint canceled.

Name	Link
🔨 Latest commit	42ba8de
🔍 Latest deploy log	https://app.netlify.com/sites/docs-eslint/deploys/64a86995b4e31700083f91c2

[mdjermanovic]

I removed n/no-deprecated-api from here because I believe it's already in eslint-plugin-n's recommended configs. Is that correct?

[aladdin-add]

👍

[nzakas]

Rather than forcing us to mix and match individual configs, what if we just exported an ESM-compatible array by default and included a CommonJS one under eslint-config-eslint/cjs?

That way, the default is equivalent to what we have now and the special case of CommonJS is set off to the side.

So you'd be able to do this:

import eslintConfigESLint from "eslint-config-eslint";

export default [
    ...eslintConfigESLint
];

And then in a CommonJS file do:

const eslintConfigESLint = require("eslint-config-eslint/cjs");

module.exports = [
    ...eslintConfigESLint
];

[mdjermanovic]

These 3 rules will be in the commonjs config only because I believe they don't apply to ESM. Is that correct?

[mdjermanovic]

Rather than forcing us to mix and match individual configs, what if we just exported an ESM-compatible array by default and included a CommonJS one under eslint-config-eslint/cjs?

That way, the default is equivalent to what we have now and the special case of CommonJS is set off to the side.

That's another option, but I think the usage would be more complicated in our ESM projects, where we have .js (ESM) and .cjs files. It would be something like this:

import eslintConfigESLint from "eslint-config-eslint";
import eslintConfigESLintCJS from "eslint-config-eslint/cjs";

export default [
    ...eslintConfigESLint.map(config => ({
        files: ["**/*.js"],
        ...config
    })),
    ...eslintConfigESLintCJS.map(config => ({
        files: ["**/*.cjs"],
        ...config
    }))
];

[nzakas]

To me that seems less complicated because you don't have to mix and match a base with specific ESM or CJS configs. I'd like to plan for a future without CommonJS and design a solution that works optimally in that world even if its suboptimal in a current CommonJS repo.

[mdjermanovic]

To me that seems less complicated because you don't have to mix and match a base with specific ESM or CJS configs. I'd like to plan for a future without CommonJS and design a solution that works optimally in that world even if its suboptimal in a current CommonJS repo.

Makes sense, I'll update the exports.

[aladdin-add]

If had to choose, I would prefer the simpler way of usage. Can we export something like 4331d3c?

[mdjermanovic]

I've updated exports so that the default export is the full configuration for ESM files, and there is an additional export /cjs that is the full configuration for CJS files.

I also added package.exports and updated package.engine because some plugins require "node": ">=16.0.0".

[mdjermanovic]

If had to choose, I would prefer the simpler way of usage. Can we export something like 4331d3c?

That's another option, but since the new config system doesn't have a feature like eslintrc's overrides.extends that handles matching multiple lists, I think it should be best practice to avoid files in shareable configs.

[aladdin-add]

That's another option, but since the new config system doesn't have a feature like eslintrc's overrides.extends that handles matching multiple lists, I think it should be best practice to avoid files in shareable configs.

just wondering the “best practice” should be strictly adhered to - it's much easier for users to move the files config to shared configs:

for esm projects:

const esmConfig = require("eslint-config-eslint/esm");
module.exports = [ ...esmConfig];

for cjs projects:

const cjsConfig = require("eslint-config-eslint/cjs");
module.exports = [ ...cjsConfig];

[mdjermanovic]

The problem with files in shareable configs occurs when a shareable config should only be applied to some files. For example, in the eslint.org repo we have tools that run in Node (currently CommonJS modules but could be converted to ESM), modules that will be bundled in the Playground, and browser scripts. Assuming that we'll eventually convert tools to ESM, eslint-config-eslint's default export (or the esm export in your version) config array looks like a good candidate for those files, but not for the Playground modules and browser scripts, so it should be applied to tools/ only. If the config array contains a mix of ESM and CJS config items, restricting the config to a directory becomes complicated. I don't have a strong opinion on this though, and this is certainly a good exercise on creating and using shareable flat configs.

[aladdin-add]

well, I see your point!

in my implementation, it has exported 3 configs: base, esm, cjs. They can easily be used directly in specific types of apps.

esm/cjs: only used for node.js apps (most eslint's repos should be using it).
base: for web apps like eslint.org, a new config can be created based on base.

I am happy with the current implementation, and I am somewhat discovering the flexibility of the new config system, we may need a little time to explore the best way to use it. 😄

[aladdin-add]

LGTM, just waiting @nzakas to take another look.

[aladdin-add]

I didn't see a need to add this - we did this in the eslint package to avoid other devs using the internal modules, but this package is (mostly) only used by eslint. :)

[mdjermanovic]

I thought it would be good to clarify which modules are intended to be public.

And with this, import "eslint-config-eslint/cjs" doesn't need .js extension :)

[aladdin-add]

we need to export ./eslintrc as well - it was used in .eslintrc.js:

eslint/.eslintrc.js

Line 65 in cdd063c

"eslint/eslintrc"

[mdjermanovic]

Good catch! Updated in 6f71c8e

[nzakas]

That's another option, but since the new config system doesn't have a feature like eslintrc's overrides.extends that handles matching multiple lists, I think it should be best practice to avoid files in shareable configs

In general, I agree that a best practice would be to avoid files for any shareable configs intended to be modified and extended by other people. In our case, as we don't intend eslint-config-eslint to be used by anyone else, I think we should make it be as drop-in as possible. The fewer steps we need to make to properly use the config, the less likely we are to make mistakes.

I'm also fine with putting a big warning message on the README to this effect. :)

[mdjermanovic]

All right, I'll update the config to include files.

[mdjermanovic]

Ready for another review.

[aladdin-add]

LGTM. 👍

[nzakas]

LGTM. Thanks!