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

Vite migration: ESLint fixes #155

Open
wants to merge 11 commits into
base: develop
Choose a base branch
from

Conversation

NextFire
Copy link
Member

Follow-up to #154

eslint errors/warnings and applied fixes:

React no-unused-vars

Remove unused React imports.

no-class-assign and other no-unused-vars

Disable these two rules. The second error was often raised on unused function arguments or object destructured properties that I believe were left for better readability and self-documentation.

no-case-declarations

Add {}.

no-undef

Add // eslint-disable-next-line.
This error was raised on DAKARA_VERSION, DAKARA_BUGTRACKER and DAKARA_PROJECT_HOMEPAGE that are defined in vite.config.js.

react/display-name

Add // eslint-disable-next-line on four decorators in https://github.com/NextFire/dakara-client-web/blob/vite-eslint/src/components/adapted/ReactRouterDom.jsx.

react/no-unescaped-entities

Escape characters.

react-refresh/only-export-components

Name some default exports + move two shared const to https://github.com/NextFire/dakara-client-web/blob/vite-eslint/src/utils/permissions.js.

react/prop-types

Add or complete propTypes on various components.
I often had to guess when to add isRequired or not…

@NextFire NextFire changed the title Vite migration Vite migration: ESLint fixes Feb 25, 2024
@Neraste
Copy link
Member

Neraste commented Mar 16, 2024

I've to check if everything works as expected.

Add // eslint-disable-next-line.
This error was raised on DAKARA_VERSION, DAKARA_BUGTRACKER and DAKARA_PROJECT_HOMEPAGE that are defined in vite.config.js.

These variables are exctracted from package.json, if there is a better way to pass them, I'm open to it.

@Neraste
Copy link
Member

Neraste commented Mar 16, 2024

This is cleaner.

@Neraste
Copy link
Member

Neraste commented Jul 28, 2024

I just merged #156, so you can re-apply the linting. Maybe you should create a new PR instead?

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.

2 participants