Skip to content

Conversation

Bentroen
Copy link
Member

@Bentroen Bentroen commented Sep 28, 2025

Overview

This PR thoroughly reviews the project's linting and formatting configurations to cover previously unaddressed cases, clearly separate linting and formatting responsibilities, and ensure that checked code adheres to the specified guidelines.

The goal of these changes is to make following the code style a natural part of the development workflow, avoiding the need for contributors to go out of their way to ensure these guidelines are followed. This should reduce the number of future linting commits and decrease the amount of time spent in future codebase reviews.

The added configurations were largely based in Open Collective's frontend and API repositories.

List of Changes

  • Remove all stylistic rules from ESLint
  • Migrate ESLint import method to the monorepo (typescript-eslint), removing outdated @typescript-eslint/parser and @typescript-eslint/eslint-plugin dependencies
  • Completely untie formatting from linting by removing eslint-config-prettier and eslint-plugin-prettier (see: What about formatting?)
  • Add husky and lint-staged to run Prettier on Git staged files before committing
    • Enforced to run only on file types supported by Prettier via the --ignore-unknown option
    • Only formatting is run at the moment (no linting)
    • Since only staged files are considered, the time added to each commit should not be long
  • Re-add eslint-plugin-import (had been removed when migrating to ESLint 8)
  • Treat imports from @nbw/** modules as internal rather than third-party
  • Adjust VS Code settings to enforce the usage of Prettier as the default formatter in many cases
  • Enforce use of ESLint's flat config in VS Code settings
  • Add .prettierignore to specify exception cases to Prettier formatting
    • One example is the .hbs email template format: Prettier doesn't support them and collapses inline CSS in the style tag to a single block.
    • Other exceptions may be added as the codebase evolves.
  • Lint the entire codebase (including configuration files) according to the new standard
  • Rename the eslint.config.js file to .mjs to enforce ESM-like syntax

Other misc changes:

  • Remove unused tests/ package (can be re-added in the future once end-to-end tests are implemented)
  • Remove personal color customizations from VS Code workspace settings
  • Fix a typo in the frontend start scripts (@nbs instead of @nbw)

TODO

  • Consider increasing Prettier's printWidth setting from 80 to 100, and re-linting the code
  • Integrate prettier-plugin-tailwind-css to automatically sort Tailwind classes
  • Consider changing eslint-plugin-import to prettier-plugin-sort-imports or prettier-plugin-organize-imports
    • Linting was not added to the pre-commit hook as it takes too long, but this means imports will not be organized, unused imports won't be removed, etc., automatically, if needed.
    • Both packages add the observation that they violate Prettier's principles by changing the AST -- so it might be better to keep it a linting responsibility in the end!

Since the changes above are ready to be merged, these changes may be incorporated in a future pull request as to prevent holding back these changes (long-lived pull requests are tricky!)

tomast1337 and others added 21 commits September 28, 2025 15:37
…e ESLint configuration for improved rule management
It's preferable we re-add this package when E2E tests are actually implemented.
@tomast1337 tomast1337 added the enhancement New feature or request label Sep 30, 2025
@tomast1337 tomast1337 linked an issue Sep 30, 2025 that may be closed by this pull request
25 tasks
Copy link
Member Author

@Bentroen Bentroen left a comment

Choose a reason for hiding this comment

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

Awesome work!

I've left a few changes, once these are done, it's fine to merge this PR.

Log ad loading errors to the console for debugging.
Removed the steps for checking changes and committing linter fixes.
@tomast1337 tomast1337 merged commit 7042299 into develop Oct 5, 2025
2 checks passed
@tomast1337 tomast1337 deleted the chore/lint-and-format-review branch October 5, 2025 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Project structure refactor
3 participants