chore: fix/check tooling improvements with turbo#8304
chore: fix/check tooling improvements with turbo#8304sriramveeraghanta merged 1 commit intopreviewfrom
Conversation
WalkthroughEnabled ESLint and Prettier caching across many packages, adjusted ESLint rule placement in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
eslint.config.mjs (1)
138-141: Lint rule tweaks look correct; consider future non-component exports
- The updated
react-refresh/only-export-componentsrule withallowExportNames: ["meta", "links", "headers", "loader", "action"]matches typical React Router route module exports, so this should reduce false positives without weakening Fast Refresh safeguards.- Moving
import/consistent-type-specifier-styleinto the TS override with["error", "prefer-top-level"]is consistent with the repo preference for top‑levelimport typestatements instead of inline type imports. Based on learnings.If you later add other non-component route exports (e.g., additional metadata handlers), you may want to extend
allowExportNamesto avoid new false positives.Also applies to: 163-165
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (19)
apps/admin/package.json(1 hunks)apps/live/package.json(1 hunks)apps/space/package.json(1 hunks)apps/web/package.json(1 hunks)eslint.config.mjs(2 hunks)packages/constants/package.json(1 hunks)packages/decorators/package.json(1 hunks)packages/editor/package.json(1 hunks)packages/editor/src/core/hooks/use-yjs-setup.ts(1 hunks)packages/hooks/package.json(1 hunks)packages/i18n/package.json(1 hunks)packages/logger/package.json(1 hunks)packages/propel/package.json(1 hunks)packages/services/package.json(1 hunks)packages/shared-state/package.json(1 hunks)packages/types/package.json(1 hunks)packages/ui/package.json(1 hunks)packages/utils/package.json(1 hunks)turbo.json(3 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{js,mjs,cjs}
📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
Use
@importtags in JSDoc for cleaner type imports in JavaScript files when working in a mixed codebase (TypeScript 5.5+)
Files:
eslint.config.mjs
{turbo.json,**/*.sh}
📄 CodeRabbit inference engine (.github/instructions/bash.instructions.md)
Use Turbo for build system orchestration with configuration in turbo.json
Files:
turbo.json
**/*.{ts,tsx,mts,cts}
📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
**/*.{ts,tsx,mts,cts}: Useconsttype parameters for more precise literal inference in TypeScript 5.0+
Use thesatisfiesoperator to validate types without widening them
Leverage inferred type predicates to reduce the need for explicitisreturn types in filter/check functions
UseNoInfer<T>utility to block inference for specific type arguments when they should be determined by other arguments
Utilize narrowing inswitch(true)blocks for control flow analysis (TypeScript 5.3+)
Rely on narrowing from direct boolean comparisons for type guards
Trust preserved narrowing in closures when variables aren't modified after the check (TypeScript 5.4+)
Use constant indices to narrow object/array properties (TypeScript 5.5+)
Use standard ECMAScript decorators (Stage 3) instead of legacyexperimentalDecorators
Useusingdeclarations for explicit resource management with Disposable pattern instead of manual cleanup (TypeScript 5.2+)
Usewith { type: "json" }for import attributes; avoid deprecatedassertsyntax (TypeScript 5.3/5.8+)
Useimport typeexplicitly when importing types to ensure they are erased during compilation, respectingverbatimModuleSyntaxflag
Use.ts,.mts,.ctsextensions inimport typestatements (TypeScript 5.2+)
Useimport type { Type } from "mod" with { "resolution-mode": "import" }for specific module resolution contexts (TypeScript 5.3+)
Use new iterator methods (map, filter, etc.) if targeting modern environments (TypeScript 5.6+)
Utilize newSetmethods likeunion,intersection, etc., when available (TypeScript 5.5+)
UseObject.groupBy/Map.groupBystandard methods for grouping instead of external libraries (TypeScript 5.4+)
UsePromise.withResolvers()for creating promises with exposed resolve/reject functions (TypeScript 5.7+)
Use copying array methods (toSorted,toSpliced,with) for immutable array operations (TypeScript 5.2+)
Avoid accessing instance fields viasuperin classes (TypeScript 5....
Files:
packages/editor/src/core/hooks/use-yjs-setup.ts
🧠 Learnings (13)
📚 Learning: 2025-11-25T10:17:39.709Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: .github/instructions/bash.instructions.md:0-0
Timestamp: 2025-11-25T10:17:39.709Z
Learning: Applies to {turbo.json,**/*.sh} : Use Turbo for build system orchestration with configuration in turbo.json
Applied to files:
packages/types/package.jsonpackages/decorators/package.jsonpackages/utils/package.jsonpackages/constants/package.jsonapps/admin/package.jsonpackages/hooks/package.jsonturbo.jsonpackages/i18n/package.jsonapps/space/package.jsonpackages/services/package.jsonpackages/editor/package.jsonpackages/shared-state/package.jsonapps/live/package.jsonpackages/logger/package.jsonpackages/propel/package.json
📚 Learning: 2025-11-25T10:18:05.172Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-11-25T10:18:05.172Z
Learning: Applies to **/*.{ts,tsx,mts,cts} : Use standard ECMAScript decorators (Stage 3) instead of legacy `experimentalDecorators`
Applied to files:
packages/decorators/package.jsoneslint.config.mjs
📚 Learning: 2025-11-25T10:18:05.172Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-11-25T10:18:05.172Z
Learning: Applies to **/*.{ts,tsx,mts,cts} : Use `with { type: "json" }` for import attributes; avoid deprecated `assert` syntax (TypeScript 5.3/5.8+)
Applied to files:
eslint.config.mjsturbo.jsonpackages/editor/src/core/hooks/use-yjs-setup.ts
📚 Learning: 2025-11-25T10:18:05.172Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-11-25T10:18:05.172Z
Learning: Applies to **/*.{js,mjs,cjs} : Use `import` tags in JSDoc for cleaner type imports in JavaScript files when working in a mixed codebase (TypeScript 5.5+)
Applied to files:
eslint.config.mjs
📚 Learning: 2025-11-25T10:18:05.172Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-11-25T10:18:05.172Z
Learning: Applies to **/*.{ts,tsx,mts,cts} : Use `.ts`, `.mts`, `.cts` extensions in `import type` statements (TypeScript 5.2+)
Applied to files:
eslint.config.mjs
📚 Learning: 2025-11-25T10:18:05.172Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-11-25T10:18:05.172Z
Learning: Applies to **/*.{ts,tsx,mts,cts} : Treat legacy `experimentalDecorators`-style behavior as deprecated in favor of standard TC39-compliant decorators
Applied to files:
eslint.config.mjs
📚 Learning: 2025-11-25T10:18:05.172Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-11-25T10:18:05.172Z
Learning: Applies to **/*.{ts,tsx,mts,cts} : Avoid `import ... assert` syntax; use `with` instead for import attributes
Applied to files:
eslint.config.mjs
📚 Learning: 2025-11-25T10:18:05.172Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-11-25T10:18:05.172Z
Learning: Applies to **/*.{ts,tsx,mts,cts} : Use `import type { Type } from "mod" with { "resolution-mode": "import" }` for specific module resolution contexts (TypeScript 5.3+)
Applied to files:
eslint.config.mjs
📚 Learning: 2025-11-25T10:18:05.172Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-11-25T10:18:05.172Z
Learning: Applies to **/*.{ts,tsx,mts,cts} : Use `import type` explicitly when importing types to ensure they are erased during compilation, respecting `verbatimModuleSyntax` flag
Applied to files:
eslint.config.mjs
📚 Learning: 2025-11-25T10:18:05.172Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-11-25T10:18:05.172Z
Learning: Applies to **/*.{ts,tsx,mts,cts} : Use `const` type parameters for more precise literal inference in TypeScript 5.0+
Applied to files:
eslint.config.mjs
📚 Learning: 2025-11-25T10:18:05.172Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-11-25T10:18:05.172Z
Learning: Applies to **/*.{ts,tsx,mts,cts} : Avoid `enum`s if the project prefers erasable syntax, using `const` objects or unions instead (TypeScript 5.8+)
Applied to files:
eslint.config.mjs
📚 Learning: 2025-11-25T10:18:05.172Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-11-25T10:18:05.172Z
Learning: Be aware of `--erasableSyntaxOnly` flag; avoid TypeScript-specific syntax that cannot be simply erased if the project aims for maximum compatibility with tools like Node.js's `--strip-types`
Applied to files:
eslint.config.mjs
📚 Learning: 2025-10-01T15:30:17.605Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7888
File: packages/propel/src/avatar/avatar.stories.tsx:2-3
Timestamp: 2025-10-01T15:30:17.605Z
Learning: In the makeplane/plane repository, avoid suggesting inline type imports (e.g., `import { Avatar, type TAvatarSize }`) due to bundler compatibility issues. Keep type imports and value imports as separate statements.
Applied to files:
eslint.config.mjs
⏰ 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). (5)
- GitHub Check: Agent
- GitHub Check: CodeQL analysis (javascript-typescript)
- GitHub Check: Build packages
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (18)
packages/editor/src/core/hooks/use-yjs-setup.ts (1)
190-190: Y.Doc assignment without explicit cast looks correctRemoving the
as Y.DockeepsyjsSession.ydocaligned with the provider’s declareddocumenttype and avoids hiding potential type mismatches; no runtime behavior change here, and usages ofyjsSession.ydocremain consistent.Please confirm your installed
@hocuspocus/providertypes still declaredocumentas aY.Docso this stays compile-clean with your current toolchain.packages/constants/package.json (1)
10-14: ESLint/Prettier caching flags in scripts look consistentThe updated
check:lint/fix:lintandcheck:format/fix:formatscripts correctly add ESLint--cache --cache-location node_modules/.cache/eslint/and Prettier--cache, matching the monorepo pattern.After merging, run the lint/format tasks once to confirm caches are created at the expected paths and that your ESLint/Prettier versions fully support the
--cacheflags.packages/hooks/package.json (1)
18-22: Hook package lint/format scripts updated correctly for cachingAdding ESLint cache (
--cache --cache-location node_modules/.cache/eslint/) and Prettier cache (--cache) to the check/fix scripts is consistent with other packages and should speed up repeat runs.Please verify in CI or locally that these scripts pick up the shared ESLint/Prettier configs as expected and that cache directories are recognized by Turbo’s config.
packages/utils/package.json (1)
18-22: Utils lint/format scripts: cache configuration looks goodThe ESLint and Prettier scripts now use caching in the same way as other packages, with cache data under
node_modules/.cache/eslint/and Prettier--cacheenabled.Run the utils lint/format tasks once to ensure the new cache behavior works and that Turbo (if configured) tracks these cache directories correctly.
apps/live/package.json (1)
18-22: Live app tooling scripts now aligned with cached lint/format patternThe
check:lint/fix:lintandcheck:format/fix:formatscripts correctly add ESLint and Prettier caching flags, matching the rest of the repo’s configuration.Confirm that running these scripts in the live app context generates and reuses the cache as expected, especially when invoked via Turbo pipelines.
packages/decorators/package.json (1)
15-19: Decorators package lint/format caching matches monorepo standardEnabling ESLint
--cachewith an explicit cache location and Prettier--cacheon check/write keeps this package in line with the shared tooling setup.Please verify these scripts integrate cleanly with your Turbo tasks (if any) so cached outputs are properly tracked and reused. Based on learnings, this keeps tooling orchestration consistent.
packages/types/package.json (1)
14-18: Types package ESLint/Prettier cache flags look correctThe updated
check:lint/fix:lintandcheck:format/fix:formatscripts correctly adopt the shared caching pattern and shouldn’t affect the published types surface.Run the types package lint/format tasks to ensure the cache usage is effective and doesn’t interfere with any typegen or build steps that might run alongside them.
apps/web/package.json (1)
13-17: Web app lint/format scripts updated to use cachingThe additions of ESLint
--cache --cache-location node_modules/.cache/eslint/and Prettier--cacheto the check/fix scripts are consistent with the wider tooling changes and should speed up repeated runs on this large app.Given the size of this app, it’s worth validating that the new caches significantly reduce lint/format times and that CI/Turbo treat the cache directories as intended.
packages/propel/package.json (1)
10-14: Lint/format caching wiring looks consistent and safeThe updated
check:*/fix:*scripts correctly enable ESLint and Prettier caching with a shared cache dir and keep existing--max-warningsbehavior. This matches the monorepo pattern and should only improve performance without changing results.apps/admin/package.json (1)
14-18: Admin lint/format scripts aligned with shared caching patternThe added
--cacheflags andnode_modules/.cache/eslint/location mirror other packages and keep the existing--max-warningsand typegen behavior intact. Looks good.packages/logger/package.json (1)
15-19: Logger tooling caching added without relaxing strictnessEnabling ESLint/Prettier caching here while keeping
--max-warnings=0preserves the strict logger standards and should speed up repeated runs. No issues.turbo.json (1)
3-3: Turbo task wiring correctly tracks ESLint/Prettier caches and config
- Adding
.prettierrcandeslint.config.mjstoglobalDependenciesmeanscheck/fixwill re-run when tool config changes, which is what you want.- The
check:*andfix:*outputs now point atnode_modules/.cache/prettier/**andnode_modules/.cache/eslint/**, matching the default Prettier cache location and the explicit ESLint--cache-locationused in package scripts, so Turbo’s caching should line up with the CLIs. (prettier.io)- Giving
fix:lintadependsOn: ["^build"]and explicit inputs mirrorscheck:lintand keeps ordering predictable.Looks solid; after this lands, it’s worth running
turbo run check fix --filter=<one-package>locally to confirm the cache hits behave as expected in your environment.Also applies to: 56-57, 61-62, 81-83, 85-88
apps/space/package.json (1)
13-17: Space app lint/format caching matches repo-wide patternThe new
--cacheflags and ESLint cache-location are consistent with other apps and with the Turbo task outputs. Existing warning limits and type checking remain unchanged, so this is a safe performance win.packages/shared-state/package.json (1)
21-25: Shared-state tooling updates are consistent and non-breakingEnabling ESLint and Prettier caching here mirrors the rest of the workspace and keeps the existing
--max-warningsthreshold intact. No functional impact beyond faster repeated checks.packages/editor/package.json (1)
21-25: Editor package adopts shared ESLint/Prettier caching without runtime changesThe editor’s
check:*/fix:*scripts now use the same ESLint/Prettier cache conventions as the rest of the monorepo, with--max-warningspreserved. This is a tooling-only improvement and looks good.packages/services/package.json (1)
17-17: Caching configuration applied correctly.The ESLint and Prettier caching flags are properly configured with explicit cache locations for ESLint. This aligns with the monorepo-wide tooling improvements and should improve build performance without affecting functionality. Existing
--max-warningsconstraints are preserved.Also applies to: 19-21
packages/ui/package.json (1)
25-25: Caching configuration applied consistently.ESLint and Prettier caching is configured identically to other packages, with the appropriate
--max-warnings=643threshold preserved for the UI package. The changes are safe and should provide performance benefits across the monorepo's build pipeline.Also applies to: 27-29
packages/i18n/package.json (1)
18-18: Caching configuration properly applied.The i18n package follows the same consistent caching pattern with
--max-warnings=51, reflecting this package's stricter linting requirements. All changes align with the monorepo-wide tooling improvements.Also applies to: 20-22
da1588e to
2cba429
Compare
There was a problem hiding this comment.
Pull request overview
This PR optimizes the development tooling by enabling caching for ESLint and Prettier across the monorepo, and updates Turbo configuration to leverage these caches. The changes improve build and linting performance by allowing Turbo to cache formatting and linting operations, and make some ESLint configuration improvements.
Key Changes:
- Enabled ESLint and Prettier caching with
--cacheflags and configured cache locations atnode_modules/.cache/ - Updated Turbo configuration to track cache directories as outputs and enable caching for
fix:formatandfix:linttasks - Added
.prettierrcandeslint.config.mjsto global dependencies to invalidate caches when config changes - Reorganized ESLint rules and updated react-refresh configuration for React Router compatibility
- Removed unnecessary type assertion in editor YJS setup
Reviewed changes
Copilot reviewed 19 out of 20 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| turbo.json | Added config files to globalDependencies; configured outputs for check/fix tasks; enabled caching for fix tasks |
| packages/*/package.json | Added --cache flags to eslint/prettier commands across all packages for consistent caching |
| apps/*/package.json | Added --cache flags to eslint/prettier commands across all apps for consistent caching |
| eslint.config.mjs | Moved import rule to TypeScript block; updated react-refresh config for React Router |
| packages/editor/src/core/hooks/use-yjs-setup.ts | Removed unnecessary type assertion on provider.document |
| pnpm-lock.yaml | Lockfile updates from dependency resolution |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
2cba429 to
f0f3169
Compare
Type of Change
Summary by CodeRabbit
Chores
Style
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.