Migrate ESLint to flat config and update dependencies#147
Migrate ESLint to flat config and update dependencies#147google-labs-jules[bot] wants to merge 2 commits intomainfrom
Conversation
- Migrated `.eslintrc.cjs` to `eslint.config.ts` using the new flat config format. - Added `jiti`, `globals`, and `prettier-plugin-organize-imports` to `devDependencies`. - Migrated `prettier.config.js` files back to `prettier.config.ts`. - Updated the `format` script to use `bun` to run Prettier, allowing it to handle TypeScript configuration files. - Ran `bun format` to apply formatting changes. - The GitHub workflow for linting was not added as it would currently fail due to existing linting issues in the codebase.
|
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the 📝 WalkthroughSummary by CodeRabbit
WalkthroughReplaces the CommonJS ESLint config with a flat TypeScript-based eslint.config.ts, updates package.json tooling, and applies formatting-only changes to a test and a service file in backend. No functional code changes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Comment |
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
package.json (1)
15-22: Remove Legacy ESLint Config and Consolidate Flat ConfigWe’ve confirmed that both
frontendandbackendworkspaces have alintscript defined ("eslint"), so the rootbun -F '*' lintrouting is correct. However, there’s still a leftover legacy ESLint config in the frontend:
- frontend/.eslintrc-auto-import.json was detected by
find . -iname ".eslintrc*".To fully migrate to the new flat ESLint configuration and avoid conflicts, please:
• Remove
frontend/.eslintrc-auto-import.json(legacy JSON config).
• Migrate any necessary rule overrides or plugin settings from that file into your flat config (e.g..eslintrc.cjsoreslint.config.jsat the repo root or within each workspace).
• Verify that runningbun lint(orbun -F frontend lint) correctly picks up only the flat config and doesn’t fall back to any legacy.eslintrc.*.Once the legacy file is removed and settings consolidated, CI lint runs should pass without referencing outdated configs.
backend/src/services/constraint-validation.service.ts (2)
369-373: Fix TS6133: remove unused parameter 'constraint' in validateSingleValueConstraint.CI reports: TS6133: 'constraint' is declared but its value is never read. The parameter is unused.
Apply this diff:
- private validateSingleValueConstraint( - values: any[], - constraint: PropertyConstraint, - propertyId: string, - ): ConstraintViolation | null { + private validateSingleValueConstraint( + values: any[], + propertyId: string, + ): ConstraintViolation | null {
446-449: Update call site after removing the unused parameter.Adjust the invocation to match the new signature.
Apply this diff:
- case 'single value constraint': - const violation = this.validateSingleValueConstraint(values, constraint, propertyId) + case 'single value constraint': + const violation = this.validateSingleValueConstraint(values, propertyId) if (violation) violations.push(violation) break
🧹 Nitpick comments (12)
package.json (1)
17-17: Prefer bunx over invoking Prettier from node_modules/.bin.Directly calling node_modules binaries is brittle. bunx resolves the binary on PATH and handles workspace-resolved versions. It also aligns with your saved learning to use bun for scripts.
Apply this diff:
- "format": "bun node_modules/.bin/prettier --cache --log-level warn --ignore-unknown --write .", + "format": "bunx prettier --cache --log-level warn --ignore-unknown --write .",I used your retrieved learning about preferring bun for scripts.
eslint.config.ts (4)
1-3: Remove no-op jiti usage; ESLint 9 loads TS configs without this.Calling jiti(...) without using the returned require has no effect. It also adds overhead and an unnecessary dependency surface.
Apply this diff:
-import jiti from 'jiti' -jiti(new URL(import.meta.url).pathname) -Then drop jiti from devDependencies if it's not used elsewhere.
12-12: Consider switching to the type-aware parser service instead of a hardcoded project path.Type-aware linting via the parser service avoids brittle project path wiring in monorepos and improves performance. It’s the recommended path in recent typescript-eslint releases.
Apply this diff to languageOptions:
- ...tseslint.configs.recommended, + ...tseslint.configs.recommended, { languageOptions: { parser: tseslint.parser, parserOptions: { - project: './tsconfig.base.json', - sourceType: 'module', + // Use the TS project service instead of explicit project globs. + projectService: true, + sourceType: 'module', },Please confirm tseslint version in your lockfile supports parserOptions.projectService (it should for v8+). If not, we can fall back to explicit project globs per workspace.
33-34: Prefer eslint-config-prettier over eslint-plugin-prettier in flat configs.Running Prettier as an ESLint rule is slower and no longer recommended. Using eslint-config-prettier disables conflicting rules while you keep formatting in a separate Prettier step (you already have a format script).
Apply this diff:
-import eslintPluginPrettierRecommended from 'eslint-plugin-prettier/recommended' +import eslintConfigPrettier from 'eslint-config-prettier' @@ - eslintPluginPrettierRecommended, + eslintConfigPrettier,After this change, ensure your Prettier plugin (prettier-plugin-organize-imports) is loaded via your Prettier config (plugins field), since it won’t be triggered by ESLint anymore.
10-11: Minor: exclude declaration files only if necessary.Ignoring '**/*.d.ts' hides potential issues in ambient type declarations. If this was added to speed up linting, consider scoping rules or files globs instead of global ignore.
If you keep it, add a short comment for future maintainers about the rationale (perf vs. signal).
backend/src/services/constraint-validation.service.ts (4)
129-137: Enable parameter extraction for common constraints (regex, min/max, etc.).The parameterMap is fully commented out, so no parameters are extracted from qualifiers. This disables validations like regex and ranges when constraints come from Wikibase.
Apply this diff:
- const parameterMap: Record<string, string> = { - // 'P2306': 'property', // property - // 'P2305': 'item', // item of property constraint - // 'P2303': 'regex', // regular expression - // 'P2313': 'minimum_value', // minimum value - // 'P2312': 'maximum_value', // maximum value - // 'P2308': 'class', // class - // 'P2309': 'relation', // relation - // 'P2316': 'constraint_status', // constraint status - } + const parameterMap: Record<string, string> = { + P2306: 'property', // property + P2305: 'item', // item of property constraint + P2303: 'regex', // regular expression + P2313: 'minimum_value', // minimum value + P2312: 'maximum_value', // maximum value + P2308: 'class', // class + P2309: 'relation', // relation + P2316: 'constraint_status', // constraint status + }If the upstream constraint model maps to different P-codes on your instance, adjust accordingly.
219-226: Support both 'regex' and 'pattern' parameter names for format constraints.Your tests use parameters.pattern, while the service expects parameters.regex. Add a fallback to avoid silent no-ops.
You can update the extraction or make validateFormatConstraint tolerant:
const regex = constraint.parameters?.regex ?? constraint.parameters?.patternThis keeps tests meaningful even if upstream provides 'pattern' instead of 'regex'.
Consider aligning the tests as well (see test file comment) to reduce ambiguity.
256-266: Allowed values: consider handling { id: string } and raw strings only.Currently, valueId falls back to the entire object if it lacks an id, which produces noisy messages like [object Object]. If your values are of shape { type, content }, compare against content (or restrict to id/string).
Example tweak:
const valueId = typeof value === 'object' ? ('id' in value ? value.id : ('content' in value ? value.content : String(value))) : value
561-567: Cache invalidation: optional improvement.Deleting keys with startsWith is fine. If cache churn grows, consider tracking per-instance keys in a separate Map<instanceId, Set> to avoid scanning all keys.
Happy to sketch this out if you see cache growth in prod.
backend/src/services/__tests__/constraint-validation.service.test.ts (3)
10-11: Align mock constraint parameter with service expectation ('regex' vs 'pattern').Service checks constraint.parameters.regex. Tests set parameters.pattern, so format validation is effectively skipped.
Apply this diff:
- parameters: { pattern: '^[A-Z]{2}$' }, + parameters: { regex: '^[A-Z]{2}$' },Alternatively, keep 'pattern' and adopt the service-side fallback suggested in the service file comment.
63-74: Strengthen the single-value constraint assertion.toBeGreaterThanOrEqual(0) always passes and won’t catch regressions. Assert at least one violation and invalidity.
Apply this diff:
- // Should have violations for single value constraint - expect(result.violations.length).toBeGreaterThanOrEqual(0) + // Should have violations for single value constraint + expect(result.violations.length).toBeGreaterThan(0) + expect(result.isValid).toBe(false)
110-116: Mock call expectation is fine; optional: assert cache hit on second call.You could add a second call and assert the mock wasn't called again to exercise the cache.
Example:
await service.getPropertyConstraints('wikidata', 'P1') await service.getPropertyConstraints('wikidata', 'P1') expect(service.getPropertyConstraints).toHaveBeenCalledTimes(2) // or spy on underlying client fetch for a stricter check
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
.eslintrc.cjs(0 hunks)backend/src/services/__tests__/constraint-validation.service.test.ts(3 hunks)backend/src/services/constraint-validation.service.ts(7 hunks)eslint.config.ts(1 hunks)package.json(2 hunks)
💤 Files with no reviewable changes (1)
- .eslintrc.cjs
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-20T14:13:40.153Z
Learnt from: CR
PR: DaxServer/dataforge#0
File: .cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc:0-0
Timestamp: 2025-07-20T14:13:40.153Z
Learning: Applies to package.json : Use `bun run <script>` instead of `npm run <script>`, `yarn run <script>`, or `pnpm run <script>` for running scripts
Applied to files:
package.json
🪛 GitHub Actions: Typecheck code
package.json
[warning] 1-1: Syncpack: Dependency 'react' has mismatched versions across workspace (18.0.0 vs 18.2.0).
backend/src/services/constraint-validation.service.ts
[error] 371-371: bun typecheck: TS6133: 'constraint' is declared but its value is never read.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (2)
package.json (2)
5-8: Dev deps alignment looks consistent with the ESLint flat-config migration.
- globals is used in eslint.config.ts.
- jiti is referenced in eslint.config.ts (see separate comment about whether it's needed).
- prettier-plugin-organize-imports matches the new formatting workflow.
33-53: Unify React versions via package catalogSyncpack flags mismatched React versions (e.g. 18.0.0 vs 18.2.0). Add a single, consistent React version to the catalog and verify across all workspaces.
• File & lines to update
– package.json, lines 33–53• Catalog diff (pick your standard version, e.g. 18.2.0):
"typescript": "^5.9.0" + ,"react": "18.2.0" + ,"react-dom": "18.2.0"• Ensure syncpack is available
– npm:npm install --save-dev syncpack
– Bun:bun install --dev syncpack• Verify current mismatches and locate all React declarations
# List workspace version mismatches npx syncpack list-mismatches || bun x syncpack list-mismatches # Find every react entry in package.json files rg -n '"react":\s*".*"' -g '**/package.json'• Apply fixes and re-verify
# Install updated catalog npm install # or bun install # Auto-fix mismatches npx syncpack fix-mismatches || bun x syncpack fix-mismatches # Confirm no leftover mismatches npx syncpack list-mismatches || bun x syncpack list-mismatchesThis will prevent hoisting surprises and subtle runtime issues by enforcing a single React version across the monorepo.
- Migrated `.eslintrc.cjs` to `eslint.config.ts` using the new flat config format. - Added `jiti`, `globals`, and `prettier-plugin-organize-imports` to `devDependencies`. - Migrated `prettier.config.js` files back to `prettier.config.ts`. - Updated the `format` script to use `bun` to run Prettier, allowing it to handle TypeScript configuration files. - Ran `bun format` to apply formatting changes. - Removed an incorrect `jiti()` call from the new `eslint.config.ts`. This commit also includes changes from a rebase of the main branch, as instructed, to resolve merge conflicts. This incorporates new dependencies and other updates from the main branch.
1 participant
This change migrates the ESLint configuration to the new flat config format, adds several development dependencies, and updates the Prettier configuration and scripts to support TypeScript configuration files.