chore(monorepo): 🤖 make linting, formatting centralized#955
chore(monorepo): 🤖 make linting, formatting centralized#955punkbit wants to merge 21 commits intochore/design-tokens-packagefrom
Conversation
|
|
Claude finished @punkbit's task in 1m 22s —— View job PR Review: chore(monorepo): make linting, formatting centralized
Nice work on this! The approach of exporting named constants from the root 💡 Suggestions (non-blocking)
✅ The core goal is solid — one source of truth for formatting/linting, clean composition at the package level, and correct exclusion of design-tokens from ESLint. The PR works as described. The points above are all minor hygiene items, not blockers. |
There was a problem hiding this comment.
Pull request overview
Centralizes linting/formatting and TypeScript base configuration at the monorepo root so packages share a single source of truth and can run ESLint/Prettier from top-level devDependencies.
Changes:
- Add root shared ESLint flat config (
eslint.config.ts) and root TS configs (tsconfig.base.json,tsconfig.eslint.json), then update packages to extend them. - Move ESLint/Prettier (and related plugins) to root devDependencies; update workspace scripts to run tools via
yarn run -T. - Add root
.prettierignoreand adjust.prettierrcto rely on Prettier’s inferred parsers.
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| yarn.lock | Reflects moving tooling deps to the root workspace and adding jiti; updates resolved Prettier version. |
| tsconfig.eslint.json | New root TS config intended for ESLint config/type-aware linting. |
| tsconfig.base.json | New shared TS compiler options for packages to extend. |
| packages/design-tokens/tsconfig.json | Simplified to extend the new root base TS config. |
| packages/design-tokens/package.json | Switch Prettier scripts to use root-installed Prettier via yarn run -T; remove local Prettier dep. |
| packages/click-ui/tsconfig.json | Switch to extending the new root base TS config; keep package-specific options. |
| packages/click-ui/tsconfig.eslint.json | New package ESLint TS project config (paths/baseUrl + includes). |
| packages/click-ui/package.json | Update lint/format scripts to run root tools via yarn run -T; remove per-package ESLint/Prettier deps. |
| packages/click-ui/eslint.config.ts | Refactors to import and extend the new shared root ESLint config pieces. |
| packages/click-ui/.scripts/bash/format | Removes package-local formatting wrapper script in favor of direct Prettier invocation. |
| package.json | Adds root type: module and centralizes ESLint/Prettier-related devDependencies at the root. |
| eslint.config.ts | Introduces the shared root ESLint flat config building blocks (ignores/base/rules/plugins). |
| .prettierrc | Removes forced TypeScript parser setting. |
| .prettierignore | Adds root ignore rules (dist/node_modules/.yarn/yarn.lock). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "baseUrl": ".", | ||
| "paths": { | ||
| "@/*": ["src/*"] | ||
| } |
There was a problem hiding this comment.
tsconfig.eslint.json is now the parserOptions.project for ESLint, but it doesn’t include the test/global typings used in src/**/*.test.tsx (e.g. describe, it, expect, vi from vitest/globals). This can degrade type-aware linting and can surface TypeScript semantic diagnostics during lint runs. Consider adding the same compilerOptions.types as tsconfig.json (e.g. vitest/globals, @testing-library/jest-dom) or extending from tsconfig.json and overriding only include/exclude.
| } | |
| }, | |
| "types": ["vitest/globals", "@testing-library/jest-dom"] |
| "globals": "^16.5.0", | ||
| "husky": "^9.1.7", | ||
| "jiti": "^2.4.0", | ||
| "prettier": "^3.7.4", |
There was a problem hiding this comment.
Root prettier was changed to a caret range (^3.7.4), which is already resolving to 3.8.1 in yarn.lock. Since formatting consistency is the goal of this PR (and @clickhouse/click-ui previously pinned Prettier), consider pinning Prettier to an exact version (or adding a Yarn resolution) to avoid formatting diffs when new minor versions are released.
| "prettier": "^3.7.4", | |
| "prettier": "3.8.1", |
package.json
Outdated
| }, | ||
| "devDependencies": { | ||
| "husky": "^9.1.7" | ||
| "@typescript-eslint/eslint-plugin": "^8.54.0", |
EvandroLG
left a comment
There was a problem hiding this comment.
Approving, but I think we should get rid of the package @typescript-eslint/eslint-plugin. The typescript-eslint package already bundles the recommended rules and configs.
The following comments is to clarify other reviewers that this package been removed in main beforehand here #943, once these branch PR are updated with latest from main, the package will not be present. |
814d7ad to
6385b7a
Compare
…notation for sharedRules allows it to be spread into any ESLint config's rules object without type conflicts
… for ESLint rule configurations
65210ea to
af27131
Compare
|
Claude finished @punkbit's task in 54s —— View job PR Review: chore(monorepo): make linting, formatting centralized
Nice work on the centralization! The named-export pattern from the root 🚨 Must Fix
{
"extends": "../../tsconfig.eslint.json",
"compilerOptions": {
"baseUrl": ".",
"paths": { "@/*": ["src/*"] },
"types": ["vitest/globals", "@testing-library/jest-dom"]
},
"include": ["src/**/*.ts", "src/**/*.tsx"]
}💡 Suggestions (non-blocking)
✅ The core approach is solid — single source of truth, clean composition, correct design-tokens exclusion. The test types fix is the only thing that could cause lint failures in practice. |
…typescript-eslint)
📚 Storybook Preview Deployed✅ Preview URL: https://click-2mpy9d23e-clickhouse.vercel.app Built from commit: |
Why?
A single root-level config ensures all packages use identical formatting rules from one source of truth making maintenance simpler (one file to update), avoiding style drift between packages formatting and linting rules.
How?
Tickets?
N/A
Contribution checklist?
buildcommand runs locallySecurity checklist?
dangerouslySetInnerHTMLPreview?