Skip to content

Conversation

@saddlepaddle
Copy link
Collaborator

@saddlepaddle saddlepaddle commented Sep 13, 2025

Summary

This PR introduces a centralized ESLint and Prettier configuration for the Onlook monorepo, heavily inspired by create-t3-turbo.

TODO

  • In a follow-up PR, we can format all code in the repo based on the eslint / prettier rules we pick.
  • Enforce linting in CI/CD for every package in the repo.
  • Consider removing husky, as bun format now runs a bit slower (also fixes eslint rules on top of prettier rules wherever possible)
  • Might need to disable a rule or two from eslint if the perf is really bad - will confirm after PR Update theming #2 (enforce linting + format all code)

What Changed

🏗️ New Tooling Packages

  1. tooling/eslint - Centralized ESLint configuration

    • base.js - Core TypeScript, import rules, and Prettier integration
    • react.js - React-specific rules and hooks
    • nextjs.js - Next.js specific configuration
      • Includes restrictEnvAccess to force users to import from typesafe env.ts file instead of process.env directly when included.
  2. tooling/prettier - Shared Prettier configuration

    • Extends apps/web/client/.prettierrc, which was removed in this PR
    • Consistent formatting rules across the codebase
    • Import sorting with @ianvs/prettier-plugin-sort-imports
    • Tailwind CSS class sorting

🔄 Key Improvements vs Original Setup

Aspect Before After
ESLint Config Mixed formats (flat config, legacy .eslintrc) Unified flat config (ESLint 9+)
Prettier Integration Separate Prettier configs, some packages using prettier --write Prettier runs through ESLint via eslint-plugin-prettier
Import Sorting No standardized import sorting Consistent import ordering with customized patterns
Scripts Inconsistent: next lint, eslint --fix, prettier --write Standardized: lint and format scripts
Dependencies Each package managed its own ESLint/Prettier deps Centralized in tooling packages

📝 Standardized Scripts

All packages now use:

  • "lint": "eslint . --max-warnings x" - Strict linting with a configurable max warning count
  • "format": "eslint --fix ." - Auto-fix through ESLint (includes Prettier)
  • "typecheck": "tsc --noEmit" - ensures there are no typescript issues in packages.

🎯 Package-Specific Configurations

  • Web Client (apps/web/client): Uses base + React + Next.js + restrictEnvAccess configs
  • UI Package (packages/ui): Uses base + React configs
  • Backend Packages: Use base config only
  • All packages properly ignore build artifacts and node_modules

🚀 Migration Details

  1. Removed direct Prettier dependencies from individual packages
  2. Converted next lint to standard ESLint setup in web client
  3. Added missing ESLint configurations to github and stripe packages
  4. Removed unnecessary TypeScript dependencies from prettier package

Benefits

  • Consistency: Same linting and formatting rules everywhere
  • Maintainability: Update rules in one place, applies everywhere
  • Performance: Prettier runs through ESLint, avoiding conflicts
  • Developer Experience: Clear, predictable lint/format commands
  • Type Safety: TypeScript ESLint with proper type checking

Testing

  • Ran bun install to install dependencies
  • Tested lint/format scripts across multiple packages
  • Verified ESLint configs load correctly
  • Confirmed import sorting works as expected

Important

Centralizes ESLint and Prettier configurations in the Onlook monorepo, standardizing linting and formatting across packages with new tooling packages.

  • Tooling Packages:
    • Adds tooling/eslint for centralized ESLint configuration with base.js, react.js, and nextjs.js.
    • Adds tooling/prettier for shared Prettier configuration, removing apps/web/client/.prettierrc.
  • Configuration Changes:
    • Standardizes ESLint and Prettier configurations across packages, using eslint-plugin-prettier for integration.
    • Introduces import sorting and Tailwind CSS class sorting in Prettier.
  • Scripts and Dependencies:
    • Updates scripts in package.json files to use eslint . --max-warnings 0 for linting and eslint --fix . for formatting.
    • Centralizes ESLint and Prettier dependencies in tooling packages, removing them from individual packages.
  • CI/CD:
    • Prepares for enforcing linting in CI/CD, with a follow-up PR planned for applying lint fixes and setting warning limits.

This description was created by Ellipsis for af7ce35. You can customize this summary. It will automatically update as commits are pushed.

Summary by CodeRabbit

  • CI
    • Introduced unified CI workflow for PRs and main; runs typecheck and unit tests with coverage. Removed legacy unit-test workflow.
  • Chores
    • Centralized linting via shared ESLint presets; added lint/format/typecheck scripts across packages. Switched formatting to ESLint, removed Prettier configs, and disabled pre-commit formatter. Set module type at root and in docs.
  • Refactor
    • Migrated package ESLint configurations and updated ignores/tsconfig includes.
  • Style
    • Cleaned up imports and class ordering in the calendar component.
  • Tests
    • Added explicit generic types in parser tests.

@vercel
Copy link

vercel bot commented Sep 13, 2025

@saddlepaddle is attempting to deploy a commit to the Onlook Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai
Copy link

coderabbitai bot commented Sep 13, 2025

Warning

Rate limit exceeded

@Kitenite has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 4 minutes and 47 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between c6419da and c751ed2.

📒 Files selected for processing (3)
  • packages/db/package.json (2 hunks)
  • packages/scripts/tsconfig.json (1 hunks)
  • packages/stripe/package.json (2 hunks)

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Adds centralized ESLint/Prettier tooling packages, replaces Prettier with ESLint-driven formatting across repo, updates package scripts and TS versions, standardizes ESLint configs per package, consolidates CI into a new workflow with typecheck/tests, removes legacy config files, tweaks Husky pre-commit, and makes minor code/tests and formatting adjustments.

Changes

Cohort / File(s) Summary of edits
CI consolidation
.github/workflows/ci.yml, .github/workflows/unit-test.yml
Added unified CI workflow with Bun typecheck and unit tests; removed old unit-test workflow.
Husky & Prettier config removals
.husky/pre-commit, .prettierrc, apps/web/client/.prettierrc, apps/web/client/prettier.config.js, packages/parser/.prettierignore, packages/fonts/.prettierignore
Disabled pre-commit formatter step; removed Prettier configs/ignores shifting formatting to ESLint.
Root configs
eslint.config.js, package.json
Added root ESLint config extending shared base; set type: module; generalized lint/typecheck scripts; added @onlook/eslint devDep.
Tooling: ESLint package
tooling/eslint/*
Introduced shared ESLint package (@onlook/eslint) with base, react, nextjs configs, exports, and tsconfig.
Tooling: Prettier package
tooling/prettier/*
Added @onlook/prettier package with unified Prettier config and manifest.
Docs linting migration
docs/.eslintrc.json, docs/eslint.config.js, docs/package.json
Replaced legacy ESLint config with shared presets; added lint/format scripts; updated devDeps; set ESM.
Web client linting migration
apps/web/client/eslint.config.js, apps/web/client/package.json
Switched to shared ESLint configs; simplified scripts to lint/format/typecheck; removed Prettier and Next configs; adjusted versions.
Web preload/server tooling
apps/web/preload/eslint.config.js, apps/web/preload/package.json, apps/web/preload/tsconfig.json, apps/web/server/eslint.config.js, apps/web/server/package.json, apps/web/server/tsup.config.ts
Added ESLint configs and scripts; updated TS includes; added devDeps; removed tsup config.
Packages: add ESLint configs
packages/*/eslint.config.js (ai, code-provider, constants, db, email, fonts, git, github, growth, image-server, models, parser, penpal, rpc, scripts, stripe, types, ui, utility)
Added per-package ESLint configs extending shared base, with local ignores where needed; some include react preset.
Packages: scripts/devDeps updates
packages/*/package.json (ai, code-provider, constants, db, email, fonts, git, github, growth, image-server, models, parser, penpal, rpc, scripts, stripe, types, ui, utility)
Standardized scripts: lint as eslint . --max-warnings 0, format as eslint --fix .; added @onlook/eslint, eslint ^9, typescript ^5.5.4 (where applicable).
GitHub package TS config
packages/github/tsconfig.json
Switched extends to @onlook/typescript/base.json.
Parser test typing
packages/parser/test/ids.test.ts
Added explicit generic types to Set/Map variables.
UI minor cleanup
packages/ui/src/components/calendar.tsx
Deduped icon imports; reordered classNames; ignored children prop in icon components.
Trailing newline normalizations
apps/backend/package.json, apps/web/package.json, tooling/typescript/package.json
Added end-of-file newlines; no functional changes.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    actor Dev as Developer
    participant GH as GitHub
    participant CI as CI Workflow (ci.yml)
    participant Bun as Bun Setup & Cache
    participant Job1 as Typecheck
    participant Job2 as Unit Test

    Dev->>GH: Push / PR
    GH-->>CI: Trigger on: [pull_request, push(main)]
    par Parallel jobs
        CI->>Bun: actions/checkout + setup-bun@v1 (latest)
        CI->>Bun: Restore/save cache (~/.bun/install/cache) keyed by bun.lockb
        CI->>Job1: bun install --frozen
        Job1->>Job1: bun typecheck
    and
        CI->>Bun: actions/checkout + setup-bun@v1 (latest)
        CI->>Bun: Restore/save cache (~/.bun/install/cache) keyed by bun.lockb
        CI->>Job2: bun install --frozen
        Job2->>Job2: bun test --timeout 30000 --coverage
    end
    CI-->>GH: Report job statuses
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Poem

A rabbit taps the linting drum—thump thump!
Prettier bows, ESLint takes the rump.
Workflows march in tidy rows,
Cache in burrows, Bun now knows.
With ears up high, I ship with flair—
No warnings left, just spotless air. 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The PR description is detailed (Summary, What Changed, Migration details, Benefits, and a Testing section) but does not follow the repository's required template: it is missing the explicit "## Description" heading, the "## Related Issues" section with issue links or GitHub keywords, and the "Type of Change" checklist from the template. Because the repository enforces that template, these omissions mean the description does not strictly comply even though the provided content is otherwise comprehensive. Please update the PR description to exactly include the repository template sections: add a "## Description" block summarizing the change, add a "## Related Issues" section with GitHub issue links (use "closes #" where applicable), and fill the "Type of Change" checklist with the appropriate box checked. Also ensure the "Testing" section lists concrete verification steps and exact commands run (e.g., specific lint/format/typecheck commands), and include screenshots only if UI changes apply. After these edits the description will comply with the template and this check should pass.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title "chore: Unify linting strategy + enforce linting across repo" is concise, clearly reflects the primary change (centralizing ESLint/Prettier tooling and enforcing linting across packages), and maps directly to the changes in the diff (new tooling packages, standardized scripts, and CI adjustments).
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Was pulled into tooling/prettier/index.js

"include": [
"src",
"test"
"script",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

no src/test in this package

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

was unused

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

replaced by eslint.config.js

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Also looks unused, since we're using bun build

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Disabling temporarily, otherwise this PR will have 1000's of files touched

@saddlepaddle saddlepaddle changed the title Chore: Unify linting strategy + enforce linting across repo chore: Unify linting strategy + enforce linting across repo Sep 13, 2025
@@ -1,55 +1,56 @@
{
"name": "@onlook/repo",
Copy link
Contributor

Choose a reason for hiding this comment

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

Would the spacing change be from linter? Seems like just different configs causing this spacing stuff

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

oop I did this on accident it looks like! added json linting to the package to standardize the formatting for it in the future 😄

/**
* All packages that leverage t3-env should use this rule
*/
export const restrictEnvAccess = tseslint.config(
Copy link
Contributor

Choose a reason for hiding this comment

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

This might just be for the nextjs package

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

true - can move it to nextjs! I tend to use https://www.npmjs.com/package/@t3-oss/env-core even in my hono server / expo etc. but ye most likely out-of-scope :^)

@saddlepaddle saddlepaddle marked this pull request as ready for review September 14, 2025 03:51
eslint.config.js Outdated
export default [
...baseConfig,
{
files: ["tooling/**/*.js, package.json"],
Copy link
Contributor

Choose a reason for hiding this comment

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

The file pattern in the root eslint.config.js contains a syntax error. The pattern should be an array of separate strings rather than a single string with a comma:

files: ["tooling/**/*.js", "package.json"],

This ensures ESLint correctly identifies both patterns as separate file matchers.

Suggested change
files: ["tooling/**/*.js, package.json"],
files: ["tooling/**/*.js", "package.json"],

Spotted by Diamond

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.

Comment on lines +2 to 3
# bun run format && git add .
# bun run lint && bun run format:write && git add .
Copy link
Contributor

Choose a reason for hiding this comment

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

The pre-commit hook has been commented out without enabling a replacement, effectively disabling Git's pre-commit functionality. Consider either:

  1. Uncommenting line 3 to use the new format command:

    bun run format && git add .
    
  2. Or uncommenting line 4 to use the previous approach:

    bun run lint && bun run format:write && git add .
    

Without an active hook, code formatting won't be automatically applied before commits.

Suggested change
# bun run format && git add .
# bun run lint && bun run format:write && git add .
bun run format && git add .
# bun run lint && bun run format:write && git add .

Spotted by Diamond

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.

eslint.config.js Outdated
export default [
...baseConfig,
{
files: ["tooling/**/*.js, package.json"],
Copy link
Contributor

Choose a reason for hiding this comment

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

The glob pattern here is a single string 'tooling//*.js, package.json'. It likely should be split into separate array elements (e.g. ['tooling//*.js', 'package.json']) to ensure proper file matching.

Suggested change
files: ["tooling/**/*.js, package.json"],
files: ["tooling/**/*.js", "package.json"],

…cess to nextjs eslint config, add .json linting
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (3)
packages/scripts/package.json (1)

24-33: Remove Jest deps to avoid type conflicts and install bloat.

You’re using Bun’s test runner (bun test). Keeping jest and @types/jest can cause duplicate globals and slower installs.

Apply this diff:

   "devDependencies": {
-    "@onlook/eslint": "*",
+    "@onlook/eslint": "*",
     "@onlook/typescript": "*",
-    "eslint": "^9.0.0",
-    "@types/jest": "^30.0.0",
+    "eslint": "^9.0.0",
     "@types/node": "^20.10.5",
     "@types/prompts": "^2.4.9",
-    "jest": "^30.0.5",
     "typescript": "^5.3.3"
   }
tooling/eslint/base.js (2)

38-44: Drop eslint-plugin-only-warn to avoid policy confusion.

You enforce --max-warnings 0 in packages, which already fails on warnings. only-warn downgrades errors to warnings in editors, causing mixed signals.

Apply this diff:

-import onlyWarn from 'eslint-plugin-only-warn';
@@
         plugins: {
             import: importPlugin,
             prettier: prettierPlugin,
-            // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
-            onlyWarn,
         },

38-64: Widen file globs to cover common module/JSX extensions.

Currently .jsx, .cjs, .mjs, .cts, .mts aren’t linted.

Apply this diff:

-        files: ['**/*.js', '**/*.ts', '**/*.tsx'],
+        files: [
+            '**/*.js', '**/*.cjs', '**/*.mjs', '**/*.jsx',
+            '**/*.ts', '**/*.cts', '**/*.mts', '**/*.tsx'
+        ],
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between af7ce35 and 7cba1b4.

⛔ Files ignored due to path filters (1)
  • bun.lock is excluded by !**/*.lock
📒 Files selected for processing (32)
  • apps/backend/package.json (1 hunks)
  • apps/web/client/eslint.config.js (1 hunks)
  • apps/web/package.json (1 hunks)
  • apps/web/preload/package.json (2 hunks)
  • apps/web/server/package.json (2 hunks)
  • docs/package.json (2 hunks)
  • package.json (3 hunks)
  • packages/ai/package.json (2 hunks)
  • packages/code-provider/package.json (2 hunks)
  • packages/constants/package.json (2 hunks)
  • packages/db/package.json (2 hunks)
  • packages/email/package.json (2 hunks)
  • packages/fonts/package.json (2 hunks)
  • packages/git/package.json (2 hunks)
  • packages/github/package.json (1 hunks)
  • packages/growth/package.json (2 hunks)
  • packages/image-server/package.json (2 hunks)
  • packages/models/package.json (2 hunks)
  • packages/parser/package.json (2 hunks)
  • packages/penpal/package.json (2 hunks)
  • packages/rpc/package.json (2 hunks)
  • packages/scripts/package.json (2 hunks)
  • packages/stripe/package.json (2 hunks)
  • packages/types/package.json (2 hunks)
  • packages/ui/package.json (2 hunks)
  • packages/utility/package.json (2 hunks)
  • tooling/eslint/base.js (1 hunks)
  • tooling/eslint/nextjs.js (1 hunks)
  • tooling/eslint/package.json (1 hunks)
  • tooling/eslint/tsconfig.json (1 hunks)
  • tooling/prettier/package.json (1 hunks)
  • tooling/typescript/package.json (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • apps/web/package.json
🚧 Files skipped from review as they are similar to previous changes (27)
  • tooling/typescript/package.json
  • apps/backend/package.json
  • packages/image-server/package.json
  • packages/db/package.json
  • packages/rpc/package.json
  • tooling/eslint/nextjs.js
  • tooling/prettier/package.json
  • packages/code-provider/package.json
  • tooling/eslint/package.json
  • packages/constants/package.json
  • packages/utility/package.json
  • packages/email/package.json
  • apps/web/server/package.json
  • packages/parser/package.json
  • tooling/eslint/tsconfig.json
  • apps/web/client/eslint.config.js
  • packages/stripe/package.json
  • packages/growth/package.json
  • packages/git/package.json
  • packages/fonts/package.json
  • packages/models/package.json
  • packages/github/package.json
  • packages/types/package.json
  • packages/penpal/package.json
  • packages/ui/package.json
  • packages/ai/package.json
  • apps/web/preload/package.json
🧰 Additional context used
📓 Path-based instructions (1)
**/package.json

📄 CodeRabbit inference engine (CLAUDE.md)

Use Bun as the package manager; do not reference npm, yarn, or pnpm in package.json scripts

Files:

  • package.json
  • packages/scripts/package.json
  • docs/package.json
🧠 Learnings (11)
📓 Common learnings
Learnt from: CR
PR: onlook-dev/onlook#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-14T01:43:40.581Z
Learning: Applies to .github/workflows/**/*.yml : In CI workflows, use Bun commands (bun install, bun run, bunx) instead of npm/yarn/pnpm/npx
📚 Learning: 2025-09-14T01:43:40.581Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-14T01:43:40.581Z
Learning: Applies to **/package.json : Use Bun as the package manager; do not reference npm, yarn, or pnpm in package.json scripts

Applied to files:

  • package.json
📚 Learning: 2025-09-14T01:43:40.581Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-14T01:43:40.581Z
Learning: Applies to .github/workflows/**/*.yml : In CI workflows, use Bun commands (bun install, bun run, bunx) instead of npm/yarn/pnpm/npx

Applied to files:

  • package.json
  • packages/scripts/package.json
📚 Learning: 2025-09-14T01:43:40.581Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-14T01:43:40.581Z
Learning: Applies to **/Makefile : In Makefiles, invoke Bun commands rather than npm/yarn/pnpm/npx

Applied to files:

  • package.json
  • packages/scripts/package.json
📚 Learning: 2025-09-14T01:43:40.581Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-14T01:43:40.581Z
Learning: Applies to **/*.{sh,bash} : In shell scripts, use Bun commands (bun install, bun run, bun add, bunx) and avoid npm/yarn/pnpm/npx

Applied to files:

  • package.json
  • packages/scripts/package.json
📚 Learning: 2025-09-14T01:43:40.581Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-14T01:43:40.581Z
Learning: Applies to **/Dockerfile{,.*} : In Dockerfiles, use Bun-based commands instead of npm/yarn/pnpm/npx

Applied to files:

  • package.json
📚 Learning: 2025-09-14T01:44:21.199Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: AGENTS.md:0-0
Timestamp: 2025-09-14T01:44:21.199Z
Learning: Use Bun for all installs and scripts; do not introduce npm, yarn, or pnpm

Applied to files:

  • package.json
📚 Learning: 2025-08-15T04:00:01.345Z
Learnt from: Raj-G07
PR: onlook-dev/onlook#2680
File: apps/web/preload/Dockerfile:20-20
Timestamp: 2025-08-15T04:00:01.345Z
Learning: When migrating Bun commands from docker-compose.yml to Dockerfile CMD, both "bun run server/index.ts" and "bun server/index.ts" work for direct file execution, but the latter is more explicit and efficient when no package.json scripts are involved.

Applied to files:

  • package.json
  • packages/scripts/package.json
📚 Learning: 2025-09-14T01:44:21.199Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: AGENTS.md:0-0
Timestamp: 2025-09-14T01:44:21.199Z
Learning: Do not run db:gen; use provided Bun scripts for tests (bun test), typecheck (bun run typecheck), and DB updates (bun run db:push)

Applied to files:

  • package.json
📚 Learning: 2025-09-14T01:44:21.199Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: AGENTS.md:0-0
Timestamp: 2025-09-14T01:44:21.199Z
Learning: Applies to apps/web/client/src/env.ts : Define and validate environment variables via t3-oss/env-nextjs in apps/web/client/src/env.ts

Applied to files:

  • tooling/eslint/base.js
📚 Learning: 2025-09-14T01:44:21.199Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: AGENTS.md:0-0
Timestamp: 2025-09-14T01:44:21.199Z
Learning: Applies to apps/web/client/next.config.ts : Import ./src/env in Next.js config to enforce env validation at build time

Applied to files:

  • tooling/eslint/base.js
🔇 Additional comments (6)
packages/scripts/package.json (1)

12-13: Good: unified ESLint-driven lint/format scripts.

This aligns with the repo-wide strategy and enables formatting via eslint-plugin-prettier.

package.json (2)

53-55: LGTM: centralizing ESLint tooling at root.

Adding @onlook/eslint at the root complements workspace adoption.


8-8: Repo set to ESM at root — confirm there are no CommonJS-only scripts

package.json (repo root) contains "type": "module". Automated scan was inconclusive (ripgrep reported "No files were searched"). Verify top-level scripts and all workspaces (packages/, apps/, tooling/*) for CommonJS patterns (require, module.exports, __dirname, __filename, exports., .cjs files, or shebang node scripts). Recommended commands to run locally:

rg -n -uu --hidden -g '!node_modules/' -g '!dist/' -g '!build/**' -e '\brequire(' -e 'module.exports\b' -e '__dirname\b' -e '__filename\b' -e '\bexports.' .

find . -type f -name '.cjs' -not -path './node_modules/' -not -path './dist/' -not -path './build/' -print

rg -n -uu --hidden -g '!node_modules/' -g '!dist/' -g '!build/**' -e '^#!.*\bnode\b' .

docs/package.json (2)

11-13: Docs: ESLint scripts look good.

Consistent with the unified linting approach.


26-36: Verify TypeScript/ESLint compatibility & align workspace TypeScript versions.

  • Most packages (including docs) use typescript ^5.5.4; packages/scripts uses ^5.3.3 — align to a single TS version.
  • No local @onlook/eslint package found in the repo — confirm the external @onlook/eslint preset's peerDependencies/@typescript-eslint requirements accept TS 5.5.x; if not, either update the preset or pin/upgrade workspace TypeScript to avoid parser mismatches.

Affected files: docs/package.json (typescript: ^5.5.4), packages/scripts/package.json (typescript: ^5.3.3).

tooling/eslint/base.js (1)

24-36: Ensure jsonc plugin is in scope when overriding its rules

The override disables jsonc/* rules but doesn't explicitly add the jsonc plugin; include jsonc: jsoncPlugin under plugins to avoid "rule not found" errors.

-    ...jsoncPlugin.configs['flat/recommended-with-json'],
+    ...jsoncPlugin.configs['flat/recommended-with-json'],
     {
         files: ['**/*.json', '**/*.jsonc'],
         plugins: {
-            prettier: prettierPlugin,
+            // keep jsonc plugin in scope for overrides
+            jsonc: jsoncPlugin,
+            prettier: prettierPlugin,
         },
         rules: {
             'prettier/prettier': ['warn', prettierConfig],
             // Disable some JSON rules that conflict with prettier
             'jsonc/comma-dangle': 'off',
             'jsonc/indent': 'off',
         },
     },

Sandbox verification failed: bunx not found. Run locally and share output of:
eslint --print-config package.json | rg -n "jsonc/(comma-dangle|indent)"

Comment on lines +13 to +14
includeIgnoreFile(path.join(import.meta.dirname, '../../.gitignore')),
{
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Use a portable dirname resolution.

import.meta.dirname isn’t universally supported (Node vs Bun). Resolve via fileURLToPath to avoid runtime errors.

Apply this diff:

-import * as path from 'node:path';
+import * as path from 'node:path';
+import { fileURLToPath } from 'node:url';
@@
-export default tseslint.config(
-    includeIgnoreFile(path.join(import.meta.dirname, '../../.gitignore')),
+export default tseslint.config(
+    // Resolve relative to this config file
+    includeIgnoreFile(path.join(path.dirname(fileURLToPath(import.meta.url)), '../../.gitignore')),
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
includeIgnoreFile(path.join(import.meta.dirname, '../../.gitignore')),
{
import * as path from 'node:path';
import { fileURLToPath } from 'node:url';
export default tseslint.config(
// Resolve relative to this config file
includeIgnoreFile(path.join(path.dirname(fileURLToPath(import.meta.url)), '../../.gitignore')),
{
🤖 Prompt for AI Agents
In tooling/eslint/base.js around lines 13 to 14, replace the direct use of
import.meta.dirname (which is not portable) with a dirname resolved via
fileURLToPath/new URL from import.meta.url: import fileURLToPath, compute
__filename and __dirname (or a dirname constant) using fileURLToPath(new
URL(import.meta.url)) and use that dirname when calling
includeIgnoreFile(path.join(dirname, '../../.gitignore')); ensure you add the
necessary import from 'url' and adjust any variable names accordingly.

Copy link
Contributor

@Kitenite Kitenite left a comment

Choose a reason for hiding this comment

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

Looks great, I think docs might be the only one not configured

@onlook/docs lint $ eslint . --max-warnings 0
│ [12 lines elided]
│     at wrapSafe (node:internal/modules/cjs/loader:1281:20)
│     at Module._compile (node:internal/modules/cjs/loader:1321:27)
│     at Module._extensions..js (node:internal/modules/cjs/loader:1416:10)
│     at Module.load (node:internal/modules/cjs/loader:1208:32)
│     at Module._load (node:internal/modules/cjs/loader:1024:12)
│     at cjsLoader (node:internal/modules/esm/translators:348:17)
│     at ModuleWrap.<anonymous> (node:internal/modules/esm/translators:297:7)
│     at ModuleJob.run (node:internal/modules/esm/module_job:222:25)
│     at async ModuleLoader.import (node:internal/modules/esm/loader:316:24)
│     at async dynamicImportConfig (/Users/kietho/workplace/onlook/clones/web/node_modules/eslint/lib/config/config-loader.js:186:17)
└─ Exited with code 2

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (2)
docs/package.json (2)

12-14: Speed up and ensure formatting coverage.

  • Add --cache for faster re-runs.
  • If Tailwind class sorting/MDX formatting is expected via Prettier, ensure ESLint config provides equivalent fixers; otherwise add a Prettier script.
- "lint": "eslint . --max-warnings 0",
- "format": "eslint --fix ."
+ "lint": "eslint . --max-warnings 0 --cache",
+ "format": "eslint --fix --cache ."

27-27: Use workspace protocol for internal toolchain.

"" can resolve to a published version. Prefer workspace: to pin to the local package.

- "@onlook/eslint": "*",
+ "@onlook/eslint": "workspace:*",
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 096fa12 and c6419da.

⛔ Files ignored due to path filters (1)
  • bun.lock is excluded by !**/*.lock
📒 Files selected for processing (5)
  • apps/web/client/package.json (2 hunks)
  • docs/package.json (2 hunks)
  • package.json (3 hunks)
  • packages/db/package.json (2 hunks)
  • packages/ui/package.json (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
  • package.json
  • packages/db/package.json
  • packages/ui/package.json
  • apps/web/client/package.json
🔇 Additional comments (1)
docs/package.json (1)

36-36: PostCSS version OK — no action required. Tailwind 4.1.x requires PostCSS v8 (tested with 8.4+); docs confirm package.json's "postcss": "^8.5.3" satisfies the peer range.

"name": "@onlook/docs",
"version": "0.0.0",
"private": true,
"type": "module",
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

🧩 Analysis chain

ESM flip: verify no CommonJS configs in this package.

"type": "module" will treat .js files as ESM. Any CJS config (e.g., next-sitemap.config.js, postcss.config.js, tailwind.config.js) using module.exports/require must be renamed to .cjs or converted.

Run to spot offenders:


🏁 Script executed:

#!/bin/bash
rg -n -C1 -g 'docs/**' -g '!**/node_modules/**' -P '(module\.exports|require\()' -- \
'**/*.config.js' 'docs/**/*.js'

Length of output: 107


🏁 Script executed:

#!/bin/bash
set -euo pipefail
echo "---- show docs/package.json (if present) ----"
if [ -f docs/package.json ]; then
  nl -ba docs/package.json | sed -n '1,200p'
else
  echo "docs/package.json: not found"
fi

echo
echo "---- search for CommonJS patterns (module.exports or require()) under docs/ ----"
rg -n -S -C1 -g 'docs/**' -g '!**/node_modules/**' -P '(module\.exports|require\()' || echo "No matches for module.exports or require() in docs/"

echo
echo "---- list config-like JS files under docs/ (tailwind/postcss/next-sitemap/*.config.js) ----"
rg --files -g 'docs/**' | rg -nE '\b(tailwind|postcss|next-sitemap|.*config)\.js$' || echo "No config-like JS files found under docs/"

echo
echo "---- count and sample .js files under docs/ ----"
rg --files -g 'docs/**' | rg '\.js$' | wc -l || true
rg --files -g 'docs/**' | rg '\.js$' | sed -n '1,50p' || true

echo
echo "---- finished ----"

Length of output: 86


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "PWD: $(pwd)"
echo
echo "---- docs/package.json (if present) ----"
if [ -f docs/package.json ]; then
  sed -n '1,200p' docs/package.json
else
  echo "docs/package.json: not found"
fi

echo
echo "---- docs/ file tree (up to depth 4) ----"
if [ -d docs ]; then
  find docs -maxdepth 4 -type f -print | sed -n '1,200p' || true
else
  echo "docs directory not found"
fi

echo
echo "---- search for CommonJS patterns (module.exports or require()) under docs/ ----"
if [ -d docs ]; then
  if command -v rg >/dev/null 2>&1; then
    rg -n -S -C1 -g 'docs/**' -g '!**/node_modules/**' -P '(module\.exports|require\()' || echo "No matches for module.exports or require() in docs/"
  else
    grep -nH -R --exclude-dir=node_modules -E '(module\.exports|require\()' docs || echo "No matches via grep"
  fi
else
  echo "skipping search: docs directory missing"
fi

echo
echo "---- find config-like JS files under docs/ ----"
if [ -d docs ]; then
  find docs -maxdepth 4 -type f \( -iname '*config.js' -o -iname 'tailwind*.js' -o -iname 'postcss*.js' -o -iname 'next-sitemap*.js' \) -print || echo "No config-like JS files found under docs/"
else
  echo "skipping find: docs directory missing"
fi

echo
echo "---- count .js files under docs/ ----"
if [ -d docs ]; then
  find docs -type f -name '*.js' | wc -l || true
else
  echo "0"
fi

echo
echo "---- finished ----"

Length of output: 3836


ESM flip — fix CommonJS configs

docs/package.json sets "type":"module", so .js files are ESM. Action required:

  • docs/next-sitemap.config.js — contains module.exports (line 2); rename to next-sitemap.config.cjs or convert to ESM.
  • docs/eslint.config.js — present as .js; verify for module.exports/require() and rename to .cjs or convert if needed.
  • Scan other .js config files under docs/ for CommonJS patterns and address them.
🤖 Prompt for AI Agents
In docs/package.json around line 5, "type":"module" makes all .js files ESM;
update the CommonJS config files: rename docs/next-sitemap.config.js to
docs/next-sitemap.config.cjs or convert its exports to ESM export default, do
the same for docs/eslint.config.js (rename to .cjs if it uses
module.exports/require or rewrite to ESM), and scan other .js config files under
docs/ for module.exports or require() patterns and either rename those to .cjs
or convert them to proper ESM exports/imports so configs remain compatible.

Comment on lines +31 to +32
"eslint": "^9.0.0",
"typescript": "^5.5.4",
Copy link

@coderabbitai coderabbitai bot Sep 23, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

🧩 Analysis chain

Align ESLint/TS versions repo‑wide and satisfy peers.

Ensure all packages resolve a single ESLint and a compatible TS version (and that @onlook/eslint peer ranges are satisfied) to avoid plugin mismatch.


🏁 Script executed:

#!/bin/bash
# Show unique versions across the repo
fd -a package.json | rg -v 'node_modules' | xargs -I{} jq -r '.name+"|eslint="+(.devDependencies.eslint // .dependencies.eslint // "")+"|ts="+(.devDependencies.typescript // .dependencies.typescript // "")+"|path={}"' {} | sort

Length of output: 2581


Align ESLint and TypeScript versions repo‑wide and satisfy @onlook/eslint peers.

  • Standardize eslint to "^9.0.0" and TypeScript to "^5.5.4" (majority currently) to avoid plugin/TS mismatches.
  • Add eslint "^9.0.0" where missing: apps/backend/package.json, packages/db/package.json, tooling/prettier/package.json, package.json, tooling/typescript/package.json, apps/web/package.json.
  • Normalize mismatched TS versions: packages/db/package.json (was ^5.8.2) and packages/scripts/package.json (was ^5.3.3) to the chosen TS version.
  • Verify @onlook/eslint peerDependencies are satisfied after these changes.
🤖 Prompt for AI Agents
In docs/package.json around lines 31-32: the repo needs consistent ESLint and
TypeScript versions to satisfy @onlook/eslint peers; update all package.json
files to standardize "eslint" to "^9.0.0" and "typescript" to "^5.5.4" as
follows: add "eslint": "^9.0.0" to apps/backend/package.json,
packages/db/package.json, tooling/prettier/package.json, package.json (root),
tooling/typescript/package.json, and apps/web/package.json if missing; change
typescript entries in packages/db/package.json and packages/scripts/package.json
from their current versions (^5.8.2 and ^5.3.3) to "^5.5.4"; then run install
and verify that @onlook/eslint peerDependencies are satisfied (adjust peer
ranges or package versions if any conflicts remain).

Copy link
Contributor

Choose a reason for hiding this comment

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

@saddlepaddle this is probably relevant but I'll just make a note and not change anything in this PR

Copy link

Choose a reason for hiding this comment

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

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

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