-
Notifications
You must be signed in to change notification settings - Fork 115
chore: add shared ESLint configuration package #625
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
base: main
Are you sure you want to change the base?
chore: add shared ESLint configuration package #625
Conversation
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.
1 issue found across 12 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/eslint-config/eslint.config.mjs">
<violation number="1" location="packages/eslint-config/eslint.config.mjs:37">
P2: The `globals` object is replaced instead of merged when passing custom `languageOptions`. When `browser()` or `node()` helpers are used, `globals.es2021` is lost because `...languageOptions` spreads at the object level, completely replacing the `globals` key. This could cause ESLint to report "undefined variable" errors for standard ES2021 globals.</violation>
</file>
Reply to cubic to teach it or ask questions. Tag @cubic-dev-ai to re-run a review.
| project, | ||
| ...(tsconfigRootDir && { tsconfigRootDir }), | ||
| }, | ||
| ...languageOptions, |
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.
P2: The globals object is replaced instead of merged when passing custom languageOptions. When browser() or node() helpers are used, globals.es2021 is lost because ...languageOptions spreads at the object level, completely replacing the globals key. This could cause ESLint to report "undefined variable" errors for standard ES2021 globals.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/eslint-config/eslint.config.mjs, line 37:
<comment>The `globals` object is replaced instead of merged when passing custom `languageOptions`. When `browser()` or `node()` helpers are used, `globals.es2021` is lost because `...languageOptions` spreads at the object level, completely replacing the `globals` key. This could cause ESLint to report "undefined variable" errors for standard ES2021 globals.</comment>
<file context>
@@ -0,0 +1,133 @@
+ project,
+ ...(tsconfigRootDir && { tsconfigRootDir }),
+ },
+ ...languageOptions,
+ },
+ plugins: {
</file context>
✅ Addressed in 3cef924
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.
Fixed
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.
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.
@yashhhguptaaa I have started the AI code review. It will take a few minutes to complete.
|
Formatting Strategy This PR includes only the configuration changes (ESLint/Prettier setup). No source code formatting has been applied yet. Rationale:
Current state:
After approval, I'll run: Please review the configuration changes, and once approved, I'll apply the formatting. |
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.
1 issue found across 12 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/eslint-config/package.json">
<violation number="1" location="packages/eslint-config/package.json:15">
P2: Redundant dependencies: `@typescript-eslint/eslint-plugin` and `@typescript-eslint/parser` are not needed when using `typescript-eslint`. The unified `typescript-eslint` package (v8+) already bundles the parser and plugin together, and the implementation only imports from `typescript-eslint`. These redundant packages increase install size and may cause version conflicts.</violation>
</file>
Reply to cubic to teach it or ask questions. Tag @cubic-dev-ai to re-run a review.
packages/eslint-config/package.json
Outdated
| "dependencies": { | ||
| "@eslint/js": "^9.25.0", | ||
| "@stylistic/eslint-plugin": "^4.2.0", | ||
| "@typescript-eslint/eslint-plugin": "^8.30.1", |
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.
P2: Redundant dependencies: @typescript-eslint/eslint-plugin and @typescript-eslint/parser are not needed when using typescript-eslint. The unified typescript-eslint package (v8+) already bundles the parser and plugin together, and the implementation only imports from typescript-eslint. These redundant packages increase install size and may cause version conflicts.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/eslint-config/package.json, line 15:
<comment>Redundant dependencies: `@typescript-eslint/eslint-plugin` and `@typescript-eslint/parser` are not needed when using `typescript-eslint`. The unified `typescript-eslint` package (v8+) already bundles the parser and plugin together, and the implementation only imports from `typescript-eslint`. These redundant packages increase install size and may cause version conflicts.</comment>
<file context>
@@ -0,0 +1,27 @@
+ "dependencies": {
+ "@eslint/js": "^9.25.0",
+ "@stylistic/eslint-plugin": "^4.2.0",
+ "@typescript-eslint/eslint-plugin": "^8.30.1",
+ "@typescript-eslint/parser": "^8.30.1",
+ "eslint-config-prettier": "^9.1.0",
</file context>
✅ Addressed in 0a45502
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.
fixed 0a45502
Closes #615
Summary
This PR adds a centralized ESLint and Prettier configuration package to standardize linting and formatting across the monorepo. It migrates all apps and packages to use the shared configuration, ensuring consistent code style and enabling auto-formatting on save.
Changes
New Packages
@probo/eslint-config: Shared ESLint configuration package with:eslint-plugin-prettierbrowser()helper)createConfig(),browser(), andnode()@probo/prettier: Enhanced Prettier configuration that aligns with ESLint stylistic rules:Migration
@probo/eslint-configand@probo/prettier@probo/eslint-configand@probo/prettier@probo/eslint-configand@probo/prettierCode Quality Improvements
eslintConfigsections from package.json filesTechnical Details
ESLint Rules
The shared config includes:
no-floating-promises,no-misused-promises,await-thenable, etc.Configuration Usage
Apps and packages now use a simple configuration:
Benefits
Testing
Migration Notes
For future packages, add:
@probo/eslint-configand@probo/prettiertodevDependencies"prettier": "@probo/prettier"topackage.jsoneslint.config.mjsusing thebrowser()ornode()helper