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: migrate to FlatConfig #64

Merged
merged 17 commits into from
Jan 15, 2025

Conversation

yoshinorin
Copy link
Member

@yoshinorin yoshinorin commented Sep 20, 2024

Refs: #55

check list

  • Add test cases for the changes.
  • Passed the CI test.

Usage

// Before FlatConfig
{
  "root": true,
  "extends": "hexo/ts.js",
  "parserOptions": {
    "sourceType": "module",
    "ecmaVersion": 2020
  },
  "rules": {
    "@typescript-eslint/no-explicit-any": 0,
    "@typescript-eslint/ban-ts-comment": 0,
    "@typescript-eslint/no-this-alias": 0
  }
}
// After
// In `eslint.config.js`
const hexoTsLint = require('eslint-config-hexo/ts');

module.exports = [
  ...hexoTsLint,
  {
    languageOptions: {
      ecmaVersion: 2022,
    },
    rules: {
      "@typescript-eslint/no-explicit-any": 0,
      "@typescript-eslint/ban-ts-comment": 0,
      "@typescript-eslint/no-this-alias": 0
    }
  }
];

@yoshinorin yoshinorin assigned yoshinorin and unassigned yoshinorin Sep 20, 2024
eslint.js Show resolved Hide resolved
package.json Outdated
"@eslint/js": "^9.10.0",
"eslint-plugin-n": "^17.10.3",
"globals": "^15.9.0",
"typescript-eslint": "^8.6.0"
Copy link
Member

@SukkaW SukkaW Sep 21, 2024

Choose a reason for hiding this comment

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

Since we are creating new presets, it is a good chance for us to add a few more presets. E.g. we could add https://www.npmjs.com/package/eslint-plugin-mocha since almost all Hexo repos use mocha.

Note that we might need to disable a few rules (E.g. mocha/no-mocha-arrows since we all use arrow functions).

Copy link
Member

@uiolee uiolee Oct 4, 2024

Choose a reason for hiding this comment

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

need "type": "module", or name js file to mjs

Oops! Something went wrong! :(

ESLint: 9.11.1

/home/user/eslint-config-hexo/eslint.js:1
import nodePlugin from 'eslint-plugin-n';
^^^^^^

SyntaxError: Cannot use import statement outside a module
    at internalCompileFunction (node:internal/vm:77:18)
    at wrapSafe (node:internal/modules/cjs/loader:1288:20)

Copy link
Member

@SukkaW SukkaW Oct 4, 2024

Choose a reason for hiding this comment

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

Then keep using const = require then. In the ESLint world, CommonJS is still the first-class citizen.

eslint.js Outdated Show resolved Hide resolved
ts.js Outdated Show resolved Hide resolved
ts-test.js Outdated Show resolved Hide resolved
ts.js Outdated Show resolved Hide resolved
@yoshinorin yoshinorin changed the title WIP: feat: migrate to FlatConfig feat: migrate to FlatConfig Oct 30, 2024
@yoshinorin yoshinorin marked this pull request as ready for review October 30, 2024 15:57
@yoshinorin
Copy link
Member Author

@hexojs/core
Sorry for the delay in implementing the changes (I'm currently very busy with personal matters...) and perhaps ready for review.

Thanks :)

@stevenjoezhang
Copy link
Member

Tested in hexojs/hexo-renderer-marked#306

@stevenjoezhang
Copy link
Member

Some tests failed here: hexojs/hexo#5599
Need fix

@yoshinorin
Copy link
Member Author

Some tests failed here: hexojs/hexo#5599
Need fix

Coverage collection has failed, but all other tests have passed. The eslint changes are unrelated to the test results.

test.js Outdated

const mochaConfig = {
...mochaPlugin.configs.flat.recommended,
rules: {
Copy link
Member

@uiolee uiolee Jan 2, 2025

Choose a reason for hiding this comment

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

this will overwrite the rules in mochaPlugin.configs.flat.recommended, please make sure this meets your expectations

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, this override is correct. I read the rules in README and configured the overrides to ignore specific settings in our test code. For example, we use allow functions in our test code, but in the eslint-plugin-mocha recommended configuration, this is a warning by default.

test.js Outdated
@@ -1,9 +1,29 @@
module.exports = {
extends: './eslint.js',
const eslint = require('./eslint.js');
Copy link
Member

Choose a reason for hiding this comment

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

Such naming may be confusing

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.
0544bdd

Personally, I think it would be better to rename the file itself from eslint.js. If we were to align it with other file names, it would be js.js, but I don't think that’s a good choice for naming.

@yoshinorin yoshinorin merged commit ea0ca0d into hexojs:master Jan 15, 2025
@yoshinorin yoshinorin deleted the chore/migrate-to-flatconfig branch January 15, 2025 14:20
@yoshinorin yoshinorin mentioned this pull request Jan 15, 2025
2 tasks
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.

5 participants