feat: Migrating to bun#52
Conversation
| "scripts": { | ||
| "test": "npm run build:ts && tsc -p test/tsconfig.json && tap --ts \"test/**/*.test.ts\"", | ||
| "start": "npm run build:ts && fastify start -l info dist/app.js", | ||
| "test": "bun install && tsc -p test/tsconfig.json && tap --ts \"test/**/*.test.ts\"", |
There was a problem hiding this comment.
We don't need these redundant bun install directives with every command.
There was a problem hiding this comment.
By directives you mean the command itself, or the commands that are following the bun install command?
There was a problem hiding this comment.
The bun install commands preceding the actual command
|
|
||
| CMD ["yarn", "fastify", "start", "-a", "::", "-l", "info", "dist/app.js"] | ||
|
|
||
| CMD ["bun", "fastify", "start", "-a", "::", "-l", "info", "dist/app.js"] |
There was a problem hiding this comment.
If you try running this command (without Docker), you're likely to find this won't work.
- We're no longer transpiling ts -> js
- Once you replace this app.js with app.ts, I expect you'll find that the
fastifyCLI does not support TypeScript files.
Possible solution: remove dependence on Fastify CLI by using fastify eject, then try and see if it works out of the box, if not we might need some more adjustments.
There was a problem hiding this comment.
Can we use bun build:ts first then let the command work normally, cause then it will be able to find dist/app.js?
Yes the fastify CLi does not support typescript files, by using fastify eject-ts there is a new server.ts file generated to let it function as a standalone app, Im figuring it out still.
Will update soon
|
Okay, will look into this and get back |
|
@ditsuke This was the way console looked with fastify CLI Now it looks like this: |
ditsuke
left a comment
ditsuke
left a comment
There was a problem hiding this comment.
Hi @AnshGupta01, thanks for your patience with the review. Overall the PR is really solid, only a few low hanging changes and it'll be good to merge :)
| "scripts": { | ||
| "test": "npm run build:ts && tsc -p test/tsconfig.json && tap --ts \"test/**/*.test.ts\"", | ||
| "start": "npm run build:ts && fastify start -l info dist/app.js", | ||
| "test": "bun server.ts && tsc -p test/tsconfig.json && tap --ts \"test/**/*.test.ts\"", |
There was a problem hiding this comment.
| "test": "bun server.ts && tsc -p test/tsconfig.json && tap --ts \"test/**/*.test.ts\"", | |
| "test": "tsc -p test/tsconfig.json && tap --ts \"test/**/*.test.ts\"", |
| @@ -1,21 +1,19 @@ | |||
| FROM node:18-alpine as builder | |||
There was a problem hiding this comment.
Remove builder build stage, instead:
- Start from oven/bun:alpine as before
- Copy first the manifest and lockfile
- Bun install like below
- Copy all assets
- ✔️
| RUN yarn install --frozen-lockfile --production | ||
| COPY --from=builder /app/package.json /app/bun.lockb ./ | ||
| RUN bun install --frozen-lockfile --production | ||
| COPY --from=builder /app/dist ./dist |
There was a problem hiding this comment.
Copy all files (COPY . .) since we're not building JS assets anymore
| // Register your application as a normal plugin. | ||
| app.register(import("./src/app")); | ||
|
|
||
| // delay is the number of milliseconds for the graceful close to finish |
|
Hi, I was on a break since November, I will get back to you soon with the requested changes! |
2 participants
Hi there, there was some git issue with the previous PR, hence raising this new PR.
I've incorporated the requested changes, but im stuck with the dockerFile updates (Please help me out)