-
Notifications
You must be signed in to change notification settings - Fork 189
support eject to fully customize next folder #653
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
support eject to fully customize next folder #653
Conversation
🦋 Changeset detectedLatest commit: 763bc23 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
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 |
WalkthroughThis update introduces an "eject" feature for the server package, enabling users to export and fully customize the default Next.js project. The changes include new scripts, documentation, configuration files, and logic for copying and adapting the project structure. Documentation and templates are updated to describe and support the eject workflow. Additional improvements include fallback environment variable support for various configuration points and reorganized imports for utility modules. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI (llamaindex-server eject)
participant FileSystem
participant NextJS Template
User->>CLI (llamaindex-server eject): Run eject command (optionally with --output)
CLI->>FileSystem: Check for required directories/files
alt All required files exist
CLI->>FileSystem: Remove output directory if exists
CLI->>FileSystem: Copy NextJS template to output directory
CLI->>FileSystem: Copy src/app and generate.ts to output directory
CLI->>FileSystem: Copy components/data/output/storage if present
CLI->>FileSystem: Copy or create .env, append defaults
CLI->>FileSystem: Write config.js for frontend
CLI->>FileSystem: Rename gitignore, remove build config/layout as needed
CLI->>User: Log success message
else Missing files
CLI->>User: Log error and exit
end
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (57)
✨ 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:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
|
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.
Actionable comments posted: 4
♻️ Duplicate comments (1)
packages/server/next/app/api/chat/route.ts (1)
62-64
: Conditional execution properly implemented.The suggested questions are now correctly gated by the
suggestNextQuestions
parameter.
🧹 Nitpick comments (10)
.changeset/many-knives-warn.md (1)
6-6
: Minor grammatical improvement needed.Consider adding the article "the" before "next folder" for better readability.
Apply this diff to improve the grammar:
-support eject to fully customize next folder +support eject to fully customize the next folder🧰 Tools
🪛 LanguageTool
[uncategorized] ~6-~6: You might be missing the article “the” here.
Context: ...h --- support eject to fully customize next folder(AI_EN_LECTOR_MISSING_DETERMINER_THE)
packages/server/src/server.ts (1)
16-17
: Consider making EJECT_FOLDER configurable.The eject folder is hardcoded as "next". Consider making this configurable through an environment variable or option to provide more flexibility.
-const EJECT_FOLDER = "next"; +const EJECT_FOLDER = process.env.EJECT_FOLDER || "next";packages/create-llama/templates/components/use-cases/typescript/code_generator/README-template.md (1)
45-53
: Unify package manager commands for eject workflow
The text mentions usingpnpm eject
but the code block showsnpm run eject
. To avoid confusion, align on one package manager (e.g., pnpm) throughout.Example diff:
- ```bash - npm run eject - npm run dev:eject - ``` + ```bash + pnpm run eject + pnpm run dev:eject + ```packages/create-llama/templates/components/use-cases/typescript/deep_research/README-template.md (1)
56-63
: Consistent package manager usage in eject instructions
The section statespnpm eject
in the narrative but usesnpm run eject
in the snippet. For clarity, usepnpm run
commands consistently.Example diff:
- ```bash - npm run eject - npm run dev:eject - ``` + ```bash + pnpm run eject + pnpm run dev:eject + ```packages/create-llama/templates/components/use-cases/typescript/document_generator/README-template.md (1)
45-52
: Align eject commands with narrative
The narrative referencespnpm eject
, but the code block usesnpm run eject
. To maintain consistency, switch to pnpm commands in the snippet.Suggested diff:
- ```bash - npm run eject - npm run dev:eject - ``` + ```bash + pnpm run eject + pnpm run dev:eject + ```packages/create-llama/templates/components/use-cases/typescript/financial_report/README-template.md (1)
44-51
: Ensure consistent use of pnpm commands
The text instructspnpm eject
, yet the snippet showsnpm run eject
. For a cohesive developer experience, update the snippet to pnpm.Example diff:
- ```bash - npm run eject - npm run dev:eject - ``` + ```bash + pnpm run eject + pnpm run dev:eject + ```packages/create-llama/templates/components/use-cases/typescript/agentic_rag/README-template.md (1)
44-51
: Fix package manager inconsistency in eject documentation.The documentation mentions using
pnpm eject
in the description but showsnpm run
commands in the examples. This inconsistency could confuse users about which package manager to use.Consider updating for consistency:
-If you want to fully customize the server UI and routes, you can use `pnpm eject`. When using `pnpm eject`, the server will be rendered in the `next` folder. Frontend and routes will be hot-reloaded when changes are made. +If you want to fully customize the server UI and routes, you can use `npm run eject`. When using `npm run eject`, the server will be rendered in the `next` folder. Frontend and routes will be hot-reloaded when changes are made.Or alternatively, use
pnpm
consistently throughout:-```bash -npm run eject -npm run dev:eject -``` +```bash +pnpm eject +pnpm dev:eject +```packages/server/README.md (2)
311-316
: Specify language for fenced code block.The static analysis tool correctly identified that this fenced code block should have a language specified for proper syntax highlighting.
-``` -next/ -- this folder is generated by pnpm eject -src/ - index.ts - ... -``` +```bash +next/ -- this folder is generated by pnpm eject +src/ + index.ts + ... +```🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
311-311: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
305-323
: Fix package manager inconsistency and command syntax.There are two issues with the package manager usage:
- The description mentions
pnpm eject
but the example commands use inconsistent syntax- Line 321-322 shows incorrect command syntax with
export EJECT=true pnpm dev
on separate lines-If you want to fully customize the server UI and routes, you can use `pnpm eject`. When using `pnpm eject`, the server will be rendered in the `next` folder. Frontend and routes will be hot-reloaded when changes are made. +If you want to fully customize the server UI and routes, you can use `pnpm eject`. When ejecting, the server will be rendered in the `next` folder. Frontend and routes will be hot-reloaded when changes are made.-```bash -export EJECT=true pnpm dev -pnpm run dev -``` +```bash +EJECT=true pnpm run dev +```🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
311-311: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
packages/server/bin/eject.cjs (1)
40-44
: Consider implications of errorOnExist setting.Using
errorOnExist: true
withforce: false
means the operation will fail if any files exist in the destination. However, since you're removing the destination directory beforehand, this setting may be unnecessarily strict and could cause issues if there are race conditions or if the removal was incomplete.Consider using more permissive settings since you've already cleaned the destination:
- await fs.cp(sourceDir, destDir, { - recursive: true, - force: false, - errorOnExist: true, - }); + await fs.cp(sourceDir, destDir, { + recursive: true, + force: true, + });
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
.changeset/many-knives-warn.md
(1 hunks)packages/create-llama/templates/components/use-cases/typescript/agentic_rag/README-template.md
(1 hunks)packages/create-llama/templates/components/use-cases/typescript/code_generator/README-template.md
(1 hunks)packages/create-llama/templates/components/use-cases/typescript/deep_research/README-template.md
(1 hunks)packages/create-llama/templates/components/use-cases/typescript/document_generator/README-template.md
(1 hunks)packages/create-llama/templates/components/use-cases/typescript/financial_report/README-template.md
(1 hunks)packages/create-llama/templates/types/llamaindexserver/nextjs/package.json
(2 hunks)packages/server/README.md
(1 hunks)packages/server/bin/eject.cjs
(1 hunks)packages/server/next/app/api/chat/route.ts
(2 hunks)packages/server/package.json
(2 hunks)packages/server/src/index.ts
(1 hunks)packages/server/src/server.ts
(3 hunks)packages/server/src/utils/suggestion.ts
(1 hunks)packages/server/src/utils/workflow.ts
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
.changeset/many-knives-warn.md
[uncategorized] ~6-~6: You might be missing the article “the” here.
Context: ...h --- support eject to fully customize next folder
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
🪛 markdownlint-cli2 (0.17.2)
packages/server/README.md
311-311: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
🔇 Additional comments (14)
packages/server/src/utils/suggestion.ts (1)
4-4
:✅ Verification successful
Verify the prompts module location.
The import path change from
../prompts
to./prompts
suggests the prompts module has been moved to the same directory as this file. This aligns with the broader reorganization visible inindex.ts
where exports now referenceutils/prompts
.Please verify that the prompts module exists at the new location:
🏁 Script executed:
#!/bin/bash # Description: Verify the prompts module exists in the utils directory # Expected: Find prompts.ts or prompts/index.ts in the utils directory fd "prompts" packages/server/src/utils --type fLength of output: 84
Module location verified: prompts.ts exists
The import path change from
../prompts
to./prompts
is valid—packages/server/src/utils/prompts.ts
is present. No further action required.packages/server/src/index.ts (1)
3-3
:✅ Verification successful
Verify the module reorganization.
The export reorganization moves events and prompts modules into the utils subdirectory, which improves modularity. However, we should verify that the files exist at the new paths and that existing imports are updated consistently.
Please verify the module reorganization:
Also applies to: 5-5
🏁 Script executed:
#!/bin/bash # Description: Verify the utils modules exist and check for any broken imports # Expected: Find the modules in utils directory and no references to old paths echo "=== Checking if utils modules exist ===" fd "events" packages/server/src/utils --type f fd "prompts" packages/server/src/utils --type f echo "=== Checking for any remaining imports of old paths ===" rg "from ['\"]\.\/events['\"]" packages/server/src --type ts rg "from ['\"]\.\/prompts['\"]" packages/server/src --type tsLength of output: 715
Let’s re-run the broken‐import checks using fixed‐string searches:
🏁 Script executed:
#!/bin/bash # Verify utils modules exist echo "=== Checking utils modules ===" fd events packages/server/src/utils --type f fd prompts packages/server/src/utils --type f # Check for any stale imports of the old paths echo "=== Checking for stale imports of './events' ===" rg -F "from './events'" -t ts packages/server/src || true rg -F 'from "./events"' -t ts packages/server/src || true echo "=== Checking for stale imports of './prompts' ===" rg -F "from './prompts'" -t ts packages/server/src || true rg -F 'from \"./prompts\"' -t ts packages/server/src || trueLength of output: 768
Module reorganization verified – no broken imports
packages/server/src/utils/events.ts
andpackages/server/src/utils/prompts.ts
both exist.- No remaining top-level imports of
from './events'
orfrom './prompts'
outside of the utils directory.- The only
from "./events"
import inpackages/server/src/utils/workflow.ts
correctly refers toutils/events.ts
.All paths are updated consistently and there are no broken imports. Approving the reorganization.
packages/create-llama/templates/types/llamaindexserver/nextjs/package.json (2)
27-27
: Cross-env dependency addition is appropriate.Adding
cross-env
as a dev dependency is the correct approach for ensuring theEJECT=true
environment variable works across all platforms (Windows, macOS, Linux).
10-11
:✅ Verification successful
Eject scripts implementation looks good.
The new eject scripts are well-implemented:
eject
script calls thellamaindex-server
CLI tooldev:eject
usescross-env
for cross-platform environment variable supportPlease verify that the llamaindex-server binary supports the eject command:
🏁 Script executed:
#!/bin/bash # Description: Verify llamaindex-server CLI tool exists and supports eject # Expected: Find the binary configuration and eject command support echo "=== Checking if llamaindex-server has bin configuration ===" rg "bin.*eject" packages/server/package.json echo "=== Checking if eject script exists ===" fd "eject" packages/server/bin --type fLength of output: 377
Eject scripts implementation confirmed
- Verified
llamaindex-server
is configured in packages/server/package.json and points to./bin/eject.cjs
- Found the
eject.cjs
file in packages/server/bin, confirming the CLI supports theeject
commandcross-env
is correctly added for cross-platform support indev:eject
All checks out—approving these script additions.
packages/server/package.json (3)
20-27
: LGTM! Package configuration properly set up for eject functionality.The addition of "bin" and "server" to the files array ensures these directories are included in the published package, and the bin field correctly registers the
llamaindex-server
CLI command.
39-39
: Build script properly extended.The addition of
pnpm copy:routes
to the prepare:ts-server script ensures API routes are copied during the build process.
44-44
: Route copying script correctly implemented.The script properly copies API routes and utilities to the distribution directory, which is essential for the eject functionality.
packages/server/next/app/api/chat/route.ts (1)
20-22
: URL parameter handling looks good.The extraction and boolean conversion of the
suggestNextQuestions
parameter is implemented correctly.packages/server/src/server.ts (4)
30-36
: Eject mode initialization looks good.The implementation properly checks the environment variable and provides clear feedback to the user when eject mode is enabled.
37-41
: Directory configuration properly handled.The conditional directory setting correctly switches between the default server directory and the eject folder.
78-81
: Config file path correctly handled for both modes.The conditional path ensures the config.js file is written to the appropriate location based on the eject mode.
100-114
:❌ Incorrect review comment
Verify the intended behavior for eject mode routing.
The current implementation skips
handleChat
when in eject mode, which means the/api/chat
endpoint won't be handled by the custom handler. The comment mentions a Next.js limitation, but it's unclear if this is the intended behavior.Please confirm:
- Is the
/api/chat
endpoint expected to be handled by the ejected Next.js routes in eject mode?- Should there be a fallback or error handling when
handleChat
is skipped?
🏁 Script executed:
#!/bin/bash # Check if there's a corresponding route handler in the ejected Next.js structure fd -t f "route\.(ts|js)" --exec grep -l "POST.*chat" {} \; # Check for any documentation about eject mode behavior rg -A 5 -B 5 "eject.*mode.*chat|chat.*eject.*mode" --type mdLength of output: 126
Logic is correct: no changes needed for eject mode routing
The
handleChat
call is intentionally skipped whenthis.eject
is true, because in eject mode the user will have their own generated Next.js route (e.g. aroute.ts
underapp/api/chat
) that handles the/api/chat
endpoint. The fallback inserver.ts
only applies when not ejected.– No action required in packages/server/src/server.ts:100–114
– Ejected users get their own route handler, so skippinghandleChat
here is the intended behaviorLikely an incorrect or invalid review comment.
packages/server/src/utils/workflow.ts (1)
22-22
: Correct import path to events module
The import source has been updated from../events
to./events
to reflect the new file organization underutils/
. This aligns with the restructured utility modules and ensures imports resolve correctly.packages/server/bin/eject.cjs (1)
17-71
: Overall script structure looks good.The eject script follows good Node.js practices with proper async/await usage, path resolution, and exit code handling. The main functionality is implemented correctly for copying the server structure and API routes to enable full customization.
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.
Actionable comments posted: 1
🧹 Nitpick comments (4)
packages/server/next/README.md (4)
5-8
: Add directory navigation to Quick StartSince the ejected Next.js project resides in the
next
folder, include acd
step before installing dependencies to orient users correctly:- npm install - npm run dev + cd next + npm install + npm run dev
14-14
: Fix grammatical number agreementAdjust the phrasing to correctly reference both configuration files:
- customize the application via .env and public/config.js file. + customize the application via the `.env` and `public/config.js` files.
16-16
: Correct singular/plural in phrasingReplace “Here’s the examples” with grammatically accurate wording:
- Here’s the examples of how to migrate from LlamaIndexServer configs to Next.js project: + Here are the examples of how to migrate from LlamaIndexServer configs to a Next.js project:
50-50
: Use correct casing for Next.jsThe term “nextjs” should use its official casing:
- generated nextjs project + generated Next.js project🧰 Tools
🪛 LanguageTool
[uncategorized] ~50-~50: The official spelling of this programming framework is “Next.js”.
Context: ... edit the layout files in the generated nextjs project (app/components/ui/chat/layout)...(NODE_JS)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/server/bin/eject.cjs
(1 hunks)packages/server/next/README.md
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/server/bin/eject.cjs
🧰 Additional context used
🪛 LanguageTool
packages/server/next/README.md
[grammar] ~15-~15: Did you mean “Here are the examples”?
Context: ...on via .env and public/config.js file. Here's the examples of how to migrate from LlamaIndexServer...
(THERE_S_MANY)
[uncategorized] ~50-~50: The official spelling of this programming framework is “Next.js”.
Context: ... edit the layout files in the generated nextjs project (app/components/ui/chat/layout)...
(NODE_JS)
⏰ Context from checks skipped due to timeout of 90000ms (57)
- GitHub Check: typescript (22, 3.11, ubuntu-22.04, nextjs, --llamacloud, llamaindexserver)
- GitHub Check: typescript (22, 3.11, ubuntu-22.04, nextjs, --llamacloud, streaming)
- GitHub Check: typescript (22, 3.11, ubuntu-22.04, nextjs, --example-file, llamaindexserver)
- GitHub Check: typescript (22, 3.11, ubuntu-22.04, nextjs, --example-file, streaming)
- GitHub Check: typescript (22, 3.11, ubuntu-22.04, nextjs, --no-files, llamaindexserver)
- GitHub Check: typescript (22, 3.11, ubuntu-22.04, nextjs, --no-files, streaming)
- GitHub Check: typescript (22, 3.11, windows-latest, nextjs, --llamacloud, llamaindexserver)
- GitHub Check: typescript (22, 3.11, windows-latest, nextjs, --llamacloud, streaming)
- GitHub Check: typescript (22, 3.11, windows-latest, nextjs, --example-file, llamaindexserver)
- GitHub Check: typescript (22, 3.11, windows-latest, nextjs, --example-file, streaming)
- GitHub Check: typescript (22, 3.11, windows-latest, nextjs, --no-files, llamaindexserver)
- GitHub Check: typescript (22, 3.11, windows-latest, nextjs, --no-files, streaming)
- GitHub Check: typescript (22, 3.11, macos-latest, nextjs, --llamacloud, llamaindexserver)
- GitHub Check: typescript (22, 3.11, macos-latest, nextjs, --llamacloud, streaming)
- GitHub Check: typescript (22, 3.11, macos-latest, nextjs, --example-file, llamaindexserver)
- GitHub Check: typescript (22, 3.11, macos-latest, nextjs, --example-file, streaming)
- GitHub Check: typescript (22, 3.11, macos-latest, nextjs, --no-files, llamaindexserver)
- GitHub Check: typescript (22, 3.11, macos-latest, nextjs, --no-files, streaming)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --llamacloud, llamaindexserver)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --llamacloud, llamaindexserver)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --llamacloud, streaming)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --llamacloud, streaming)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --example-file, llamaindexserver)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --example-file, llamaindexserver)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --example-file, streaming)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --example-file, streaming)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --no-files, llamaindexserver)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --no-files, llamaindexserver)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --no-files, streaming)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --no-files, streaming)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --llamacloud, llamaindexserver)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --llamacloud, llamaindexserver)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --llamacloud, streaming)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --llamacloud, streaming)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --example-file, llamaindexserver)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --example-file, llamaindexserver)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --example-file, streaming)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --example-file, streaming)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --no-files, llamaindexserver)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --no-files, llamaindexserver)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --no-files, streaming)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --no-files, streaming)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --llamacloud, llamaindexserver)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --llamacloud, llamaindexserver)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --llamacloud, streaming)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --llamacloud, streaming)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --example-file, llamaindexserver)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --example-file, llamaindexserver)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --example-file, streaming)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --example-file, streaming)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --no-files, llamaindexserver)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --no-files, llamaindexserver)
- GitHub Check: Unit Tests (windows-latest, 3.9)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --no-files, streaming)
- GitHub Check: Unit Tests (ubuntu-latest, 3.9)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --no-files, streaming)
- GitHub Check: lint
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.
Actionable comments posted: 0
♻️ Duplicate comments (2)
packages/server/README.md (1)
309-309
: Clarify the eject command invocationThe command
npm eject
may be ambiguous. Please specify whether this should benpm run eject
(if it's a script in package.json) ornpx llamaindex-server eject
(if it's a CLI command).packages/server/next/README.md (1)
1-1
: Clarify the eject command invocation in documentationThe reference to
npm eject
is ambiguous and may not invoke the correct script. Please specify the exact command users should run.-This is a [LlamaIndex](https://www.llamaindex.ai/) project using [Next.js](https://nextjs.org/) that is ejected from [`llamaindex-server`](https://github.com/run-llama/create-llama/tree/main/packages/server) via `npm eject` command. +This is a [LlamaIndex](https://www.llamaindex.ai/) project using [Next.js](https://nextjs.org/) that is ejected from [`llamaindex-server`](https://github.com/run-llama/create-llama/tree/main/packages/server) via the `npx llamaindex-server eject` command.
🧹 Nitpick comments (4)
packages/server/next/app/components/ui/chat/chat-section.tsx (1)
20-20
: Consider using nullish coalescing for consistency.The fallback logic is correct and provides a good default. However, for consistency with other files in this PR (like
loader.ts
anddev-mode-panel.tsx
), consider using nullish coalescing (??
) instead of logical OR (||
):- api: getConfig("CHAT_API") || "/api/chat", + api: getConfig("CHAT_API") ?? "/api/chat",packages/server/README.md (1)
314-314
: Fix framework name capitalizationUse the official spelling "Next.js" instead of "nextjs" for consistency with the framework's branding.
-1. Init nextjs project with eslint, prettier, postcss, tailwindcss, shadcn components, etc. +1. Init Next.js project with eslint, prettier, postcss, tailwindcss, shadcn components, etc.🧰 Tools
🪛 LanguageTool
[uncategorized] ~314-~314: The official spelling of this programming framework is “Next.js”.
Context: ...rectory> ``` How eject works: 1. Init nextjs project with eslint, prettier, postcss,...(NODE_JS)
packages/server/next/README.md (2)
16-16
: Fix grammar in documentationCorrect the grammar for better readability.
-Here's the examples of how to migrate from LlamaIndexServer configs to Next.js project: +Here are the examples of how to migrate from LlamaIndexServer configs to Next.js project:
45-45
: Use official framework name capitalizationUse "Next.js" instead of "nextjs" for consistency with the official branding.
-For customizing layout, you can directly edit the layout files in the generated nextjs project (app/components/ui/chat/layout). +For customizing layout, you can directly edit the layout files in the generated Next.js project (app/components/ui/chat/layout).🧰 Tools
🪛 LanguageTool
[uncategorized] ~45-~45: The official spelling of this programming framework is “Next.js”.
Context: ... edit the layout files in the generated nextjs project (app/components/ui/chat/layout)...(NODE_JS)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
packages/server/README.md
(1 hunks)packages/server/bin/eject.cjs
(1 hunks)packages/server/next/README.md
(1 hunks)packages/server/next/app/components/ui/chat/chat-input.tsx
(1 hunks)packages/server/next/app/components/ui/chat/chat-section.tsx
(1 hunks)packages/server/next/app/components/ui/chat/chat-starter.tsx
(1 hunks)packages/server/next/app/components/ui/chat/custom/events/loader.ts
(1 hunks)packages/server/next/app/components/ui/chat/custom/llama-cloud-selector.tsx
(1 hunks)packages/server/next/app/components/ui/chat/dev-mode-panel.tsx
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/server/bin/eject.cjs
🧰 Additional context used
🧬 Code Graph Analysis (6)
packages/server/next/app/components/ui/chat/dev-mode-panel.tsx (1)
packages/server/next/app/components/ui/lib/utils.ts (1)
getConfig
(10-13)
packages/server/next/app/components/ui/chat/chat-section.tsx (1)
packages/server/next/app/components/ui/lib/utils.ts (1)
getConfig
(10-13)
packages/server/next/app/components/ui/chat/chat-starter.tsx (1)
packages/server/next/app/components/ui/lib/utils.ts (1)
getConfig
(10-13)
packages/server/next/app/components/ui/chat/chat-input.tsx (1)
packages/server/next/app/components/ui/lib/utils.ts (1)
getConfig
(10-13)
packages/server/next/app/components/ui/chat/custom/llama-cloud-selector.tsx (1)
packages/server/next/app/components/ui/lib/utils.ts (1)
getConfig
(10-13)
packages/server/next/app/components/ui/chat/custom/events/loader.ts (1)
packages/server/next/app/components/ui/lib/utils.ts (1)
getConfig
(10-13)
🪛 LanguageTool
packages/server/README.md
[uncategorized] ~314-~314: The official spelling of this programming framework is “Next.js”.
Context: ...rectory> ``` How eject works: 1. Init nextjs project with eslint, prettier, postcss,...
(NODE_JS)
packages/server/next/README.md
[grammar] ~15-~15: Did you mean “Here are the examples”?
Context: ...stomize the application via .env file. Here's the examples of how to migrate from LlamaIndexServer...
(THERE_S_MANY)
[uncategorized] ~45-~45: The official spelling of this programming framework is “Next.js”.
Context: ... edit the layout files in the generated nextjs project (app/components/ui/chat/layout)...
(NODE_JS)
⏰ Context from checks skipped due to timeout of 90000ms (57)
- GitHub Check: typescript (22, 3.11, ubuntu-22.04, nextjs, --llamacloud, llamaindexserver)
- GitHub Check: typescript (22, 3.11, ubuntu-22.04, nextjs, --llamacloud, streaming)
- GitHub Check: typescript (22, 3.11, ubuntu-22.04, nextjs, --example-file, llamaindexserver)
- GitHub Check: typescript (22, 3.11, ubuntu-22.04, nextjs, --example-file, streaming)
- GitHub Check: typescript (22, 3.11, ubuntu-22.04, nextjs, --no-files, llamaindexserver)
- GitHub Check: typescript (22, 3.11, ubuntu-22.04, nextjs, --no-files, streaming)
- GitHub Check: typescript (22, 3.11, windows-latest, nextjs, --llamacloud, llamaindexserver)
- GitHub Check: typescript (22, 3.11, windows-latest, nextjs, --llamacloud, streaming)
- GitHub Check: typescript (22, 3.11, windows-latest, nextjs, --example-file, llamaindexserver)
- GitHub Check: typescript (22, 3.11, windows-latest, nextjs, --example-file, streaming)
- GitHub Check: typescript (22, 3.11, windows-latest, nextjs, --no-files, llamaindexserver)
- GitHub Check: typescript (22, 3.11, windows-latest, nextjs, --no-files, streaming)
- GitHub Check: typescript (22, 3.11, macos-latest, nextjs, --llamacloud, llamaindexserver)
- GitHub Check: typescript (22, 3.11, macos-latest, nextjs, --llamacloud, streaming)
- GitHub Check: typescript (22, 3.11, macos-latest, nextjs, --example-file, llamaindexserver)
- GitHub Check: typescript (22, 3.11, macos-latest, nextjs, --example-file, streaming)
- GitHub Check: typescript (22, 3.11, macos-latest, nextjs, --no-files, llamaindexserver)
- GitHub Check: typescript (22, 3.11, macos-latest, nextjs, --no-files, streaming)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --llamacloud, llamaindexserver)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --llamacloud, streaming)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --example-file, llamaindexserver)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --example-file, streaming)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --llamacloud, llamaindexserver)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --no-files, llamaindexserver)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --llamacloud, streaming)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --no-files, streaming)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --example-file, llamaindexserver)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --llamacloud, llamaindexserver)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --example-file, streaming)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --llamacloud, streaming)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --no-files, llamaindexserver)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --example-file, llamaindexserver)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --no-files, streaming)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --example-file, streaming)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --llamacloud, llamaindexserver)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --no-files, llamaindexserver)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --llamacloud, streaming)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --no-files, streaming)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --example-file, llamaindexserver)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --llamacloud, llamaindexserver)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --example-file, streaming)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --no-files, llamaindexserver)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --llamacloud, streaming)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --no-files, streaming)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --example-file, llamaindexserver)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --llamacloud, llamaindexserver)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --llamacloud, streaming)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --example-file, streaming)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --no-files, llamaindexserver)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --example-file, llamaindexserver)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --example-file, streaming)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --no-files, streaming)
- GitHub Check: Unit Tests (windows-latest, 3.9)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --no-files, llamaindexserver)
- GitHub Check: Unit Tests (ubuntu-latest, 3.9)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --no-files, streaming)
- GitHub Check: lint
🔇 Additional comments (5)
packages/server/next/app/components/ui/chat/custom/events/loader.ts (1)
20-24
: LGTM! Good fallback mechanism for endpoint configuration.The fallback pattern using nullish coalescing (
??
) is well-implemented. This change enhances configurability by allowing environment variables to provide default endpoints when explicit configuration is unavailable, which aligns well with the eject feature.packages/server/next/app/components/ui/chat/dev-mode-panel.tsx (1)
22-23
: LGTM! Consistent fallback pattern for dev mode configuration.The environment variable fallback enhances configurability while maintaining the existing behavior when explicit configuration is provided.
packages/server/next/app/components/ui/chat/chat-input.tsx (1)
11-15
: LGTM! Well-structured fallback mechanism.The conditional fallback logic is clean and follows the established pattern. The empty string default when the environment variable is not set ensures the component behavior remains controlled.
packages/server/next/app/components/ui/chat/chat-starter.tsx (1)
9-11
: Excellent fallback pattern implementation!The addition of environment variable fallback for
starterQuestions
is well-implemented and aligns perfectly with the eject functionality. This allows configuration via.env
files in ejected projects while maintaining backward compatibility with the programmatic API.packages/server/next/app/components/ui/chat/custom/llama-cloud-selector.tsx (1)
68-74
: Consistent fallback pattern implementation!The environment variable fallback for the LlamaCloud API endpoint follows the same pattern as other components and properly supports the eject functionality. The conditional fetch logic prevents unnecessary API calls when no endpoint is configured.
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.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/server/README.md (1)
305-310
: Clarify eject CLI command usage
The snippet showsnpm eject <path-to-output-directory>
, which may not run the intended script. To avoid confusion, specify the CLI invocation, e.g.,# using npx npx llamaindex-server eject <path-to-output-directory> # or via npm script npm run eject -- <path-to-output-directory>This aligns with how the eject command is exposed in
package.json
.🧰 Tools
🪛 LanguageTool
[uncategorized] ~306-~306: You might be missing the article “a” here.
Context: ...hange the output directory by providing custom path aftereject
command: ```bash np...(AI_EN_LECTOR_MISSING_DETERMINER_A)
[uncategorized] ~306-~306: You might be missing the article “the” here.
Context: ...irectory by providing custom path aftereject
command: ```bash npm eject <path...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
🧹 Nitpick comments (2)
packages/server/README.md (2)
306-306
: Fix missing articles in description
Add the necessary articles for clarity and correctness.- You can change the output directory by providing custom path after `eject` command: + You can change the output directory by providing a custom path to the `eject` command:🧰 Tools
🪛 LanguageTool
[uncategorized] ~306-~306: You might be missing the article “a” here.
Context: ...hange the output directory by providing custom path aftereject
command: ```bash np...(AI_EN_LECTOR_MISSING_DETERMINER_A)
[uncategorized] ~306-~306: You might be missing the article “the” here.
Context: ...irectory by providing custom path aftereject
command: ```bash npm eject <path...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
314-314
: Use consistent casing and wording for tools and frameworks
Use the official names and expand abbreviations for clarity.- 1. Init nextjs project with eslint, prettier, postcss, tailwindcss, shadcn components, etc. + 1. Initialize a Next.js project with ESLint, Prettier, PostCSS, Tailwind CSS, Shadcn components, etc.🧰 Tools
🪛 LanguageTool
[uncategorized] ~314-~314: The official spelling of this programming framework is “Next.js”.
Context: ...rectory> ``` How eject works: 1. Init nextjs project with eslint, prettier, postcss,...(NODE_JS)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/server/README.md
(1 hunks)packages/server/bin/eject.cjs
(1 hunks)packages/server/next/app/api/dev/files/workflow/route.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/server/next/app/api/dev/files/workflow/route.ts
- packages/server/bin/eject.cjs
🧰 Additional context used
🪛 LanguageTool
packages/server/README.md
[uncategorized] ~306-~306: You might be missing the article “a” here.
Context: ...hange the output directory by providing custom path after eject
command: ```bash np...
(AI_EN_LECTOR_MISSING_DETERMINER_A)
[uncategorized] ~306-~306: You might be missing the article “the” here.
Context: ...irectory by providing custom path after eject
command: ```bash npm eject <path...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~314-~314: The official spelling of this programming framework is “Next.js”.
Context: ...rectory> ``` How eject works: 1. Init nextjs project with eslint, prettier, postcss,...
(NODE_JS)
⏰ Context from checks skipped due to timeout of 90000ms (57)
- GitHub Check: typescript (22, 3.11, ubuntu-22.04, nextjs, --llamacloud, llamaindexserver)
- GitHub Check: typescript (22, 3.11, ubuntu-22.04, nextjs, --llamacloud, streaming)
- GitHub Check: typescript (22, 3.11, ubuntu-22.04, nextjs, --example-file, llamaindexserver)
- GitHub Check: typescript (22, 3.11, ubuntu-22.04, nextjs, --example-file, streaming)
- GitHub Check: typescript (22, 3.11, ubuntu-22.04, nextjs, --no-files, llamaindexserver)
- GitHub Check: typescript (22, 3.11, ubuntu-22.04, nextjs, --no-files, streaming)
- GitHub Check: typescript (22, 3.11, windows-latest, nextjs, --llamacloud, llamaindexserver)
- GitHub Check: typescript (22, 3.11, windows-latest, nextjs, --llamacloud, streaming)
- GitHub Check: typescript (22, 3.11, windows-latest, nextjs, --example-file, llamaindexserver)
- GitHub Check: typescript (22, 3.11, windows-latest, nextjs, --example-file, streaming)
- GitHub Check: typescript (22, 3.11, windows-latest, nextjs, --no-files, llamaindexserver)
- GitHub Check: typescript (22, 3.11, windows-latest, nextjs, --no-files, streaming)
- GitHub Check: typescript (22, 3.11, macos-latest, nextjs, --llamacloud, llamaindexserver)
- GitHub Check: typescript (22, 3.11, macos-latest, nextjs, --llamacloud, streaming)
- GitHub Check: typescript (22, 3.11, macos-latest, nextjs, --example-file, llamaindexserver)
- GitHub Check: typescript (22, 3.11, macos-latest, nextjs, --example-file, streaming)
- GitHub Check: typescript (22, 3.11, macos-latest, nextjs, --no-files, llamaindexserver)
- GitHub Check: typescript (22, 3.11, macos-latest, nextjs, --no-files, streaming)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --llamacloud, llamaindexserver)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --llamacloud, streaming)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --example-file, llamaindexserver)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --llamacloud, llamaindexserver)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --example-file, streaming)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --llamacloud, streaming)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --no-files, llamaindexserver)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --example-file, llamaindexserver)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --no-files, streaming)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --example-file, streaming)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --llamacloud, llamaindexserver)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --no-files, llamaindexserver)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --llamacloud, streaming)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --no-files, streaming)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --example-file, llamaindexserver)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --llamacloud, llamaindexserver)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --example-file, streaming)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --llamacloud, streaming)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --no-files, llamaindexserver)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --example-file, llamaindexserver)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --no-files, streaming)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --example-file, streaming)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --llamacloud, llamaindexserver)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --no-files, llamaindexserver)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --llamacloud, streaming)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --no-files, streaming)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --example-file, llamaindexserver)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --llamacloud, llamaindexserver)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --example-file, streaming)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --llamacloud, streaming)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --no-files, llamaindexserver)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --example-file, llamaindexserver)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --no-files, streaming)
- GitHub Check: Unit Tests (windows-latest, 3.9)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --example-file, streaming)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --no-files, llamaindexserver)
- GitHub Check: Unit Tests (ubuntu-latest, 3.9)
- GitHub Check: lint
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --no-files, streaming)
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.
Actionable comments posted: 0
♻️ Duplicate comments (3)
packages/server/bin/eject.cjs (3)
93-93
:⚠️ Potential issueFix critical bug: incorrect destination path creates nested app directory.
This is the same critical issue identified in previous reviews. The destination path
path.join(chatRouteDir, "app")
creates an incorrect nested structuredestDir/app/api/chat/app
instead of the intendeddestDir/app/api/chat
.Apply this diff to fix the destination path:
- await fs.cp(srcAppDir, path.join(chatRouteDir, "app"), { recursive: true }); + await fs.cp(srcAppDir, chatRouteDir, { recursive: true });
135-138
: 🛠️ Refactor suggestionAdd error handling for gitignore rename operation.
The rename operation lacks error handling and could cause the script to crash if the source file doesn't exist, which is the same issue identified in previous reviews.
Apply this diff to add proper error handling:
- // rename gitignore -> .gitignore - await fs.rename( - path.join(destDir, "gitignore"), - path.join(destDir, ".gitignore"), - ); + // rename gitignore -> .gitignore if it exists + try { + await fs.rename( + path.join(destDir, "gitignore"), + path.join(destDir, ".gitignore"), + ); + } catch (error) { + if (error.code !== 'ENOENT') { + console.warn(`Warning: Could not rename gitignore file: ${error.message}`); + } + }
151-151
: 🛠️ Refactor suggestionAdd error handling for file removal operation.
The unlink operation could fail if the file doesn't exist, causing the script to terminate unexpectedly. This is the same issue identified in previous reviews.
Apply this diff to add proper error handling:
- await fs.unlink(path.join(destDir, "next-build.config.ts")); + // remove next-build.config.ts if it exists + try { + await fs.unlink(path.join(destDir, "next-build.config.ts")); + } catch (error) { + if (error.code !== 'ENOENT') { + console.warn(`Warning: Could not remove next-build.config.ts: ${error.message}`); + } + }
🧹 Nitpick comments (2)
packages/server/bin/eject.cjs (2)
77-80
: Improve command line argument parsing logic.The current logic for determining the custom path is fragile - it assumes the last argument is always the destination path, which could lead to unexpected behavior if other arguments are passed.
Consider using a more robust argument parsing approach:
- // Get destination directory from command line arguments (pnpm eject <path>) - const customPath = process.argv[process.argv.length - 1]; - const destDir = customPath - ? path.resolve(customPath) // Use provided path - : path.join(process.cwd(), "next"); // Default to "next" folder in the current working directory + // Get destination directory from command line arguments (pnpm eject <path>) + const args = process.argv.slice(2); + const customPath = args.length > 0 && !args[args.length - 1].startsWith('-') ? args[args.length - 1] : null; + const destDir = customPath + ? path.resolve(customPath) // Use provided path + : path.join(process.cwd(), "next"); // Default to "next" folder in the current working directory
83-83
: Add logging for directory cleanup operation.The script silently removes the destination directory, which could be confusing for users if they have existing content there.
Add logging to inform users about the cleanup:
+ console.log(`Cleaning up existing directory: ${destDir}`); await fs.rm(destDir, { recursive: true, force: true });
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/server/bin/eject.cjs
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (57)
- GitHub Check: typescript (22, 3.11, ubuntu-22.04, nextjs, --llamacloud, llamaindexserver)
- GitHub Check: typescript (22, 3.11, ubuntu-22.04, nextjs, --llamacloud, streaming)
- GitHub Check: typescript (22, 3.11, ubuntu-22.04, nextjs, --example-file, llamaindexserver)
- GitHub Check: typescript (22, 3.11, ubuntu-22.04, nextjs, --example-file, streaming)
- GitHub Check: typescript (22, 3.11, ubuntu-22.04, nextjs, --no-files, llamaindexserver)
- GitHub Check: typescript (22, 3.11, ubuntu-22.04, nextjs, --no-files, streaming)
- GitHub Check: typescript (22, 3.11, windows-latest, nextjs, --llamacloud, llamaindexserver)
- GitHub Check: typescript (22, 3.11, windows-latest, nextjs, --llamacloud, streaming)
- GitHub Check: typescript (22, 3.11, windows-latest, nextjs, --example-file, llamaindexserver)
- GitHub Check: typescript (22, 3.11, windows-latest, nextjs, --example-file, streaming)
- GitHub Check: typescript (22, 3.11, windows-latest, nextjs, --no-files, llamaindexserver)
- GitHub Check: typescript (22, 3.11, windows-latest, nextjs, --no-files, streaming)
- GitHub Check: typescript (22, 3.11, macos-latest, nextjs, --llamacloud, llamaindexserver)
- GitHub Check: typescript (22, 3.11, macos-latest, nextjs, --llamacloud, streaming)
- GitHub Check: typescript (22, 3.11, macos-latest, nextjs, --example-file, llamaindexserver)
- GitHub Check: typescript (22, 3.11, macos-latest, nextjs, --example-file, streaming)
- GitHub Check: typescript (22, 3.11, macos-latest, nextjs, --no-files, llamaindexserver)
- GitHub Check: typescript (22, 3.11, macos-latest, nextjs, --no-files, streaming)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --llamacloud, llamaindexserver)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --llamacloud, streaming)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --llamacloud, llamaindexserver)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --example-file, llamaindexserver)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --llamacloud, streaming)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --example-file, streaming)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --example-file, llamaindexserver)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --no-files, llamaindexserver)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --example-file, streaming)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --no-files, streaming)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --no-files, llamaindexserver)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --llamacloud, llamaindexserver)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --no-files, streaming)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --llamacloud, streaming)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --example-file, llamaindexserver)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --llamacloud, llamaindexserver)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --llamacloud, streaming)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --example-file, streaming)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --example-file, llamaindexserver)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --no-files, llamaindexserver)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --example-file, streaming)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --no-files, streaming)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --no-files, llamaindexserver)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --llamacloud, llamaindexserver)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --no-files, streaming)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --llamacloud, streaming)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --llamacloud, llamaindexserver)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --example-file, llamaindexserver)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --llamacloud, streaming)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --example-file, streaming)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --example-file, llamaindexserver)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --no-files, llamaindexserver)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --example-file, streaming)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --no-files, streaming)
- GitHub Check: Unit Tests (windows-latest, 3.9)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --no-files, llamaindexserver)
- GitHub Check: Unit Tests (ubuntu-latest, 3.9)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --no-files, streaming)
- GitHub Check: lint
🔇 Additional comments (2)
packages/server/bin/eject.cjs (2)
160-170
: The copy helper function is well-implemented.This helper function properly handles optional file copying with existence checks and provides a clean return value indicating whether the operation was successful.
16-59
: Configuration constants are well-structured.The SERVER_CONFIG_VARS array provides comprehensive environment variable definitions with descriptions and sensible defaults. This makes the configuration clear and maintainable.
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.
just needs e2e test and updated doc
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.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/server/next/README.md (1)
1-1
: Clarify the eject command invocation.The docs currently instruct users to run
npm eject
, which may not invoke the custom CLI provided by this package. Please update to the exact command, for example:- via `npm eject` command. + via the `npx llamaindex-server eject` command.
🧹 Nitpick comments (3)
packages/server/next/README.md (3)
16-16
: Fix typo and improve readability in command description."Generate Datasource" → "Generate Data Source" for consistency, and adjust phrasing:
- - Generate Datasource (in case you're having a `./data` folder): `npm run generate` + - Generate Data Source (if you have a `./data` folder): `npm run generate`🧰 Tools
🪛 LanguageTool
[uncategorized] ~16-~16: This verb may not be in the correct tense. Consider changing the tense to fit the context better.
Context: ...ands - Generate Datasource (in case you're having a./data
folder):npm run generate
...(AI_EN_LECTOR_REPLACEMENT_VERB_TENSE)
24-24
: Add a comma for clarity.Insert a comma after "Next.js" to improve readability:
- The project can be deployed to any platform that supports Next.js like Vercel. + The project can be deployed to any platform that supports Next.js, like Vercel.🧰 Tools
🪛 LanguageTool
[uncategorized] ~24-~24: Possible missing comma found.
Context: ...oyed to any platform that supports Next.js like Vercel. ## Configuration Your or...(AI_HYDRA_LEO_MISSING_COMMA)
32-35
: Specify language for the fenced code block.Add a language identifier (
bash
) to the code fence for proper syntax highlighting:- ``` + ```bash🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
32-32: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/server/bin/eject.cjs
(1 hunks)packages/server/next/README.md
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/server/bin/eject.cjs
🧰 Additional context used
🪛 LanguageTool
packages/server/next/README.md
[uncategorized] ~16-~16: This verb may not be in the correct tense. Consider changing the tense to fit the context better.
Context: ...ands - Generate Datasource (in case you're having a ./data
folder): npm run generate
...
(AI_EN_LECTOR_REPLACEMENT_VERB_TENSE)
[uncategorized] ~24-~24: Possible missing comma found.
Context: ...oyed to any platform that supports Next.js like Vercel. ## Configuration Your or...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~28-~28: A punctuation mark might be missing here.
Context: ... configurations have been migrated to a .env
file. Changing the .env
...
(AI_EN_LECTOR_MISSING_PUNCTUATION)
🪛 markdownlint-cli2 (0.17.2)
packages/server/next/README.md
32-32: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms (56)
- GitHub Check: typescript (22, 3.11, ubuntu-22.04, nextjs, --llamacloud, llamaindexserver)
- GitHub Check: typescript (22, 3.11, ubuntu-22.04, nextjs, --llamacloud, streaming)
- GitHub Check: typescript (22, 3.11, ubuntu-22.04, nextjs, --example-file, llamaindexserver)
- GitHub Check: typescript (22, 3.11, ubuntu-22.04, nextjs, --example-file, streaming)
- GitHub Check: typescript (22, 3.11, ubuntu-22.04, nextjs, --no-files, llamaindexserver)
- GitHub Check: typescript (22, 3.11, ubuntu-22.04, nextjs, --no-files, streaming)
- GitHub Check: typescript (22, 3.11, windows-latest, nextjs, --llamacloud, llamaindexserver)
- GitHub Check: typescript (22, 3.11, windows-latest, nextjs, --llamacloud, streaming)
- GitHub Check: typescript (22, 3.11, windows-latest, nextjs, --example-file, llamaindexserver)
- GitHub Check: typescript (22, 3.11, windows-latest, nextjs, --example-file, streaming)
- GitHub Check: typescript (22, 3.11, windows-latest, nextjs, --no-files, llamaindexserver)
- GitHub Check: typescript (22, 3.11, windows-latest, nextjs, --no-files, streaming)
- GitHub Check: typescript (22, 3.11, macos-latest, nextjs, --llamacloud, llamaindexserver)
- GitHub Check: typescript (22, 3.11, macos-latest, nextjs, --llamacloud, streaming)
- GitHub Check: typescript (22, 3.11, macos-latest, nextjs, --example-file, llamaindexserver)
- GitHub Check: typescript (22, 3.11, macos-latest, nextjs, --example-file, streaming)
- GitHub Check: typescript (22, 3.11, macos-latest, nextjs, --no-files, llamaindexserver)
- GitHub Check: typescript (22, 3.11, macos-latest, nextjs, --no-files, streaming)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --llamacloud, llamaindexserver)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --llamacloud, llamaindexserver)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --llamacloud, streaming)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --llamacloud, streaming)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --example-file, llamaindexserver)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --example-file, llamaindexserver)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --example-file, streaming)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --example-file, streaming)
- GitHub Check: typescript (20, 3.11, ubuntu-22.04, nextjs, --no-files, llamaindexserver)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --no-files, llamaindexserver)
- GitHub Check: python (20, 3.11, ubuntu-22.04, fastapi, --no-files, streaming)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --llamacloud, llamaindexserver)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --llamacloud, llamaindexserver)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --llamacloud, streaming)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --llamacloud, streaming)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --example-file, llamaindexserver)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --example-file, llamaindexserver)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --example-file, streaming)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --example-file, streaming)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --no-files, llamaindexserver)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --no-files, llamaindexserver)
- GitHub Check: python (20, 3.11, windows-latest, fastapi, --no-files, streaming)
- GitHub Check: typescript (20, 3.11, windows-latest, nextjs, --no-files, streaming)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --llamacloud, llamaindexserver)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --llamacloud, llamaindexserver)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --llamacloud, streaming)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --llamacloud, streaming)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --example-file, llamaindexserver)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --example-file, llamaindexserver)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --example-file, streaming)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --example-file, streaming)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --no-files, llamaindexserver)
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --no-files, llamaindexserver)
- GitHub Check: Unit Tests (windows-latest, 3.9)
- GitHub Check: Unit Tests (ubuntu-latest, 3.9)
- GitHub Check: typescript (20, 3.11, macos-latest, nextjs, --no-files, streaming)
- GitHub Check: lint
- GitHub Check: python (20, 3.11, macos-latest, fastapi, --no-files, streaming)
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.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/create-llama/e2e/shared/llamaindexserver_template.spec.ts
(3 hunks)
🔇 Additional comments (2)
packages/create-llama/e2e/shared/llamaindexserver_template.spec.ts (2)
2-2
: LGTM! Import addition is appropriate.The addition of
execSync
to the existing import is necessary for the new eject test functionality.
31-31
: LGTM! Constant definition is clear and appropriate.The
ejectDir
constant is well-named and aligns with the eject functionality that creates anext
subdirectory.
Summary by CodeRabbit
New Features
npm run eject
.Documentation
Chores
Bug Fixes