feat: add Bun deployment adapter#196
Open
rturnq wants to merge 5 commits into
Open
Conversation
Add @marko/run-adapter-bun for deploying Marko Run apps on Bun. - Bundles a self-contained server entry (dist/index.mjs) that uses Bun.serve, targeting Node with the bun export condition - Serves the app and falls back to serving static assets from dist/public via Bun.file (immutable caching for /assets, path traversal guard), listening on the PORT env var - Exposes the Bun.serve server (requestIP, etc.) to handlers via BunPlatformInfo - Adds a run-package test fixture (preview skipped; it needs the Bun CLI)
🦋 Changeset detectedLatest commit: b75990a 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 |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
WalkthroughThis PR adds a new Related Issues: None found 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/adapters/bun/src/index.ts (1)
46-51: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNon-null assertion on
startDev.
startDev!assumes the base adapter always returnsstartDev, which holds per the base-adapter snippet, but the assertion silently masks a contract change if the base adapter ever makes it optional/undefined.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/adapters/bun/src/index.ts` around lines 46 - 51, The `startDev` wrapper in `index.ts` uses a non-null assertion on `startDev`, which hides a potential contract change from the base adapter. Update the `startDev(event)` implementation to avoid `startDev!` by either validating the function exists before calling it or by structuring the adapter types so the contract is enforced without assertion, keeping the existing `event.entry` normalization intact.packages/adapters/bun/package.json (1)
1-33: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNo
devDependenciesdeclared despite build script needingtsx,esbuild, andtypescript.The
buildscript (tsc -b && tsx scripts/build.ts) relies ontypescript,tsx, andesbuild, none of which are declared as dependencies of this package. In a strict workspace (e.g. pnpm with hoisting disabled), this could causepnpm --filter@marko/run-adapter-bunbuildto fail due to phantom dependency resolution.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/adapters/bun/package.json` around lines 1 - 33, The package manifest for `@marko/run-adapter-bun` is missing the build-time tooling it uses. Update the package.json for this adapter to declare the required devDependencies for the build script in the same place as the existing scripts and metadata, so `build` can resolve `tsc`, `tsx`, and `esbuild` without relying on workspace hoisting. Keep the change scoped to the Bun adapter package manifest and ensure the dependency declarations match the tools referenced by the `build` script.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/adapters/bun/src/index.ts`:
- Around line 59-77: The spawned child process in the Bun adapter lacks an error
listener, so a failed launch can surface as an unhandled event and crash the
parent. Update the `spawn("bun", ...)` flow in the `create...` helper to attach
an `on("error", ...)` handler on `proc` and handle launch failures gracefully.
Make sure the fix is localized around the `proc` returned from `spawn` and
preserves the existing stdout/stderr piping and `close()` behavior.
- Around line 59-63: The Bun launcher in spawnBun currently invokes spawn("bun",
["run", entryFile], { shell: true }), which lets the shell reinterpret the
computed entryFile and cwd values. Update this path to avoid shell parsing
entirely by removing shell: true and using a direct spawn/execFile-style
invocation in spawnBun, while keeping the same bun run behavior and validating
the computed entryFile before execution if needed.
---
Nitpick comments:
In `@packages/adapters/bun/package.json`:
- Around line 1-33: The package manifest for `@marko/run-adapter-bun` is missing
the build-time tooling it uses. Update the package.json for this adapter to
declare the required devDependencies for the build script in the same place as
the existing scripts and metadata, so `build` can resolve `tsc`, `tsx`, and
`esbuild` without relying on workspace hoisting. Keep the change scoped to the
Bun adapter package manifest and ensure the dependency declarations match the
tools referenced by the `build` script.
In `@packages/adapters/bun/src/index.ts`:
- Around line 46-51: The `startDev` wrapper in `index.ts` uses a non-null
assertion on `startDev`, which hides a potential contract change from the base
adapter. Update the `startDev(event)` implementation to avoid `startDev!` by
either validating the function exists before calling it or by structuring the
adapter types so the contract is enforced without assertion, keeping the
existing `event.entry` normalization intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b4e7d72a-2de0-49d4-8aa4-2fa113853abb
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonand included by**packages/run/src/__tests__/fixtures/bun-adapter/__snapshots__/dev.expected.mdis excluded by!**/__snapshots__/**and included by**
📒 Files selected for processing (17)
.changeset/bun-adapter.mdpackages/adapters/bun/README.mdpackages/adapters/bun/package.jsonpackages/adapters/bun/scripts/build.tspackages/adapters/bun/scripts/importMetaURL.jspackages/adapters/bun/src/default-entry.tspackages/adapters/bun/src/index.tspackages/adapters/bun/src/types.tspackages/adapters/bun/tsconfig.jsonpackages/run/src/__tests__/fixtures/bun-adapter/.gitignorepackages/run/src/__tests__/fixtures/bun-adapter/.marko-run/routes.d.tspackages/run/src/__tests__/fixtures/bun-adapter/package.jsonpackages/run/src/__tests__/fixtures/bun-adapter/src/components/counter.markopackages/run/src/__tests__/fixtures/bun-adapter/src/routes/+page.markopackages/run/src/__tests__/fixtures/bun-adapter/test.config.tspackages/run/src/__tests__/fixtures/bun-adapter/tsconfig.jsonpackages/run/src/__tests__/fixtures/bun-adapter/vite.config.ts
The runtime's fetch returns a 404 response (not undefined) for unmatched paths, so the entry must fall back to serving static assets on a 404 rather than only when fetch returns nothing. Otherwise requests for static files (eg. /assets/*.js) returned 404 and the client bundle never loaded. Verified end-to-end under Bun.
- Add package.toggle.json so publish points exports/types at dist/ (the pkg-toggle release step skips packages without it) - Make the injected import.meta.url shim a string (.href) so the CJS build's path.dirname(import.meta.url) works - Fix the README platform example to read from context.platform (handlers receive (context, next), not a platform destructure) - Preserve an explicit PORT=0 (free port) instead of falling back to 3000 - Attach an error handler to the preview child process and throw instead of exiting the process when the Bun CLI is missing
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/adapters/bun/src/index.ts (1)
59-63: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
shell: truewith computedentryFile/cwdremains unresolved.This still spawns via
shell: truewithentryFilederived fromdir/entry. On POSIX, args are joined into a single string parsed by/bin/sh -c, so shell metacharacters or spaces inentryFile/cwdcan be mis-parsed or enable injection if those values are ever influenced by less-trusted config. This diff only adds the error handler; the underlyingshell: truerisk previously flagged is still present.🔒 Suggested fix: avoid shell interpretation
- const proc = spawn("bun", ["run", entryFile], { - cwd, - env: { ...process.env, PORT: port.toString() }, - shell: true, - }); + const proc = spawn(process.platform === "win32" ? "bun.cmd" : "bun", ["run", entryFile], { + cwd, + env: { ...process.env, PORT: port.toString() }, + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/adapters/bun/src/index.ts` around lines 59 - 63, The spawn call in the bun adapter still uses shell: true with a computed entryFile, which leaves the command subject to shell parsing and potential misinterpretation. Update the process launch in the bun adapter’s spawn call to avoid shell interpretation entirely, and pass the executable/arguments in a way that preserves the computed entryFile and cwd safely without invoking a shell.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@packages/adapters/bun/src/index.ts`:
- Around line 59-63: The spawn call in the bun adapter still uses shell: true
with a computed entryFile, which leaves the command subject to shell parsing and
potential misinterpretation. Update the process launch in the bun adapter’s
spawn call to avoid shell interpretation entirely, and pass the
executable/arguments in a way that preserves the computed entryFile and cwd
safely without invoking a shell.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ea0c8ba8-46ba-4fbd-ae47-63e3bebb9954
📒 Files selected for processing (5)
packages/adapters/bun/README.mdpackages/adapters/bun/package.toggle.jsonpackages/adapters/bun/scripts/importMetaURL.jspackages/adapters/bun/src/default-entry.tspackages/adapters/bun/src/index.ts
✅ Files skipped from review due to trivial changes (1)
- packages/adapters/bun/README.md
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/adapters/bun/scripts/importMetaURL.js
- packages/adapters/bun/src/default-entry.ts
Use `shell: true` only on Windows (where `bun` may be a `.cmd` shim). On POSIX the CLI is spawned directly, so `close()` kills Bun itself instead of an orphaned shell wrapper and argument values are no longer re-parsed by the shell. Verified end-to-end under Bun: the child receives SIGTERM directly on close.
Clarify that Marko Run automatically uses an installed adapter with no Vite config, and put the `npm run build` + `bun run dist/index.mjs` steps in a Running section right after installation.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds a new adapter package,
@marko/run-adapter-bun, for running and deploying Marko Run apps on Bun.dist/index.mjs) that usesBun.serve. It targets Node (Bun is Node-compatible) with thebunexport condition andnoExternal: true.dist/publicviaBun.file— content-type is inferred by Bun,/assets/*getsimmutablecaching, and a path-traversal guard is applied. Listens on thePORTenvironment variable (default3000).bun run dist/index.mjs.Bun.serveserver instance (exposingrequestIP, etc.) via the exportedBunPlatformInfotype.Also adds a test fixture to the
@marko/runpackage (bun-adapter) mirroring the existing adapter fixtures, and a changeset for the new package.Motivation and Context
Bun is a popular, fast JavaScript runtime with a web-standard server API (
Bun.serve), making it a natural target for Marko Run. This provides a zero-config, self-contained build that runs on Bun, alongside the existing Node, static, and Netlify adapters.Screenshots (if appropriate):
Checklist:
Generated by Claude Code