[Fix] export flat configs from plugin root and fix flat config crash #3694

bradzacher · 2024-02-21T22:36:19Z

This PR exposes the flat configs at the root of the plugin:

const reactPlugin = require('eslint-plugin-react');

module.exports = [
  // ...
  reactPlugin.configs['flat/recommended'],
  // ...
];

The flat/ prefix is an approach a number of plugins are taking to enable supporting both styles at the top-level.

This was the easiest way to set things up so that the plugin reference was consistent across the root and all configs.

I'm open to a different approach to fixing the issue - for example if you want to ensure the config/ exports all work as well - it's just going to be a much larger refactor to make that work because we need to restructure the configs so there isn't a cyclic dependency between the configs and the plugin.

Personally I'm of the opinion that this "everything from the root" approach offers better DevX compared to forcing users to separately import the configs. It's a stylistic thing though - up to you really.

codecov · 2024-02-21T22:39:03Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.76%. Comparing base (393bfa2) to head (eb0c123).

❗ Current head eb0c123 differs from pull request most recent head a44e025

Please upload reports for the commit a44e025 to get more accurate results.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3694      +/-   ##
==========================================
+ Coverage   94.28%   97.76%   +3.47%     
==========================================
  Files         134      133       -1     
  Lines        9613     9475     -138     
  Branches     3486     3472      -14     
==========================================
+ Hits         9064     9263     +199     
+ Misses        549      212     -337

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

ljharb · 2024-02-21T22:53:26Z

hmm - i definitely prefer deep imports; barrel files/manifest exports cause all sorts of problems.

how would the refactor work to have flat configs not be at the root?

bradzacher · 2024-02-21T23:05:43Z

how would the refactor work to have flat configs not be at the root?

An example of a plugin that goes your deep import approach is https://github.com/eslint-community/eslint-plugin-eslint-plugin/

They define the flat configs as a function of the legacy configs.

EG

const plugin = require('../index');

module.exports = {
  plugins: { react: plugin },
  rules: plugin.configs.recommended.rules,
};

But their usecase is a bit simpler as it doesn't need to worry about language options etc.

ljharb · 2024-03-03T23:34:58Z

I've rebased this; i'm still hopeful for a way to have the root be the current legacy config, in particular to avoid a breaking change.

jakec-dev · 2024-04-07T23:30:55Z

Any update on this? This plugin is unusable with Airbnb config until this is pulled

ljharb · 2024-04-08T03:53:11Z

@jakec-dev the airbnb config doesn't use flat config, so it should be perfectly usable if you use a compatible version of eslint (not 9, yet) and the default config format of the supported eslint versions (eslintrc)

mdjermanovic · 2024-06-12T20:39:55Z

I've rebased this; i'm still hopeful for a way to have the root be the current legacy config, in particular to avoid a breaking change.

It isn't a problem for either of the two config systems (eslintrc and flat) to include both config formats in configs, with different keys of course. Eslintrc users would keep using the same configs (no breaking changes) and flat config users would be using the new configs (reactPlugin.configs['flat/recommended'] as in the original post).

mdjermanovic · 2024-06-12T20:53:22Z

On the other hand, if you prefer deep imports, a solution could be to invert 17858be: move rule configs and parserOptions back to index.js and update configs/recommended.js and others to load index.js and translate configs into the flat config format.

mdjermanovic · 2024-06-12T21:06:56Z

Also relevant: eslint/eslint#18095

ljharb · 2024-06-12T21:10:28Z

I vastly prefer deep imports, for both eslintrc and flat.

If we can use those, and ensure no breakage for existing eslintrc users, then that sounds like a great solution to get this over the line.

mdjermanovic · 2024-06-12T21:42:03Z

I vastly prefer deep imports, for both eslintrc and flat.

If we can use those, and ensure no breakage for existing eslintrc users, then that sounds like a great solution to get this over the line.

eslintrc config system doesn't provide a way for plugins to export configs other than under the configs key, so there's probably no point in adding new (deep) exports for eslintrc configs.

As for flat configs, this is doable as @bradzacher suggested in #3694 (comment). Technically, it could be implemented as suggested in #3694 (comment).

ljharb · 2024-06-12T21:43:03Z

Sounds great, let's do that :-)

If you drop a link to a sha or branch here, i can pull it into this PR

mdjermanovic · 2024-06-15T19:16:16Z

Here's the branch:

https://github.com/mdjermanovic/eslint-plugin-react/tree/fix-flat-configs

mdjermanovic@31ef12a fixes this problem.
mdjermanovic@740bf7c adds tests.

I'm not sure why languageOptions in configs were set non-enumerable, but changing that would probably be a breaking change.

ljharb · 2024-06-15T20:01:00Z

It was set that way because eslint threw when it was enumerable - i forget whether it threw with eslintrc, or with flat config, but making it nonenumerable hid it from one and allowed the other to read it.

ljharb · 2024-06-16T04:43:15Z

@mdjermanovic hm, those two commits are on top of master, not this PR, and there's a conflict. I've rebased this PR; can you stack your commits on top of it, and then I can pull them in?

mdjermanovic · 2024-06-16T08:03:31Z

@ljharb to clarify, do you want eslint-plugin-react to provide two ways to use its flat configs?

1st:

const reactRecommended = require('eslint-plugin-react/configs/recommended');

module.exports = [
  reactRecommended
];

2nd:

const reactPlugin = require('eslint-plugin-react');

module.exports = [
  reactPlugin.configs['flat/recommended']
];

ljharb · 2024-06-16T17:26:36Z

I’m comfortable with that, as long as it doesn’t introduce cycles - but it’d also be sufficient to only have the deep import way (but it should say “flat” somewhere in there)

mdjermanovic · 2024-06-16T22:12:00Z

Here's a branch on top of this PR. It keeps flat configs in plugin root (those added in this PR), fixes existing flat configs intended for deep imports, and adds tests for both:

https://github.com/mdjermanovic/eslint-plugin-react/tree/fix-deep-flat-configs

ljharb · 2024-06-18T17:58:08Z

index.js

+plugin.configs['flat/recommended'] = {
+  plugins: { react: plugin },
+  rules: plugin.configs.recommended.rules,
+  languageOptions: { parserOptions: plugin.configs.recommended.parserOptions },
+};
+
+plugin.configs['flat/all'] = {
+  plugins: { react: plugin },
+  rules: plugin.configs.all.rules,
+  languageOptions: { parserOptions: plugin.configs.all.parserOptions },
+};
+
+plugin.configs['flat/jsx-runtime'] = {
+  plugins: { react: plugin },
+  rules: plugin.configs['jsx-runtime'].rules,
+  languageOptions: { parserOptions: plugin.configs['jsx-runtime'].parserOptions },
+};


this might be more ergonomic if it's a "flat" object with 3 properties, "recommended", "all", "jsx-runtime"?

Unlike eslintrc, there are no restrictions on how flat configs can be exported from plugins. However, the convention ESLint recommends is to still export configs as properties of configs (eslint/eslint#18095).

true - if we're following that tho we'd avoid "flat" entirely too.

For destructuring, being able to do { configs: { flat: { all } } } seems much more convenient than requiring brackets and quotes, to me. Thoughts?

Sounds good to me at the moment.

In some future major version of this plugin, it would be good to switch to:

{ configs: { 'legacy/recommended' /* eslintrc */, 'recommended' /* flat */, ... } }

certainly, if indeed the ecosystem prefers flat config then there will come a time when it doesn't make sense to support anything prior to that.

Here's a commit that updates this: mdjermanovic@bbdaaa7

ljharb · 2024-06-18T17:59:04Z

tests/flat-config.js

+if (!semver.satisfies(eslintPkg.version, '>= 8.57.0')) {
+  return;
+}


flat config wasn't added in 8.57; is there a reason this isn't targeting the first version of 8 that has flat config?

No particular reason other than that 8.57 is the latest 8.x and thus is the one that's used in CI testing.

I've just tested this with v8.23.0 (the first version when flat config support is announced) and it works well, so this can be updated to '>= 8.23.0'. However, I probably don't have write access to update this PR's branch.

cool, i can do that.

Here's also a commit that updates this: mdjermanovic@2f28188

[Fix] export flat configs from plugin root and fix flat config crash

2692c07

This was referenced Feb 21, 2024

Bug: Type inference error when using tseslint.config() with eslint-plugin-react typescript-eslint/typescript-eslint#8522

Closed

Untyped JS module is not assignable to type even though it satisfies it microsoft/TypeScript#57460

Closed

bradzacher mentioned this pull request Feb 21, 2024

[Fix] add a few jsdoc type annotations to work around TS inference bugs for consumers #3695

Merged

bradzacher force-pushed the 3693-flat-config-crash branch from f826040 to 674cd1f Compare February 21, 2024 23:01

ljharb force-pushed the 3693-flat-config-crash branch from 674cd1f to eb0c123 Compare March 3, 2024 23:34

ljharb marked this pull request as draft March 3, 2024 23:35

ljharb mentioned this pull request Jun 12, 2024

[Bug]: Key "plugins": Cannot redefine plugin "react" #3765

Closed

2 tasks

ljharb force-pushed the 3693-flat-config-crash branch from eb0c123 to 2692c07 Compare June 16, 2024 04:43

mdjermanovic added 4 commits June 16, 2024 21:58

update docs

60340f8

[Fix] use same plugin instance in flat configs

4be7f0e

[Tests] test plugin in flat configs

a32c0ad

ignore flat configs in doc generator

a44e025

ljharb reviewed Jun 18, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Fix] export flat configs from plugin root and fix flat config crash #3694

[Fix] export flat configs from plugin root and fix flat config crash #3694

bradzacher commented Feb 21, 2024 •

edited

Loading

codecov bot commented Feb 21, 2024 •

edited

Loading

ljharb commented Feb 21, 2024

bradzacher commented Feb 21, 2024

ljharb commented Mar 3, 2024

jakec-dev commented Apr 7, 2024

ljharb commented Apr 8, 2024

mdjermanovic commented Jun 12, 2024

mdjermanovic commented Jun 12, 2024

mdjermanovic commented Jun 12, 2024

ljharb commented Jun 12, 2024

mdjermanovic commented Jun 12, 2024

ljharb commented Jun 12, 2024

mdjermanovic commented Jun 15, 2024

ljharb commented Jun 15, 2024

ljharb commented Jun 16, 2024

mdjermanovic commented Jun 16, 2024

ljharb commented Jun 16, 2024

mdjermanovic commented Jun 16, 2024

ljharb Jun 18, 2024

mdjermanovic Jun 18, 2024

ljharb Jun 18, 2024

mdjermanovic Jun 18, 2024

ljharb Jun 18, 2024

mdjermanovic Jun 18, 2024

ljharb Jun 18, 2024

mdjermanovic Jun 18, 2024

ljharb Jun 18, 2024

mdjermanovic Jun 18, 2024

[Fix] export flat configs from plugin root and fix flat config crash #3694

Are you sure you want to change the base?

[Fix] export flat configs from plugin root and fix flat config crash #3694

Conversation

bradzacher commented Feb 21, 2024 • edited Loading

codecov bot commented Feb 21, 2024 • edited Loading

Codecov Report

ljharb commented Feb 21, 2024

bradzacher commented Feb 21, 2024

ljharb commented Mar 3, 2024

jakec-dev commented Apr 7, 2024

ljharb commented Apr 8, 2024

mdjermanovic commented Jun 12, 2024

mdjermanovic commented Jun 12, 2024

mdjermanovic commented Jun 12, 2024

ljharb commented Jun 12, 2024

mdjermanovic commented Jun 12, 2024

ljharb commented Jun 12, 2024

mdjermanovic commented Jun 15, 2024

ljharb commented Jun 15, 2024

ljharb commented Jun 16, 2024

mdjermanovic commented Jun 16, 2024

ljharb commented Jun 16, 2024

mdjermanovic commented Jun 16, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bradzacher commented Feb 21, 2024 •

edited

Loading

codecov bot commented Feb 21, 2024 •

edited

Loading