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: custom eslint configuration #77

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

mlvieras
Copy link
Collaborator

Ticket/Card

Description of the change

This PR updates our Eslint rules. Most importantly, I've removed the Airbnb Eslint configuration packages. Let's break this down.

Removal of eslint-config-airbnb and eslint-config-airbnb-typescript

The rationale here is that our linting rules are hidden behind a package which is not owned by us, and so it's hard to know what our actual standards are and have an open discussion around them. Another argument against using those packages is that we're fully dependent on those packages being updated in order to update our Eslint version (and even plugin versions).

Finally, this forces us to understand what plugins we have installed and it's much easier to make changes to the configuration.

Rule changes

I've tried to keep as much of our rules as possible, but also have taken the chance to make some changes I think are necessary.

As far as plugins go, I've ported the React, React Hooks and JSX A11y plugins from Airbnb's configuration.

Other changes

  • I've taken the change to update our NPM version to 10.9.0.
  • I added a .prettierrc file since I was having a conflict with VS Code and format-before-save.
  • I've fixed the teardown configuration for Vitest, which was actually not running.
  • Made changes to the logger to make it much easier to use.
  • Removed classes with only static methods, which are not really useful.
  • Removed code that was not useful, like the ParamsHelper.

Next Steps

  1. Upgrade Eslint to V9
  2. Related to the previous point, we need to migrate to use flat configuration files (which I completely despise; the things one does for love).
  3. Upgrade all our plugins to their latest version.
  4. Identify more plugins that could be useful?

src/routes/route-helpers.ts Outdated Show resolved Hide resolved
src/helpers/logger.ts Show resolved Hide resolved
@chaba11 chaba11 requested a review from Copilot November 18, 2024 18:41

Choose a reason for hiding this comment

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

Copilot reviewed 50 out of 65 changed files in this pull request and generated no suggestions.

Files not reviewed (15)
  • .prettierrc: Language not supported
  • package.json: Language not supported
  • src/common/app-link/app-link.stories.tsx: Evaluated as low risk
  • src/common/badge/badge.tsx: Evaluated as low risk
  • README.md: Evaluated as low risk
  • src/common/avatar/avatar.test.tsx: Evaluated as low risk
  • src/app.test.tsx: Evaluated as low risk
  • src/common/button/button.tsx: Evaluated as low risk
  • src/app.tsx: Evaluated as low risk
  • src/common/base-button/base-button.test.tsx: Evaluated as low risk
  • src/common/badge/badge.stories.tsx: Evaluated as low risk
  • src/common/app-link/app-link.tsx: Evaluated as low risk
  • src/common/error-boundary/error-boundary.test.tsx: Evaluated as low risk
  • src/common/button/button.stories.tsx: Evaluated as low risk
  • src/common/avatar/avatar.stories.tsx: Evaluated as low risk
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