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

infra: use eslint.config.ts #3044

Merged
merged 6 commits into from
Sep 16, 2024
Merged

Conversation

Shinigami92
Copy link
Member

@Shinigami92 Shinigami92 commented Aug 10, 2024

This PR switched to eslint.config.ts so we can benefit from TypeScript checks and don't need to rely anymore on a // @ts-check comment at the top of the file

Also it serves as a one-of-the-first examples in big Open Source adopting it and ensure stability for eslint/eslint#18134


Right now we CANT merge this, because VSCode extension is not possible to pick-up the new eslint.config.ts file

Also the feature should be stable and not require a flag

@Shinigami92 Shinigami92 added p: 1-normal Nothing urgent do NOT merge yet Do not merge this PR into the target branch yet c: infra Changes to our infrastructure or project setup labels Aug 10, 2024
@Shinigami92 Shinigami92 added this to the vAnytime milestone Aug 10, 2024
@Shinigami92 Shinigami92 self-assigned this Aug 10, 2024
Copy link

netlify bot commented Aug 10, 2024

Deploy Preview for fakerjs ready!

Name Link
🔨 Latest commit 1dd9db3
🔍 Latest deploy log https://app.netlify.com/sites/fakerjs/deploys/66e6fe720038a500087bda6c
😎 Deploy Preview https://deploy-preview-3044.fakerjs.dev
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@Shinigami92 Shinigami92 changed the base branch from next to renovate/eslint August 10, 2024 06:24
Copy link

codecov bot commented Aug 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.97%. Comparing base (15fd536) to head (1dd9db3).
Report is 1 commits behind head on next.

Additional details and impacted files
@@           Coverage Diff            @@
##             next    #3044    +/-   ##
========================================
  Coverage   99.97%   99.97%            
========================================
  Files        2776     2776            
  Lines      226262   226262            
  Branches      942      591   -351     
========================================
+ Hits       226199   226204     +5     
+ Misses         63       58     -5     

see 1 file with indirect coverage changes

@Shinigami92
Copy link
Member Author

Somehow there is a problem with the deprecation plugin (again) 👀
This is a bit confusing, as I thought that jiti will only strip types and don't try to process the code. But maybe I mixed that up with the new Node v22.6 feature 🤔

https://github.com/faker-js/faker/actions/runs/10329602297/job/28597594550?pr=3044#step:6:16

> @faker-js/faker@9.0.0-rc.0 lint /home/runner/work/faker/faker
> eslint --cache --cache-strategy content --flag unstable_ts_config .


Oops! Something went wrong! :(

ESLint: 9.9.0

TypeError: Cannot read properties of undefined (reading 'rules')
    at fixupPluginRules (/home/runner/work/faker/faker/node_modules/.pnpm/@eslint+compat@1.1.1/node_modules/@eslint/compat/dist/cjs/index.cjs:214:44)
    at /home/runner/work/faker/faker/eslint.config.ts:137:36
    at evalModule (/home/runner/work/faker/faker/node_modules/.pnpm/jiti@1.21.6/node_modules/jiti/dist/jiti.js:1:247313)
    at jiti (/home/runner/work/faker/faker/node_modules/.pnpm/jiti@1.21.6/node_modules/jiti/dist/jiti.js:1:245241)
    at /home/runner/work/faker/faker/node_modules/.pnpm/jiti@1.21.6/node_modules/jiti/dist/jiti.js:1:24[8](https://github.com/faker-js/faker/actions/runs/10329602297/job/28597594550?pr=3044#step:6:9)272
    at Generator.next (<anonymous>)
    at /home/runner/work/faker/faker/node_modules/.pnpm/[email protected]/node_modules/jiti/dist/jiti.js:1:238153
    at new Promise (<anonymous>)
    at __awaiter (/home/runner/work/faker/faker/node_modules/.pnpm/[email protected]/node_modules/jiti/dist/jiti.js:1:237714)
    at jiti.import (/home/runner/work/faker/faker/node_modules/.pnpm/[email protected]/node_modules/jiti/dist/jiti.js:1:248217)
    at loadFlatConfigFile (/home/runner/work/faker/faker/node_modules/.pnpm/eslint@[9](https://github.com/faker-js/faker/actions/runs/10329602297/job/28597594550?pr=3044#step:6:10)[email protected]/node_modules/eslint/lib/eslint/eslint.js:383:41)
    at async calculateConfigArray (/home/runner/work/faker/faker/node_modules/.pnpm/[email protected][email protected]/node_modules/eslint/lib/eslint/eslint.js:473:28)
    at async ESLint.lintFiles (/home/runner/work/faker/faker/node_modules/.pnpm/[email protected][email protected]/node_modules/eslint/lib/eslint/eslint.js:905:25)
    at async Object.execute (/home/runner/work/faker/faker/node_modules/.pnpm/[email protected][email protected]/node_modules/eslint/lib/cli.js:502:23)
    at async main (/home/runner/work/faker/faker/node_modules/.pnpm/[email protected][email protected]/node_modules/eslint/bin/eslint.js:[15](https://github.com/faker-js/faker/actions/runs/10329602297/job/28597594550?pr=3044#step:6:16)3:22)
 ELIFECYCLE  Command failed with exit code 2.

faker/eslint.config.ts

Lines 131 to 150 in cdf54e4

//#region deprecation
{
plugins: {
deprecation:
// https://github.com/gund/eslint-plugin-deprecation/issues/78
// @ts-expect-error: Just eat it!
fixupPluginRules(eslintPluginDeprecation),
},
languageOptions: {
parser: tseslint.parser,
parserOptions: {
project: true,
warnOnUnsupportedTypeScriptVersion: false,
},
},
rules: {
'deprecation/deprecation': 'error',
},
},
//#endregion

@Shinigami92
Copy link
Member Author

When using eslint.config.ts we need to import eslint-plugin-deprecation via * as syntax, because we do not have esModuleInterop turned on

@Shinigami92 Shinigami92 changed the title chore(deps): update dependency eslint to v9.9.0 infra: use eslint.config.ts Aug 10, 2024
Base automatically changed from renovate/eslint to next August 10, 2024 08:32
@hyoban
Copy link

hyoban commented Aug 10, 2024

Try the following settings for vscode

{
  "eslint.options": {
    "flags": ["unstable_ts_config"]
  }
}

@Shinigami92 Shinigami92 force-pushed the infra-switch-to-eslint-config-ts branch from 3232e7e to 03e2cb4 Compare August 10, 2024 09:25
@Shinigami92 Shinigami92 marked this pull request as ready for review August 10, 2024 09:34
@Shinigami92 Shinigami92 requested a review from a team as a code owner August 10, 2024 09:34
eslint.config.ts Outdated Show resolved Hide resolved
eslint.config.ts Outdated Show resolved Hide resolved
eslint.config.ts Outdated Show resolved Hide resolved
@ST-DDT ST-DDT added the needs rebase There is a merge conflict label Aug 12, 2024
@Shinigami92 Shinigami92 force-pushed the infra-switch-to-eslint-config-ts branch from fbe241f to 9d684ea Compare August 15, 2024 15:02
@Shinigami92 Shinigami92 removed needs rebase There is a merge conflict do NOT merge yet Do not merge this PR into the target branch yet labels Aug 15, 2024
@Shinigami92 Shinigami92 force-pushed the infra-switch-to-eslint-config-ts branch from 9d684ea to c32ac78 Compare August 30, 2024 18:34
@Shinigami92 Shinigami92 requested review from ST-DDT and a team August 30, 2024 18:37
@ST-DDT ST-DDT added the needs rebase There is a merge conflict label Aug 30, 2024
@Shinigami92 Shinigami92 force-pushed the infra-switch-to-eslint-config-ts branch from 659fbb9 to 3ba0721 Compare August 30, 2024 19:49
@Shinigami92 Shinigami92 removed the needs rebase There is a merge conflict label Aug 30, 2024
ST-DDT
ST-DDT previously approved these changes Aug 30, 2024
@ST-DDT ST-DDT requested a review from a team August 30, 2024 21:12
@ST-DDT ST-DDT added the needs rebase There is a merge conflict label Sep 14, 2024
@Shinigami92 Shinigami92 force-pushed the infra-switch-to-eslint-config-ts branch from 1eb18c0 to 1dd9db3 Compare September 15, 2024 15:34
@Shinigami92 Shinigami92 removed the needs rebase There is a merge conflict label Sep 15, 2024
@Shinigami92
Copy link
Member Author

Merging this already, because it had approvals in the past, but we just focused on v9.0.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: infra Changes to our infrastructure or project setup p: 1-normal Nothing urgent
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants