Skip to content

Conversation

@karlhorky
Copy link
Contributor

@karlhorky karlhorky commented Oct 22, 2024

ESLint v8 is end-of-life and ESLint v9 uses flat config by default, so starting with this as a first example seems like the right option (https://x.com/karlhorky/status/1848758371127583015)

Also, fixed the ESLint brand name typos

cc @poteto

@vercel
Copy link

vercel bot commented Oct 22, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
19-react-dev ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 23, 2024 9:05am
react-dev ✅ Ready (Inspect) Visit Preview Oct 23, 2024 9:05am

@github-actions
Copy link

github-actions bot commented Oct 22, 2024

Size changes

📦 Next.js Bundle Analysis for react-dev

This analysis was generated by the Next.js Bundle Analysis action. 🤖

This PR introduced no changes to the JavaScript bundle! 🙌

Comment on lines -143 to +160
'react-compiler/react-compiler': "error",
'react-compiler/react-compiler': 'error',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I made the quote style here consistent with the other quotes

'eslint-plugin-react-compiler': reactCompiler,
},
rules: {
'react-compiler/react-compiler': 'error',

Choose a reason for hiding this comment

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

I think this should be:

'eslint-plugin-react-compiler/react-compiler': 'error'

If not, you will get the following error:

[Error] Request textDocument/diagnostic failed.
  Message: Request textDocument/diagnostic failed with message: Key "rules": Key "react-compiler/react-compiler": Could not find plugin "react-compiler".
  Code: -32603 
[Error] Document pull failed for text document file:///.../eslint.config.mjs
  Message: Request textDocument/diagnostic failed with message: Key "rules": Key "react-compiler/react-compiler": Could not find plugin "react-compiler".
  Code: -32603

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, that's not how it works - check other plugin docs:

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yea @AnthonyLzq is right here

in flat config you can infer the plugin namespace in the plugins object ref here

in the example you shared the plugin namespace is react-x thats why rules are referred with react-x/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sorry @AnthonyLzq @harish-sethuraman I see what you mean now

I fixed it the more conventional way, by using 'react-compiler' as a key in the plugins object:

Copy link
Collaborator

@harish-sethuraman harish-sethuraman left a comment

Choose a reason for hiding this comment

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

LGTM thanks! would like someone from the compiler team to verify before we merge this cc: @poteto @gsathya

@poteto
Copy link
Member

poteto commented Oct 23, 2024

Sweet, thanks!

@poteto poteto merged commit e628e5d into reactjs:main Oct 23, 2024
6 checks passed
Comment on lines +138 to +149
import reactCompiler from 'eslint-plugin-react-compiler'

export default [
{
plugins: {
'react-compiler': reactCompiler,
},
rules: {
'react-compiler/react-compiler': 'error',
},
},
]
Copy link
Contributor Author

@karlhorky karlhorky Oct 23, 2024

Choose a reason for hiding this comment

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

in case this would be an accepted change, I actually tend to prefer:

  1. a separate configs variable (instead of an anonymous export)
  2. usage of the ESLint Linter.Config[] TS type with a JSDoc annotation
Suggested change
import reactCompiler from 'eslint-plugin-react-compiler'
export default [
{
plugins: {
'react-compiler': reactCompiler,
},
rules: {
'react-compiler/react-compiler': 'error',
},
},
]
import reactCompiler from 'eslint-plugin-react-compiler'
/** @type {import('eslint').Linter.Config[]} */
const configs = [
{
plugins: {
'react-compiler': reactCompiler,
},
rules: {
'react-compiler/react-compiler': 'error',
},
},
]
export default configs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@poteto @harish-sethuraman oh lol too slow (thanks for the merge!)

in case this is a desirable change, I can open a separate PR

Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer leaving it off since this is more about personal preference. We just want the minimal config in the docs.

@karlhorky karlhorky deleted the patch-1 branch October 23, 2024 16:35
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.

5 participants