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

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

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

bradzacher
Copy link
Contributor

@bradzacher bradzacher commented Feb 21, 2024

fixes #3693

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.

Copy link

codecov bot commented Feb 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.76%. Comparing base (2124d13) to head (eb0c123).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3694   +/-   ##
=======================================
  Coverage   97.76%   97.76%           
=======================================
  Files         133      133           
  Lines        9471     9475    +4     
  Branches     3472     3472           
=======================================
+ Hits         9259     9263    +4     
  Misses        212      212           

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

@ljharb
Copy link
Member

ljharb commented Feb 21, 2024

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
Copy link
Contributor Author

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
Copy link
Member

ljharb commented Mar 3, 2024

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.

@ljharb ljharb marked this pull request as draft March 3, 2024 23:35
@jakec-dev
Copy link

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

@ljharb
Copy link
Member

ljharb commented Apr 8, 2024

@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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: flat config crash when using manual plugin def and recommended config
3 participants