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

fix(create-vite): remove eslint-plugin-react #19514

Merged
merged 4 commits into from
Feb 26, 2025
Merged

Conversation

bluwy
Copy link
Member

@bluwy bluwy commented Feb 25, 2025

Description

eslint-plugin-react is setup in the react js template only as it requires the react/jsx-use-vars rule to properly check if a React component is used by eslint. It's not needed in the TS template as typescript-eslint uses typescript itself to check for unused vars instead, which is more accurate.

It feels a bit overkill to add a large dependency to handle this case, or introducing a single package with a single rule dealing with this. So I think it's easiest to ignore the var checks for variables that starts with an uppercase for now.

Alternatively we could keep things as is, so I wonder what others think about this. But I think it's useful to keep the dependency size down so it's easier for users to try out the template.

@bluwy bluwy added p2-nice-to-have Not breaking anything but nice to have (priority) feat: create-vite create-vite package labels Feb 25, 2025
bluwy and others added 2 commits February 26, 2025 12:11
Copy link
Member

@sapphi-red sapphi-red left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍

},
}
```
If you are developing a production application, we recommend using TypeScript and enable type-aware lint rules. Check out the [TS template](https://github.com/vitejs/vite/tree/main/packages/create-vite/template-react-ts) to integrate TypeScript and [`typescript-eslint`](https://typescript-eslint.io) in your project.
Copy link
Member

Choose a reason for hiding this comment

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

🙏

- Replace `tseslint.configs.recommended` to `tseslint.configs.recommendedTypeChecked` or `tseslint.configs.strictTypeChecked`
- Optionally add `...tseslint.configs.stylisticTypeChecked`
- Install [eslint-plugin-react](https://github.com/jsx-eslint/eslint-plugin-react) and update the config:
You can also install [eslint-plugin-react](https://github.com/jsx-eslint/eslint-plugin-react) for React-specific lint rules:
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we can already mention it? Even if the code example is still for eslint-plugin-react

Suggested change
You can also install [eslint-plugin-react](https://github.com/jsx-eslint/eslint-plugin-react) for React-specific lint rules:
You can also install [eslint-plugin-react](https://github.com/jsx-eslint/eslint-plugin-react) or [[eslint-plugin-react-x](eslint-plugin-react-x](https://github.com/Rel1cx/eslint-react) for React-specific lint rules:

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah maybe it's not a lot of work left than I expected. I wanted to rewrite this part to eslint-plugin-react-x only though and suggest a code example for it.

I'll try to get to that later, but if not I think this PR is ok to be merged and I can create another PR later.

Copy link
Member

Choose a reason for hiding this comment

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

As you prefer, thanks for doing this!

@bluwy bluwy merged commit c0e3dba into main Feb 26, 2025
15 checks passed
@bluwy bluwy deleted the remove-eslint-plugin-react branch February 26, 2025 09:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat: create-vite create-vite package p2-nice-to-have Not breaking anything but nice to have (priority)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants