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

feat: export flat/recommended-script and flat/recommended-module #113

Merged
merged 1 commit into from
Sep 8, 2023

Conversation

aladdin-add
Copy link

@aladdin-add aladdin-add commented Aug 25, 2023

exporting flat configs as flat/*, e.g. flat/recommended-module. it avoids users having to access the internal modules to get the flat configs.

before:

const esmConfig = require("eslint-plugin-n/configs/recommended-module");

after:

const esmConfig = require("eslint-plugin-n").configs["flat/recommended-module"];

@aladdin-add aladdin-add force-pushed the feat/re-export-flat-configs branch 3 times, most recently from 44040a8 to 3c9648d Compare August 28, 2023 06:08
@aladdin-add aladdin-add requested a review from a team August 28, 2023 06:11
@aladdin-add aladdin-add marked this pull request as ready for review August 28, 2023 06:11
Comment on lines +8 to +9
["flat/recommended-script", "✅"],
["flat/recommended-module", "🟢"],
Copy link
Author

Choose a reason for hiding this comment

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

@bmish I am uncertain about the optimal approach here - having the same rules in both flat config and eslintrc config. Therefore, it may not be necessary to duplicate emojis. thoughts?

Copy link
Member

@bmish bmish Aug 28, 2023

Choose a reason for hiding this comment

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

To make sure I have the right context, is exporting a config twice (under one name for eslintrc, and under another name for flat config) the recommended approach to support both kinds of configs? Does ESLint mention a recommendation about this somewhere?

Using the same emoji for multiple configs, even if they are intended to represent a single config, seems confusing and breaks assumptions that each config has a unique emoji.

Some options to handle this:

  1. eslint-doc-generator should not allow the same emoji for multiple configs. Perhaps we can recommend using the same emoji but different colors for eslintrc vs. flat?
  2. eslint-doc-generator allows the same emoji for multiple configs, but better display/handling (e.g. avoid repeating the emoji in the readme rules table).
  3. eslint-doc-generator adds some special handling/representation to indicate that one config is the flat version of an eslintrc config. But not sure how this can be reliably detected.

I personally prefer 1 as of now.

Related: bmish/eslint-doc-generator#376

Copy link
Author

@aladdin-add aladdin-add Aug 29, 2023

Choose a reason for hiding this comment

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

it's not something recommended by eslint - The examples in the eslint documentation use eslint-plugin-jsdoc, which is how this plugin is exported. Personally, I'd like to stay as close as possible to the usage in the documentation, or do you have a better recommendation?

eslint/eslint@056499d

Copy link
Author

@aladdin-add aladdin-add Aug 29, 2023

Choose a reason for hiding this comment

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

I prefer 2>1>3.

for eslint plugins, there is not too much special eslintrc configs vs. flat configs. we just need to enhance the eslint-doc-generator configs.

…recommended-script, and flat/mixed-esm-and-cjs

it avoid users having to access the internal modules to get the flat configs.
@aladdin-add aladdin-add changed the title feat: export flat configs in the entry feat: export flat configs flat/recommended-script and flat/recommended-module Sep 8, 2023
@aladdin-add aladdin-add changed the title feat: export flat configs flat/recommended-script and flat/recommended-module feat: export flat/recommended-script and flat/recommended-module Sep 8, 2023
@aladdin-add aladdin-add merged commit 1f8fdc8 into master Sep 8, 2023
9 checks passed
@aladdin-add aladdin-add deleted the feat/re-export-flat-configs branch September 8, 2023 06:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants