-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Chore: Finalize Eslint rules #2963
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
Conversation
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
WalkthroughAdds/upgrades ESLint configs and deps (shared tooling and per-package), updates VS Code recommendations, tweaks an email template's content and class ordering, and removes Husky pre-commit hooks and its prepare script from package.json. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Git as Git
participant Husky as Husky (pre-change)
participant ESLint as ESLint/Formatter
note over Dev,GIT: Before
Dev->>Git: git commit
Git->>Husky: run pre-commit
Husky->>ESLint: run format & lint
ESLint-->>Husky: pass/fail
Husky-->>Git: allow or block commit
note over Dev,GIT: After
Dev->>Git: git commit
Git-->>Dev: commit completes (no pre-commit hook)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
f280327 to
4eb8330
Compare
4eb8330 to
5895e93
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
.husky/pre-commit(0 hunks).vscode/extensions.json(1 hunks)package.json(0 hunks)packages/email/src/templates/invite-user.tsx(2 hunks)packages/file-system/eslint.config.js(1 hunks)packages/file-system/package.json(2 hunks)tooling/eslint/base.js(2 hunks)tooling/eslint/package.json(1 hunks)tooling/eslint/react.js(2 hunks)
💤 Files with no reviewable changes (2)
- package.json
- .husky/pre-commit
🚧 Files skipped from review as they are similar to previous changes (3)
- tooling/eslint/base.js
- .vscode/extensions.json
- packages/file-system/eslint.config.js
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use the any type unless necessary
Files:
packages/email/src/templates/invite-user.tsx
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Avoid using the any type unless absolutely necessary
Files:
packages/email/src/templates/invite-user.tsx
🔇 Additional comments (4)
tooling/eslint/package.json (1)
18-18: LGTM! Dependency additions align with ESLint finalization.The addition of
@tanstack/eslint-plugin-query(v5.62.0) andeslint-plugin-jsx-a11y(v6.10.2) properly supports the new plugin integrations intooling/eslint/react.js. Both versions are stable and no security advisories are noted.Based on learnings.
Also applies to: 22-22
tooling/eslint/react.js (1)
1-2: LGTM! Plugin integration follows best practices.The integration of
@tanstack/eslint-plugin-queryandeslint-plugin-jsx-a11yis correctly implemented:
- Plugins are properly imported and registered with standard naming conventions
- Recommended rule sets are spread into the configuration, which is the advised baseline approach
- The implementation follows ESLint flat config patterns
Based on learnings.
Also applies to: 14-15, 21-22
packages/file-system/package.json (1)
37-37: LGTM!Adding
@onlook/eslintandeslint@^9.0.0as devDependencies correctly supports the new ESLint-based linting and formatting setup.Also applies to: 41-41
packages/email/src/templates/invite-user.tsx (1)
38-76: LGTM! Formatting and content improvements.The Tailwind className reordering throughout the component (lines 38, 39, 42, 46, 56, 62, 68, 69) is cosmetic and likely the result of auto-formatting by ESLint or Prettier plugins. This improves consistency without affecting functionality.
The content additions enhance the email template:
- Line 51: Clearer invitation message ("has invited you to their project on Onlook.")
- Lines 69-75: Improved safety guidance instructing recipients to reply if concerned about account safety
These changes improve both code consistency and user experience.
| "lint": "eslint . --max-warnings 0", | ||
| "format": "eslint --fix .", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Script changes align with ESLint-first approach.
The lint script now enforces zero warnings with --max-warnings 0, and the format script delegates formatting to ESLint (via --fix) instead of Prettier. This is consistent with the PR's goal of integrating eslint-config-prettier and consolidating on ESLint for both linting and formatting.
Verify that other packages in the monorepo follow the same pattern for lint and format scripts to ensure consistency:
🏁 Script executed:
#!/bin/bash
# Description: Check lint and format scripts across all packages
# Find all package.json files and extract their lint/format scripts
fd -t f '^package\.json$' packages apps | while read -r file; do
echo "=== $file ==="
jq -r '.scripts | select(. != null) | {lint: .lint, format: .format}' "$file"
echo
doneLength of output: 10617
🏁 Script executed:
#!/bin/bash
# Description: Check lint and format scripts across all packages
fd -t f '^package\.json$' packages apps | while read -r file; do
echo "=== $file ==="
jq -r '
.scripts? // {} |
"lint: \(.lint // "N/A")\nformat: \(.format // "N/A")"
' "$file"
echo
doneLength of output: 2251
Inconsistent scripts in monorepo
All packages under apps/web/client, preload, server and every packages/* use
"lint": "eslint . --max-warnings 0"
"format": "eslint --fix ."
but apps/backend/package.json and apps/web/package.json have no lint/format scripts. Add the same scripts there for consistency.
🤖 Prompt for AI Agents
In packages/file-system/package.json around lines 16 to 17, the repo expects
consistent npm scripts across the monorepo but this package.json contains only
"lint" and "format" while other top-level apps (apps/backend and apps/web) are
missing them; add "lint": "eslint . --max-warnings 0" and "format": "eslint
--fix ." to apps/backend/package.json and apps/web/package.json (or ensure their
package.json files include identical script entries) so all packages use the
same linting and formatting commands.
Description
Fixes some import jankiness due to eslint and prettier having different formats they're expecting under the hood by disabling / adjusting some eslint rules.
Along with this, it finalizes the eslint and prettier setup and ensures it's consistent across all packages in the repo. It also removes husky since we'll rely on CI/CD to confirm the repo is up-to-date
Related Issues
Type of Change
Testing
Screenshots (if applicable)
Additional Notes
Important
Finalizes ESLint and Prettier configurations, removes Husky, and updates email template styling.
eslint.config.jstofile-systempackage.base.js.base.js.eslint-plugin-jsx-a11yand@tanstack/eslint-plugin-querytoreact.js.package.jsonandbun.lock.eslintas a devDependency infile-system/package.json.invite-user.tsxfor consistent styling.This description was created by
for 5895e93. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Chores
Style
Chores