-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Early access generate #699
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
Changes from all commits
cb1b219
643c61d
9061fad
5ffabfd
73d22bd
588ab02
1347a8e
373423b
cd8833b
49acc64
8dba8a9
7dd0fcd
0d4be12
e35bd3f
d1bcb68
f9f16db
e730552
85e8fbf
dd9f236
2398a61
93eb1d2
814fc48
749cabc
4d51e20
d3c72b5
972ddd0
4ee76c3
38d85e4
8f687d7
458b065
640efee
8a0a833
ca1bc22
a864044
03106ce
b73d3be
02c0c99
a71f0c4
9e22918
db58b12
9313985
7725a50
fd056ff
14dfeed
2bdcd8d
cffc4f6
870dca9
32a698d
576cc73
c82605a
f3f04fc
45dca02
a0ef4d8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,34 @@ | ||||||||||||||||||||||||||||||||||||
| FROM oven/bun:canary | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| WORKDIR /app | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| # Install turbo globally | ||||||||||||||||||||||||||||||||||||
| RUN bun install -g next turbo | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| COPY package.json bun.lock turbo.json ./ | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| RUN mkdir -p apps packages | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| COPY apps/*/package.json ./apps/ | ||||||||||||||||||||||||||||||||||||
| COPY packages/*/package.json ./packages/ | ||||||||||||||||||||||||||||||||||||
| COPY packages/tsconfig/ ./packages/tsconfig/ | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
+13
to
+15
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Directory structure may not be preserved correctly The current approach for copying package.json files doesn't preserve the directory structure, which could cause build issues. -COPY apps/*/package.json ./apps/
-COPY packages/*/package.json ./packages/
-COPY packages/tsconfig/ ./packages/tsconfig/
+COPY apps/mail/package.json ./apps/mail/
+COPY packages/*/package.json ./packages/
+COPY packages/tsconfig/ ./packages/tsconfig/You should include copies for each specific subdirectory under apps/ to ensure the directory structure is maintained correctly.
|
||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| RUN bun install | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| COPY . . | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| # Installing with full context. Prevent missing dependencies error. | ||||||||||||||||||||||||||||||||||||
| RUN bun install | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
+17
to
+22
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Redundant dependency installation Installing dependencies twice is inefficient and increases build time and image size unnecessarily. -RUN bun install
-
-COPY . .
-
-# Installing with full context. Prevent missing dependencies error.
-RUN bun install
+# Copy necessary files for dependency installation
+COPY apps/mail/package.json ./apps/mail/
+COPY packages/*/package.json ./packages/
+COPY packages/tsconfig/ ./packages/tsconfig/
+COPY turbo.json ./
+
+# Install dependencies
+RUN bun install --frozen-lockfile
+
+# Copy the rest of the application
+COPY . .📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| RUN bun run build | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| ENV NODE_ENV=production | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| # Resolve Nextjs TextEncoder error. | ||||||||||||||||||||||||||||||||||||
| ENV NODE_OPTIONS=--no-experimental-fetch | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
+29
to
+30
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainVerify if NODE_OPTIONS override is still necessary The comment indicates this is addressing a TextEncoder error, but it's worth verifying if this is still required with the current versions of Next.js and Bun. 🌐 Web query: 💡 Result: As of 2024, the --no-experimental-fetch flag is not required when running Next.js with Bun. Key points:
Summary Table
Conclusion: Citations:
🏁 Script executed: #!/bin/bash
# Display the first 50 lines of Dockerfile to inspect the base image and context
sed -n '1,50p' DockerfileLength of output: 607 Remove unneeded The • File: - # Resolve Nextjs TextEncoder error.
- ENV NODE_OPTIONS=--no-experimental-fetch📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| EXPOSE 3000 | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| CMD ["bun", "run", "start", "--host", "0.0.0.0"] | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
+32
to
+34
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Missing health check and user configuration The Dockerfile lacks a health check and runs as root by default, which is not recommended for production containers. EXPOSE 3000
+
+# Add a health check
+HEALTHCHECK --interval=30s --timeout=10s --start-period=5s --retries=3 \
+ CMD curl -f http://localhost:3000/api/health || exit 1
+
+# Create a non-root user
+RUN adduser --disabled-password --gecos "" appuser
+USER appuser
CMD ["bun", "run", "start", "--host", "0.0.0.0"]
|
||||||||||||||||||||||||||||||||||||
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.
🛠️ Refactor suggestion
Consider using a stable Bun version rather than canary
Using the
oven/bun:canarytag could introduce instability in production as it might include experimental features that change frequently.📝 Committable suggestion