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: optional react support #326

Merged
merged 9 commits into from
Nov 22, 2023
Merged

feat: optional react support #326

merged 9 commits into from
Nov 22, 2023

Conversation

suppayami
Copy link
Contributor

Description
Added basic support for react, using eslint-plugin-react and eslint-plugin-react-hooks.

Linked Issues
Closes #282

package.json Outdated
@@ -56,6 +56,8 @@
"eslint-plugin-n": "^16.3.1",
"eslint-plugin-no-only-tests": "^3.1.0",
"eslint-plugin-perfectionist": "^2.4.0",
"eslint-plugin-react": "^7.33.2",
"eslint-plugin-react-hooks": "^4.6.0",
Copy link
Owner

Choose a reason for hiding this comment

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

I'd like to keep them as optional peer deps, and ask ppl to explicitly install them when they want to use react. This way we don't introduce those deps to every user of this config.

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 moved them to optional peer deps. Should I leave them in devDeps for dev?

rules: {
// recommended rules react
...pluginReact.configs.recommended.rules as any,
...pluginReactHooks.configs.recommended.rules as any,
Copy link
Owner

Choose a reason for hiding this comment

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

I would like to avoid referencing configs presets, but rather flatten them as inline to have better control.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

By flattening + inline, do you mean copy the config over?

@usercao
Copy link

usercao commented Nov 19, 2023

Hi👋 @suppayami , can you add eslint-plugin-react-refresh or eslint-plugin-better-styled-components? This is also the most basic configuration item required in React.

@suppayami
Copy link
Contributor Author

@usercao I put eslint-plugin-react-refresh in, currently allowConstantExport is enabled by default, should we only enable it only when certain dev implementation (for example vite) exists in deps?

@usercao
Copy link

usercao commented Nov 19, 2023

@suppayami You're right, I'm sorry I didn't think carefully. Have you been using CSS-in-JS? It would be even better if you could refer to the packages listed above and add CSS-in-JS support.

@suppayami
Copy link
Contributor Author

@usercao as for css-in-js, I think it will be more appropriate for users to extend the config and put it in themselves, since not everyone use them in React. What do you think of this? @antfu

@usercao
Copy link

usercao commented Nov 19, 2023

export default antfu(
  {
    "vue": true, 
    "react": false, 
    "typescript": true,
    "styled-components": true,
  },
)

I respect your idea, just hope that only one @antfu/eslint-config in devDependencies is enough.

@Dimava
Copy link
Contributor

Dimava commented Nov 19, 2023

@suppayami you may use feature flag, same as typescript config has for project (type aware rules won't be enabled if you don't set the tsconfig location)

@suppayami
Copy link
Contributor Author

suppayami commented Nov 19, 2023

@suppayami you may use feature flag, same as typescript config has for project (type aware rules won't be enabled if you don't set the tsconfig location)

@Dimava Do you mean allowConstantExport or css in js? 😂
As for allowConstantExport, I added a list of packages that allow it and auto enable if package(s) exists

@Dimava
Copy link
Contributor

Dimava commented Nov 20, 2023

@suppayami No I don't mean anything specific, I didn't use React ever 😂
Just noted so you've considered if it's usable in this PR context

@suppayami
Copy link
Contributor Author

@antfu I think feature-wise this is ready to go, please have a review when you have time

@antfu antfu changed the title feat: react support feat: optional react support Nov 22, 2023
@antfu antfu merged commit 86ca184 into antfu:main Nov 22, 2023
5 checks passed
@antfu
Copy link
Owner

antfu commented Nov 22, 2023

I updated the PR to not have auto-detection, instead asking users to turn it on manually. Also, it will prompt users to auto-install necessary deps if missing.

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.

Question: add react config in flat config?
4 participants