chore: move lint & prettier configs to root #590
Conversation
🦋 Changeset detectedLatest commit: c7b7990 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Warning Rate limit exceeded@thucpn has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 1 minutes and 15 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the 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. 📒 Files selected for processing (43)
WalkthroughThis change centralizes and streamlines code formatting and linting configurations across the project. It introduces a new root-level Prettier configuration and ignore file, removes package-specific Prettier and ESLint configuration files, and consolidates related scripts and dependencies into the root Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant RootPrettier
participant RootESLint
Developer->>RootPrettier: Run format/format:write script
RootPrettier->>Developer: Format code using root config and ignore
Developer->>RootESLint: Run lint script
RootESLint->>Developer: Lint code with package-specific overrides
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
package.json (1)
18-24: Enhance lint/format scripts with explicit file extensions
Right noweslint .andprettier .will pick up default file types, but it’s safer (and faster) to scope them to the extensions you actually use. For example:"scripts": { - "lint": "eslint .", + "lint": "eslint --ext .js,.jsx,.ts,.tsx .", - "format": "prettier --ignore-unknown --cache --check .", + "format": "prettier --ignore-unknown --cache --check \"**/*.{js,jsx,ts,tsx,css,md}\" .", - "format:write": "prettier --ignore-unknown --write ." + "format:write": "prettier --ignore-unknown --write \"**/*.{js,jsx,ts,tsx,css,md}\" ." }This will ensure only relevant files are processed and can improve performance.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
.prettierignore(1 hunks)eslint.config.mjs(2 hunks)package.json(1 hunks)packages/create-llama/.eslintrc(0 hunks)packages/create-llama/.prettierignore(0 hunks)packages/create-llama/package.json(0 hunks)packages/create-llama/prettier.config.js(0 hunks)packages/server/.prettierignore(0 hunks)packages/server/package.json(1 hunks)packages/server/prettier.config.mjs(0 hunks)prettier.config.mjs(1 hunks)
💤 Files with no reviewable changes (6)
- packages/server/.prettierignore
- packages/create-llama/.eslintrc
- packages/create-llama/prettier.config.js
- packages/server/prettier.config.mjs
- packages/create-llama/package.json
- packages/create-llama/.prettierignore
⏰ Context from checks skipped due to timeout of 90000ms (27)
- GitHub Check: typescript (22, 3.11, ubuntu-22.04, nextjs, --llamacloud)
- GitHub Check: typescript (22, 3.11, ubuntu-22.04, nextjs, --example-file)
- GitHub Check: typescript (22, 3.11, ubuntu-22.04, nextjs, --no-files)
- GitHub Check: typescript (22, 3.11, windows-latest, nextjs, --llamacloud)
- GitHub Check: typescript (22, 3.11, windows-latest, nextjs, --example-file)
- GitHub Check: typescript (22, 3.11, windows-latest, nextjs, --no-files)
- GitHub Check: typescript (22, 3.11, macos-latest, nextjs, --llamacloud)
- GitHub Check: typescript (22, 3.11, macos-latest, nextjs, --example-file)
- GitHub Check: typescript (22, 3.11, macos-latest, nextjs, --no-files)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --llamacloud)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --example-file)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --llamacloud)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --no-files)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --example-file)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --llamacloud)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --no-files)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --example-file)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --llamacloud)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --no-files)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --example-file)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --llamacloud)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --example-file)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --no-files)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --no-files)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --llamacloud)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --example-file)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --no-files)
🔇 Additional comments (10)
.prettierignore (1)
1-9: Effective configuration for ignoring build artifacts and dependenciesThis new root-level
.prettierignorefile appropriately excludes common build artifacts, dependency directories, and generated files from Prettier formatting. The centralization aligns with the PR objective of moving formatting configurations to the root level, making maintenance easier.prettier.config.mjs (1)
1-4: Clean configuration with proper pluginThe configuration properly includes the
prettier-plugin-organize-importsplugin which will automatically organize imports in files. The TODO comment acknowledges the plan to addprettier-plugin-tailwindcssin the future, which aligns with the PR objective.Keep in mind that adding the Tailwind plugin later will likely require significant formatting changes across the codebase as mentioned in your TODO comment.
packages/server/package.json (2)
38-38: Successfully removed linting and formatting scriptsThe package.json has been streamlined by removing all linting and formatting-related scripts, keeping only the essential
devscript. This aligns perfectly with the PR objective of centralizing these configurations at the root level.
40-51: Properly removed formatting and linting dependenciesThe devDependencies have been correctly streamlined to remove all ESLint, Prettier, and TypeScript ESLint-related packages. This reinforces the centralization of tooling at the root level and reduces package-level maintenance overhead.
eslint.config.mjs (3)
20-25: Good package-specific configuration for create-llamaYou've properly set up specific linting rules for the create-llama package, preserving the essential rules from its previous configuration. This ensures consistent code quality while centralizing the configuration.
27-31: Server package configuration includes proper TypeScript rulesThe configuration for server packages correctly applies the TypeScript ESLint recommended rules. The TODO comment acknowledges the plan to eventually unify the rules across packages, which is good documentation of technical debt.
43-47: Appropriate ignore patternsThe ignore patterns have been correctly extended to include additional directories (
python/**and**/e2e/cache/**) that should be excluded from linting. This prevents unnecessary linting of non-source code files.package.json (3)
2-17: Meta configuration looks correct
The root-level fields (name,version,private,description,keywords,repository,license, andworkspaces) are well structured and appropriately scoped for a monorepo setup.
25-30: Verify Husky hook migration and release scripts
You’ve consolidatedprepare,new-snapshot,new-version, andrelease*under the root. Please confirm that:
- The
.husky/directory is present at the repo root and Husky hooks are installed correctly.- Running
pnpm run new-snapshot/new-version/releasebehaves as expected across all workspaces.
51-54: Confirm Node.js engine requirement
You’ve set"node": ">=16.14.0". Next.js 15 and some modern packages often recommend Node.js 18+. Please verify that all workspace packages (e.g., Next.js components) function correctly on Node 16, or consider bumping the engine to match upstream requirements.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/create-llama/e2e/utils.ts (1)
70-79: Logic improvement and variable declaration enhancement.The changes here properly fix the conditional logic for handling different data source types. By using separate conditions for
--web-sourceand--db-source, the code now correctly processes each data source type independently.While changing from
lettoconstis generally good practice, note that the array is still being mutated throughpush()operations. Consider using immutable array operations if you want to fully embrace functional programming principles.- const dataSourceArgs = []; - if (dataSource.includes("--web-source")) { + const dataSourceArgs = dataSource.includes("--web-source") + ? ["--web-source", dataSource.split(" ")[1]] + : dataSource.includes("--db-source") + ? ["--db-source", dataSource.split(" ")[1]] + : [dataSource];This alternative approach would remove the need for mutation, but the current implementation is also valid and clear.
packages/create-llama/templates/types/streaming/nextjs/app/components/ui/chat/tools/artifact.tsx (1)
288-288: Consider using Next.js Image component.While the ESLint directive disabling the
@next/next/no-img-elementrule has been removed, the code still uses a standard HTMLimgelement instead of Next.js's optimizedImagecomponent.Consider replacing the HTML img element with Next.js's Image component for better performance and optimization:
- <img src={url.url} alt={url.filename} className="my-4 w-1/2" /> + <Image src={url.url} alt={url.filename} width={500} height={300} className="my-4 w-1/2" />Don't forget to import the Image component:
+ import Image from "next/image";packages/create-llama/helpers/python.ts (1)
27-35: Consider refactoring the function to use an options objectThe ESLint directive was added to bypass the
max-paramsrule, which is now being enforced by the consolidated linting configuration. While this approach works, having 6 parameters makes the function harder to maintain and use correctly.Consider refactoring the function to use a configuration object pattern:
-const getAdditionalDependencies = ( - modelConfig: ModelConfig, - vectorDb?: TemplateVectorDB, - dataSources?: TemplateDataSource[], - tools?: Tool[], - templateType?: TemplateType, - observability?: TemplateObservability, - // eslint-disable-next-line max-params -) => { +interface DependencyOptions { + modelConfig: ModelConfig; + vectorDb?: TemplateVectorDB; + dataSources?: TemplateDataSource[]; + tools?: Tool[]; + templateType?: TemplateType; + observability?: TemplateObservability; +} + +const getAdditionalDependencies = ({ + modelConfig, + vectorDb, + dataSources, + tools, + templateType, + observability, +}: DependencyOptions) => {You would also need to update the function call at line 670:
- const addOnDependencies = getAdditionalDependencies( - modelConfig, - vectorDb, - dataSources, - tools, - template, - ); + const addOnDependencies = getAdditionalDependencies({ + modelConfig, + vectorDb, + dataSources, + tools, + templateType: template, + observability, + });This approach improves readability, maintainability, and avoids the need for ESLint exceptions.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (31)
eslint.config.mjs(2 hunks)packages/create-llama/create-app.ts(0 hunks)packages/create-llama/e2e/shared/llamaindexserver_template.spec.ts(0 hunks)packages/create-llama/e2e/shared/reflex_template.spec.ts(0 hunks)packages/create-llama/e2e/shared/streaming_template.spec.ts(0 hunks)packages/create-llama/e2e/utils.ts(1 hunks)packages/create-llama/helpers/copy.ts(0 hunks)packages/create-llama/helpers/env-variables.ts(2 hunks)packages/create-llama/helpers/git.ts(0 hunks)packages/create-llama/helpers/install.ts(0 hunks)packages/create-llama/helpers/is-folder-empty.ts(0 hunks)packages/create-llama/helpers/poetry.ts(0 hunks)packages/create-llama/helpers/providers/index.ts(1 hunks)packages/create-llama/helpers/python.ts(1 hunks)packages/create-llama/helpers/validate-pkg.ts(0 hunks)packages/create-llama/index.ts(0 hunks)packages/create-llama/playwright.config.ts(0 hunks)packages/create-llama/templates/components/engines/typescript/agent/tools/interpreter.ts(1 hunks)packages/create-llama/templates/components/vectordbs/typescript/astra/generate.ts(0 hunks)packages/create-llama/templates/components/vectordbs/typescript/astra/index.ts(0 hunks)packages/create-llama/templates/components/vectordbs/typescript/chroma/generate.ts(0 hunks)packages/create-llama/templates/components/vectordbs/typescript/chroma/index.ts(0 hunks)packages/create-llama/templates/components/vectordbs/typescript/milvus/generate.ts(0 hunks)packages/create-llama/templates/components/vectordbs/typescript/mongo/generate.ts(0 hunks)packages/create-llama/templates/components/vectordbs/typescript/mongo/index.ts(0 hunks)packages/create-llama/templates/components/vectordbs/typescript/pinecone/generate.ts(0 hunks)packages/create-llama/templates/components/vectordbs/typescript/pinecone/index.ts(0 hunks)packages/create-llama/templates/components/vectordbs/typescript/qdrant/generate.ts(0 hunks)packages/create-llama/templates/components/vectordbs/typescript/weaviate/generate.ts(0 hunks)packages/create-llama/templates/types/streaming/nextjs/app/components/ui/chat/tools/artifact.tsx(1 hunks)packages/create-llama/templates/types/streaming/nextjs/app/components/ui/chat/tools/chat-tools.tsx(1 hunks)
💤 Files with no reviewable changes (23)
- packages/create-llama/templates/components/vectordbs/typescript/pinecone/index.ts
- packages/create-llama/templates/components/vectordbs/typescript/mongo/index.ts
- packages/create-llama/templates/components/vectordbs/typescript/milvus/generate.ts
- packages/create-llama/templates/components/vectordbs/typescript/pinecone/generate.ts
- packages/create-llama/templates/components/vectordbs/typescript/mongo/generate.ts
- packages/create-llama/templates/components/vectordbs/typescript/astra/generate.ts
- packages/create-llama/templates/components/vectordbs/typescript/weaviate/generate.ts
- packages/create-llama/templates/components/vectordbs/typescript/astra/index.ts
- packages/create-llama/helpers/poetry.ts
- packages/create-llama/playwright.config.ts
- packages/create-llama/index.ts
- packages/create-llama/helpers/install.ts
- packages/create-llama/templates/components/vectordbs/typescript/qdrant/generate.ts
- packages/create-llama/templates/components/vectordbs/typescript/chroma/generate.ts
- packages/create-llama/e2e/shared/streaming_template.spec.ts
- packages/create-llama/e2e/shared/llamaindexserver_template.spec.ts
- packages/create-llama/helpers/is-folder-empty.ts
- packages/create-llama/helpers/copy.ts
- packages/create-llama/e2e/shared/reflex_template.spec.ts
- packages/create-llama/helpers/validate-pkg.ts
- packages/create-llama/create-app.ts
- packages/create-llama/helpers/git.ts
- packages/create-llama/templates/components/vectordbs/typescript/chroma/index.ts
✅ Files skipped from review due to trivial changes (3)
- packages/create-llama/templates/components/engines/typescript/agent/tools/interpreter.ts
- packages/create-llama/templates/types/streaming/nextjs/app/components/ui/chat/tools/chat-tools.tsx
- packages/create-llama/helpers/providers/index.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- eslint.config.mjs
⏰ Context from checks skipped due to timeout of 90000ms (27)
- GitHub Check: typescript (22, 3.11, ubuntu-22.04, nextjs, --llamacloud)
- GitHub Check: typescript (22, 3.11, ubuntu-22.04, nextjs, --example-file)
- GitHub Check: typescript (22, 3.11, ubuntu-22.04, nextjs, --no-files)
- GitHub Check: typescript (22, 3.11, windows-latest, nextjs, --llamacloud)
- GitHub Check: typescript (22, 3.11, windows-latest, nextjs, --example-file)
- GitHub Check: typescript (22, 3.11, windows-latest, nextjs, --no-files)
- GitHub Check: typescript (22, 3.11, macos-latest, nextjs, --llamacloud)
- GitHub Check: typescript (22, 3.11, macos-latest, nextjs, --example-file)
- GitHub Check: typescript (22, 3.11, macos-latest, nextjs, --no-files)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --llamacloud)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --example-file)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --llamacloud)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --no-files)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --example-file)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --llamacloud)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --no-files)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --example-file)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --llamacloud)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --no-files)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --example-file)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --llamacloud)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --no-files)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --example-file)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --llamacloud)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --no-files)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --example-file)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --no-files)
🔇 Additional comments (1)
packages/create-llama/helpers/env-variables.ts (1)
184-209: Improve code structure with explicit block scopingThe addition of curly braces around the "chroma" case in the switch statement improves readability and prevents variable leakage between cases. This is a good practice that helps clearly delineate the scope of the case block.
...ages/create-llama/templates/types/streaming/nextjs/app/components/ui/chat/tools/artifact.tsx
Show resolved
Hide resolved
Updated configs:
Key changes:
typescript recommendrules and prettier-plugin-tailwindcss before. So that, this PR will apply these configs, also fix format and eslint errors in all files.Summary by CodeRabbit