chore: add pre-commit hooks, ESLint type-import rules, and code quality guide#517
Draft
Just-Insane wants to merge 11 commits intoSableClient:devfrom
Draft
chore: add pre-commit hooks, ESLint type-import rules, and code quality guide#517Just-Insane wants to merge 11 commits intoSableClient:devfrom
Just-Insane wants to merge 11 commits intoSableClient:devfrom
Conversation
Just-Insane
force-pushed
the
feat/code-quality
branch
from
15743f4 to
362534b
Compare
… guards) - Add husky + lint-staged pre-commit hooks (eslint --fix, prettier --write) - Enable @typescript-eslint/consistent-type-definitions (type over interface) - Enable @typescript-eslint/consistent-type-imports (inline type imports) - Add ESLint rule banning direct localStorage in components/features - Apply type-imports codebase-wide lint fix (~500 files) - Add sentryStorage and mediaVolume utilities in src/app/state/ - Fix all direct localStorage violations in components/features
The consistent-type-definitions and consistent-type-imports rules
converted two module augmentation patterns in ways TypeScript doesn't
support:
- slate.d.ts: import { type X } from augmented module breaks the
declare module augmentation; must use regular imports. Also reverts
interface CustomTypes to interface (type aliases don't support
declaration merging that Slate relies on).
- matrix-sdk-events.d.ts: type alias cannot merge with existing
interface declarations in matrix-js-sdk; must use interface.
Add eslint-disable-next-line comments to suppress the rules at these
specific locations where they must be violated.
- Add vitest coverage thresholds (statements/functions/lines: 1.5%, branches: 1%) — locked at current baseline, never to be lowered - Add 'coverage' CI job that runs pnpm test:coverage on every PR/push - Add 'missing-tests' CI job that comments on PRs listing any changed logic files that have no corresponding .test. file (advisory, not blocking, to allow deliberate exceptions)
GitHub restricts GITHUB_TOKEN to read-only for fork PRs, even when pull-requests: write is declared in the workflow. The comment posting step fails with 403 on fork PRs. The advisory comment is informational; failing the entire check because the comment can't be posted is unnecessary noise.
Just-Insane
force-pushed
the
feat/code-quality
branch
from
362534b to
0b40ad6
Compare
Member
|
Let's chill on this for now. because there is some other tooling stuff that should happen as well. Like the whole tsconfig needs to be redone. it's completely busted. so might as well do it once, since this is going to cause a large merge conflict. Also I'm not sure we need a pre commit hook? we already got the CI for that.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds developer tooling to enforce code quality standards:
consistent-type-importsrule: enforcesimport type { Foo }syntax for type-only imports, reducing runtime overhead and avoiding circular-dependency issues.localStorageguard lint rule: flags directlocalStorageaccess outside the approved wrapper, ensuring consistent error handling.vite-tsconfig-paths: replaces manualresolve.aliasentries invite.config.tswith the plugin, keeping path aliases in sync withtsconfig.json.CODE_QUALITY.md: documents all of the above for contributors.Fixes #
Type of change
Checklist:
AI disclosure:
Redux of a previous PR that added code coverage requirements (tests), updates the related documentation, and adds some additional linting requirements (documented in CODE_QUALITY.md). Also adds some pre-commit hooks which should catch most issues on commit.
Note: This PR is large due to the new linting changes which require updates across most files to pass the new lint rules.