Skip to content

feat!: Switch to flat config - #232

Merged
mdjermanovic merged 21 commits into
mainfrom
issue231
Feb 15, 2024
Merged

feat!: Switch to flat config#232
mdjermanovic merged 21 commits into
mainfrom
issue231

Conversation

@nzakas

@nzakas nzakas commented Jan 31, 2024

Copy link
Copy Markdown
Member
  • Switch recommended config to flat config format
  • Rename old recommended config to recommended-legacy
  • Updated documentation
  • Upgraded ESLint and associated plugins
  • Updated tests

fixes #231

- Switch recommended config to flat config format
- Rename old recommended config to recommended-legacy
- Updated documentation
- Upgraded ESLint
- Updated tests

fixes #231
@nzakas

nzakas commented Feb 2, 2024

Copy link
Copy Markdown
Member Author

Ping @eslint/eslint-team

Comment thread README.md
### Installing

Install the plugin alongside ESLint v6 or greater:
Install the plugin alongside ESLint v8 or greater:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Which versions of ESLint will be officially supported? Per this sentence and the CI update, it looks like only >=8. However, there are still instructions for v6 and v7 below, and package.json still has "peerDependencies": { "eslint": "^6.0.0 || ^7.0.0 || ^8.0.0" } .

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'm going to update the supported ESLint versions in another PR.

Comment thread README.md Outdated
Comment thread README.md
Comment thread package.json
Comment thread .github/workflows/ci.yml
eslint: [6, 7, 8]
node: [12.22.0, 14, 16, 17, 18, 19, 20, 21]
eslint: [8]
node: [16, 17, 18, 19, 20, 21]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this also dropped some node.js/eslint versions support. To reflect this in the generated changelog:

  1. update the PR title.
  2. put these changes to a separate PR.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

These changes are necessary to make CI pass. I'll officially change the Node.js supported versions in a separate PR.

Comment thread .github/workflows/ci.yml
matrix:
os: [ubuntu-latest]
eslint: [6, 7, 8]
node: [12.22.0, 14, 16, 17, 18, 19, 20, 21]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If we want to drop support for Node < 16, we should also update the engines field in package.json.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

As previously mentioned, I'm going to update Node.js support in a separate PR so it comes out in the changelog.

Comment thread README.md Outdated
nzakas and others added 3 commits February 5, 2024 13:12
Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
Co-authored-by: 唯然 <hh_2013@foxmail.com>
Comment thread README.md Outdated
Comment thread README.md Outdated
Comment thread README.md Outdated
Comment thread package.json Outdated
Comment thread package.json Outdated
Comment thread tests/lib/plugin.js Outdated
// the plugin, so we need to make sure it's resolvable and link it
// if not.

// eslint-disable-next-line n/no-missing-require -- Known possible failure.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Now that the eslint config for this project enables reporting unused disable directives, this directive is reported when linting is run after npm test, which is confusing. It might be best to allow requiring eslint-plugin-markdown from this file and remove this directive (and the same one on line 1065).

// in eslint.config.js
{
    files: ["tests/lib/plugin.js"],
    rules: {
        "n/no-missing-require": ["error", {
            allowModules: ["eslint-plugin-markdown"]
        }]
    }
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Actually, we can now just remove the whole try-catch (lines 73-95 and 1059-1081) as there's no need to install eslint-plugin-markdown in node_modules.

As of ESLint v8.1.0 (eslint/eslint@a1f7ad7), plugin implementations passed to the constructor are used when resolving extends:["plugin:plugin-name/config-name"] as well. In this case, eslint-plugin-markdown implementation we're already passing to the ESLint constructor will be used to get the "plugin:markdown/recommended-legacy" config from.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Ah good catch!

Comment thread examples/react/eslint.config.js Outdated
Comment thread examples/typescript/eslint.config.js Outdated
Comment thread examples/react/eslint.config.js Outdated
module.exports = [
js.configs.recommended,
...markdown.configs.recommended,
...compat.extends("plugin:react/recommended"),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

As of v7.32.0, eslint-plugin-react has an additional export for the recommended config in flat config format, so we could use that for this example:

require('eslint-plugin-react/configs/recommended') // this is a flat config object

Comment thread tests/lib/plugin.js Outdated
cwd: path.resolve(__dirname, "../fixtures/"),
ignore: false,
overrideConfigFile: path.resolve(__dirname, "../fixtures/", fixtureConfigName),
plugins: { markdown: plugin },

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
plugins: { markdown: plugin },

This doesn't seem necessary as the config files load the plugin.

nzakas and others added 10 commits February 9, 2024 13:42
Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
Comment thread tests/lib/plugin.js Outdated
Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>

@mdjermanovic mdjermanovic left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thanks!

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Flat config

3 participants