[All-Chore] 도커 세팅 #427
[All-Chore] 도커 세팅 #427
Conversation
|
""" WalkthroughThis update revises frontend development and deployment workflows, including upgrading pnpm to version 10 in CI/CD pipelines, introducing a new multi-stage Dockerfile, adjusting npm scripts for client and server apps, updating workspace configuration, and raising the minimum required pnpm version in the project configuration. Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant GitHub Actions
participant Docker
participant pnpm
participant Server
Developer->>GitHub Actions: Push code / PR
GitHub Actions->>pnpm: Setup pnpm@10
GitHub Actions->>Docker: Build Docker image (multi-stage)
Docker->>pnpm: Install dependencies, build, deploy
Docker->>Server: Start server with built artifacts
Possibly related PRs
Suggested labels
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
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
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
frontend/Dockerfile (3)
1-3: Pinning pnpm for reproducible builds.Using
pnpm@latest-10may pull in different patch versions over time. Consider locking to a specific version (e.g.,[email protected]) to ensure deterministic Docker layers.
6-9: Optimize cache: includepackage.jsonbeforepnpm fetch.For better layer caching, copy
package.jsonwith your lockfiles:COPY package.json pnpm-lock.yaml pnpm-workspace.yaml ./This ensures
pnpm fetch -rinvalidates only when dependencies change.
11-14: Leverage offline install after fetching modules.Since you prefetch with
pnpm fetch -r, run the install in offline mode:RUN pnpm install -r --offlineThis will use the prefetched cache and speed up CI builds.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
.github/workflows/fe-cd.yml(1 hunks).github/workflows/fe-ci.yml(1 hunks)frontend/Dockerfile(1 hunks)frontend/apps/client/package.json(1 hunks)frontend/apps/server/package.json(1 hunks)frontend/package.json(1 hunks)frontend/pnpm-workspace.yaml(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
frontend/apps/client/package.json (1)
Learnt from: hamo-o
PR: softeer5th/Team4-enDolphin#411
File: frontend/apps/client/package.json:7-7
Timestamp: 2025-05-27T17:33:56.448Z
Learning: In the Team4-enDolphin project, the server uses Vite as middleware in development mode, so the `dev` script in frontend/apps/client/package.json only needs to start the server (`pnpm --filter server start`) rather than running separate Vite dev server and server processes.
🔇 Additional comments (8)
frontend/pnpm-workspace.yaml (3)
4-4: Add config directory to workspace packages.
Includingconfigs/*ensures shared configurations are included in the workspace.
5-5: Enable workspace package injection.
SettinginjectWorkspacePackages: trueallows local packages to be symlinked automatically.
6-7: Synchronize injected deps after the build.
ThesyncInjectedDepsAfterScriptskey will re-link dependencies post-build, matching PNPM workspace enhancements..github/workflows/fe-ci.yml (1)
35-38: Upgrade pnpm setup action to v10.
This aligns with the raised PNPM engine requirement infrontend/package.json..github/workflows/fe-cd.yml (1)
27-30: Upgrade pnpm setup action to v10.
Consistent with CI and workspace engine bump for PNPM.frontend/apps/client/package.json (1)
7-7: Verifydevscript in server package.
Ensurefrontend/apps/server/package.jsondefines adevscript to match this filter invocation.frontend/package.json (2)
7-9: Validate recursive start script.
Confirm that all workspace packages expose astartscript sopnpm -r startboots correctly.
15-16: Bump PNPM engine requirement to >=10.0.0.
Matches CI/CD workflows and aligns local development with action versions.
| "dev": "cross-env NODE_ENV=start tsup --watch", | ||
| "start": "node index.js" |
There was a problem hiding this comment.
Missing cross-env dependency and non-standard NODE_ENV values.
You reference cross-env in both scripts but haven’t installed it—CI/build will break. Also, setting NODE_ENV=start is unconventional; use development for dev and production for start.
Apply this diff:
"scripts": {
- "dev": "cross-env NODE_ENV=start tsup --watch",
+ "dev": "cross-env NODE_ENV=development tsup --watch",
- "start": "node index.js"
+ "start": "cross-env NODE_ENV=production node index.js"
},
"devDependencies": {
+ "cross-env": "^7.0.3",
"@types/express": "^5.0.2",
"typescript": "^5.6.2",
"@endolphin/tsup-config": "workspace:^",
"@endolphin/vitest-config": "workspace:^"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "dev": "cross-env NODE_ENV=start tsup --watch", | |
| "start": "node index.js" | |
| "scripts": { | |
| - "dev": "cross-env NODE_ENV=start tsup --watch", | |
| + "dev": "cross-env NODE_ENV=development tsup --watch", | |
| - "start": "node index.js" | |
| + "start": "cross-env NODE_ENV=production node index.js" | |
| }, | |
| "devDependencies": { | |
| + "cross-env": "^7.0.3", | |
| "@types/express": "^5.0.2", | |
| "typescript": "^5.6.2", | |
| "@endolphin/tsup-config": "workspace:^", | |
| "@endolphin/vitest-config": "workspace:^" | |
| } |
🤖 Prompt for AI Agents
In frontend/apps/server/package.json around lines 9 to 10, the scripts use
cross-env without it being listed as a dependency, which will cause build
failures. Add cross-env to the devDependencies in package.json. Also, change the
NODE_ENV value in the "dev" script from "start" to "development" and in the
"start" script from the default to "production" to follow standard environment
naming conventions.
frontend/Dockerfile
Outdated
| RUN pnpm --filter @endolphin/server --prod deploy ./build/server | ||
| COPY ./apps/client/dist ./build/client |
There was a problem hiding this comment.
Invalid deploy target for @endolphin/server.
There’s no deploy script in frontend/apps/server/package.json, so this command will fail. Did you mean:
pnpm --filter @endolphin/server run buildor another custom script?
🤖 Prompt for AI Agents
In frontend/Dockerfile at lines 16-17, the RUN command uses an invalid `deploy`
script for @endolphin/server, which does not exist in its package.json. Replace
`pnpm --filter @endolphin/server --prod deploy ./build/server` with a valid
script such as `pnpm --filter @endolphin/server run build` or another existing
custom script defined in the package.json to ensure the command succeeds.
| COPY --from=builder /app/package.json ./package.json | ||
| COPY --from=builder /app/build/client ./client | ||
| COPY --from=builder /app/build/server/dist ./server | ||
| COPY --from=builder /app/build/server/package.json ./server/package.json | ||
| COPY --from=builder /app/build/server/node_modules ./server/node_modules | ||
|
|
||
| EXPOSE 5173 | ||
| ENV NODE_ENV=production | ||
| CMD ["pnpm", "start"] No newline at end of file |
There was a problem hiding this comment.
Final stage missing runtime dependencies and incorrect entrypoint.
You only copy server’s node_modules and the root package.json, but don’t install production dependencies for the root or client. Moreover, CMD ["pnpm","start"] relies on workspace scripts that aren’t available here. Either:
- Copy and install all production deps in this stage, or
- Change
CMDto directly run your server (e.g.,node server/index.js) and serve static assets.
Without this, the container will error on startup.
🤖 Prompt for AI Agents
In frontend/Dockerfile lines 20 to 28, the final stage only copies server
node_modules and package.json but does not install production dependencies for
the root or client, and the CMD uses "pnpm start" which depends on workspace
scripts not present in the container. To fix this, either copy and install all
necessary production dependencies in this stage or change the CMD to directly
run the server entry point (e.g., "node server/index.js") and serve static
assets, ensuring the container has all runtime dependencies and a valid startup
command.
kwon204
left a comment
kwon204
left a comment
There was a problem hiding this comment.
고생하셨습니다! 수정할 건 없어보이는데 cd 코드는 레지스트리 같은 게 정해지면 수정할 계획인가요??
#️⃣ 연관된 이슈>
📝 작업 내용> 이번 PR에서 작업한 내용을 간략히 설명해주세요(이미지 첨부 가능)
프론트엔드
client를 빌드해서server에서 실행하기 위한 이미지를 만드는 스크립트를 작성했습니다.server가 하나밖에 없지만, 확장성을 고려하는 게 좋을 것 같아package.json도 복사하고start명령어 추가해줬습니다.clientserverpackage.json요렇게 나오게 조정했습니다.🙏 여기는 꼭 봐주세요! > 리뷰어가 특별히 봐주었으면 하는 부분이 있다면 작성해주세요
Summary by CodeRabbit
Chores
New Features
Refactor