chore: move api to vercel #257
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded@mezotv has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 16 minutes and 26 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
WalkthroughMigration from Cloudflare Workers (Wrangler) to Vercel deployment platform. Environment variable access refactored from Hono context ( Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Vercel as Vercel Runtime
participant App as Hono App (index.ts)
participant MW as Global Middleware
participant Routes as Route Handlers
participant Env as process.env
Client->>Vercel: HTTP request
Vercel->>App: route to app
App->>MW: run cache-control, ratelimit, analytics, trailing-slash
MW->>Env: read REDIS_URL / REDIS_TOKEN / DATABASE_URL
MW-->>App: middleware outcome
App->>Routes: dispatch to workspace/v1 handlers
Routes->>Env: check DATABASE_URL / POLAR_ACCESS_TOKEN
alt required env present
Routes->>Routes: query DB / emit analytics
Routes->>Client: 200 + payload
else missing required env
Routes->>Client: 500 Internal Server Error
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
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. Comment |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ❌ Deployment failed View logs |
marble-api | 8edb62d | Nov 24 2025, 11:09 PM |
![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 (3)
apps/api/.env.example (1)
1-4: Align.env.examplewith dotenv-linter expectationsThe variable set looks fine. To stay in sync with
dotenv-linter(per the hints), consider:
- Adding a trailing newline at the end of the file.
- Optionally reordering keys if you want to keep alphabetical ordering enforced, or relaxing that rule in the linter config if intentional.
apps/api/tsconfig.json (1)
3-12: Node/Vercel tsconfig looks good; ensuredist/is ignoredThe shift to
"module": "NodeNext","types": ["node"], andjsxImportSource: "hono/jsx"is consistent with the new runtime. Adding"outDir": "./dist"is fine, but note thatdist/is no longer ignored inapps/api/.gitignore.If you plan to run
tsclocally, consider re‑ignoringapps/api/dist(either here or at the repo root) so build artifacts don’t get committed.apps/api/src/index.ts (1)
39-41: Confirm analytics behavior on Node/Vercel and ENVIRONMENT usageTwo things to verify around
analytics()being mounted on/v1/:workspaceId/*:
executionCtxon Node/Vercel
Theanalyticsmiddleware still relies onc.executionCtx?.waitUntil(task());. On a Node/Vercel runtime,executionCtxmay not be populated the same way as on Cloudflare Workers. If it’s undefined, the analytics task will never run.
- If Vercel’s adapter for Hono does not provide
executionCtx, consider falling back to invokingtask()directly (or adapting to whatever background mechanism Vercel exposes).
ENVIRONMENTvs new env typing
analyticsdestructuresENVIRONMENTfromc.env, butEnvno longer contains that field and.env.exampleno longer documents it. If you are moving away from a customENVIRONMENTvariable, you may want to:
- Switch
analyticsto useNODE_ENV/VERCEL_ENVinstead, or- Reintroduce and document
ENVIRONMENTso the production/dev Polar behavior is explicit.Please confirm the intended behavior on the new runtime and adjust
analyticsaccordingly if needed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
.vscode/settings.json(1 hunks)apps/api/.env.example(1 hunks)apps/api/.gitignore(1 hunks)apps/api/README.md(1 hunks)apps/api/package.json(1 hunks)apps/api/src/app.ts(0 hunks)apps/api/src/index.ts(1 hunks)apps/api/src/routes/authors.ts(1 hunks)apps/api/src/types/env.ts(0 hunks)apps/api/tsconfig.json(1 hunks)apps/api/wrangler.toml(0 hunks)
💤 Files with no reviewable changes (3)
- apps/api/src/types/env.ts
- apps/api/src/app.ts
- apps/api/wrangler.toml
🧰 Additional context used
🧬 Code graph analysis (1)
apps/api/src/index.ts (3)
apps/api/src/types/env.ts (1)
Env(1-6)apps/api/src/middleware/ratelimit.ts (1)
ratelimit(14-56)apps/api/src/middleware/analytics.ts (1)
analytics(5-100)
🪛 dotenv-linter (4.0.0)
apps/api/.env.example
[warning] 4-4: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
[warning] 4-4: [UnorderedKey] The POLAR_ACCESS_TOKEN key should go before the REDIS_TOKEN key
(UnorderedKey)
🪛 GitHub Actions: Code quality
apps/api/src/routes/authors.ts
[error] 2-8: Ultracite check failed due to formatting issue. Import formatting would be adjusted by formatter (e.g., combining named imports). See diff in the reported content.
🪛 markdownlint-cli2 (0.18.1)
apps/api/README.md
9-9: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
14-14: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
20-20: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
27-27: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Workers Builds: marble-api
🔇 Additional comments (4)
.vscode/settings.json (1)
41-41: Clarify the purpose of pinning Prisma to version 6.The new setting pins the Prisma editor extension to Prisma 6. Please clarify:
- Is Prisma 6 the baseline for this project, or a new requirement introduced by the Vercel migration?
- Are there version-specific API changes or breaking changes in Prisma that necessitate this pin?
- Is this setting documented elsewhere in the project (e.g.,
README.md, migration guide)?apps/api/.gitignore (1)
29-29: Ignore.verceldirectory – LGTMAdding
.vercelto.gitignoreis appropriate for the Vercel-based workflow; no issues here.apps/api/package.json (1)
3-24: Vercel CLI scripts look consistent; verify tool versions existThe new
vc-based scripts (dev,build,deploy) and the Hono bump to^4.10.6look consistent with the move to Vercel.Please double‑check that the devDependency versions you’re pinning actually exist and are the ones you want to standardize on, especially:
"typescript": "^5.8.3""vc": "^0.1.4"to avoid
npm installfailures in CI or on fresh environments.apps/api/src/index.ts (1)
14-37: Global cache middleware and overall Hono wiring look solidThe
stale-if-errormiddleware is scoped correctly to successful GET/HEAD responses and avoids clobbering existingno-store/stale-if-errordirectives, and the overall Hono setup (app/v1 separation, health routes, trailing‑slash handling, and workspace routing) reads clean and coherent. No issues from a routing/middleware-order standpoint.
| # Marble API | ||
|
|
||
| API endpoints users can fetch data from. | ||
| Prerequisites: | ||
|
|
||
| - [Vercel CLI](https://vercel.com/docs/cli) installed globally | ||
|
|
||
| To develop locally: | ||
|
|
||
| ``` | ||
| npm install | ||
| vc dev | ||
| ``` | ||
|
|
||
| ``` | ||
| open http://localhost:3000 | ||
| ``` | ||
|
|
||
| To build locally: | ||
|
|
||
| ``` | ||
| npm install | ||
| vc build | ||
| ``` | ||
|
|
||
| To deploy: | ||
|
|
||
| ``` | ||
| npm install | ||
| vc deploy | ||
| ``` |
There was a problem hiding this comment.
Align dev port with scripts and add languages to code fences
Two things to tighten up here:
-
Port mismatch
package.jsondefines"dev": "vc dev --listen 8787", but the README says to openhttp://localhost:3000.- Please align these so the documented URL matches the actual dev port (or drop
--listenand rely on the default if 3000 is intended).
-
Fenced code block languages
- The code fences for the CLI commands currently have no language. Adding something like
```bashwill satisfy markdownlint and improve readability.
- The code fences for the CLI commands currently have no language. Adding something like
Optionally, you could also show npm run dev, npm run build, and npm run deploy here to mirror the scripts and avoid duplicating the raw vc invocations.
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
9-9: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
14-14: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
20-20: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
27-27: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In apps/api/README.md around lines 1 to 30, the README's dev URL and code fences
are wrong: update the dev instructions so the documented URL matches
package.json (either change the README to open http://localhost:8787 to match
"vc dev --listen 8787" or remove the --listen flag in package.json so the
default 3000 is used), convert the triple-backtick blocks to bash fenced code
blocks (use ```bash) for all CLI examples, and optionally replace raw vc
commands with their npm script equivalents (npm run dev, npm run build, npm run
deploy) so README mirrors package.json scripts.
…vercel-functions
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/api/src/index.ts (2)
40-43: v1 workspace middleware ordering is reasonable; keep future v1 routes in mindMounting
ratelimit,validateWorkspace, andanalyticson/v1/:workspaceId/*ensures all current v1 workspace routes go through the full protection + metrics stack in the intended order.Just be aware that any future v1 routes that don’t include
:workspaceId(e.g./v1/health) will bypass these middlewares; that may or may not be what you want.
45-66: Limit workspace redirects to safe methods to avoid surprising behaviorThe workspace redirect (
/:workspaceId/*→/v1/:workspaceId/*for tags/categories/posts/authors) is well-scoped and avoids/v1,/, and/status, and its interaction withtrimTrailingSlash()is correct.Right now it runs for all HTTP methods, so a
POST /:workspaceId/posts(if ever used) would get a 308 redirect too. Many clients will follow this correctly, but it’s more conventional to only auto-redirect safe methods.Consider adding a method guard at the top of this middleware:
app.use("/:workspaceId/*", async (c, next) => { - const path = c.req.path; + const method = c.req.method; + if (method !== "GET" && method !== "HEAD") { + return next(); + } + const path = c.req.path; const workspaceId = c.req.param("workspaceId");This keeps redirects purely for read-only traffic.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/api/package.json(1 hunks)apps/api/src/index.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/api/package.json
🧰 Additional context used
🧬 Code graph analysis (1)
apps/api/src/index.ts (4)
apps/api/src/types/env.ts (1)
Env(1-6)apps/api/src/middleware/ratelimit.ts (1)
ratelimit(14-56)apps/api/src/middleware/workspace.ts (1)
validateWorkspace(4-44)apps/api/src/middleware/analytics.ts (1)
analytics(5-100)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Workers Builds: marble-api
🔇 Additional comments (2)
apps/api/src/index.ts (2)
15-37: Globalstale-if-errorcache middleware looks solidThe global middleware correctly:
- Runs post-handler (
await next()).- Restricts behavior to
GET/HEAD.- Only applies to 2xx/3xx responses.
- Preserves existing
Cache-Controlwhile skipping whenno-storeor an existingstale-if-errordirective is present.This is a good, conservative default and should be safe against most responses without surprising overrides.
68-80: Health endpoints and v1 route mounting are clear and consistentThe health endpoints (
/and/status) are straightforward and cheap, and the v1 router composition (app.route("/v1", v1);with the four workspace subroutes) cleanly isolates versioned API surface. This structure should make future versioning and workspace-specific additions easy to reason about.
| import { Hono } from "hono"; | ||
| import { trimTrailingSlash } from "hono/trailing-slash"; | ||
| import { analytics } from "./middleware/analytics"; | ||
| import { ratelimit } from "./middleware/ratelimit"; | ||
| import { validateWorkspace } from "./middleware/workspace"; | ||
| import authorsRoutes from "./routes/authors"; | ||
| import categoriesRoutes from "./routes/categories"; | ||
| import postsRoutes from "./routes/posts"; | ||
| import tagsRoutes from "./routes/tags"; | ||
| import type { Env } from "./types/env"; | ||
|
|
||
| const app = new Hono<{ Bindings: Env }>(); | ||
| const v1 = new Hono<{ Bindings: Env }>(); | ||
|
|
||
| const staleTime = 3600; | ||
|
|
There was a problem hiding this comment.
Align Env bindings with analytics middleware expectations
Env (from ./types/env) no longer includes ENVIRONMENT, but analytics (imported on Line 3) still destructures ENVIRONMENT from c.env. With app and v1 typed as Hono<{ Bindings: Env }>() (Lines 12–13), this will cause TypeScript errors in analytics.ts and/or incorrect assumptions about available bindings at runtime.
Either:
- Reintroduce
ENVIRONMENTintoEnv, or - Update
analyticsto deriveisProductionfrom something likeprocess.env.NODE_ENVand removeENVIRONMENTfrom itsc.envusage, and keepEnvas-is.
🤖 Prompt for AI Agents
In apps/api/src/index.ts lines 1–16: the app/v1 Hono bindings use Env but
analytics middleware expects c.env.ENVIRONMENT which no longer exists in
./types/env; either add ENVIRONMENT:string to the Env binding type so
c.env.ENVIRONMENT is available, or modify the analytics middleware to stop
destructuring ENVIRONMENT from c.env and instead derive isProduction from
process.env.NODE_ENV (and update any types/usages accordingly) — pick one
approach and make the corresponding change to the Env type or to analytics so
TypeScript and runtime assumptions are consistent.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/api/package.json(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Workers Builds: marble-api
🔇 Additional comments (1)
apps/api/package.json (1)
19-24: Verify build and deployment configuration is present.The migration adds
@types/node,tsx,typescript, andvcto devDependencies, but there's no visibility into supporting configuration files (e.g.,vercel.json,tsconfig.json). Ensure that:
vercel.json(or equivalent) exists and correctly configures the build command and function entry points- TypeScript build output is properly directed for Vercel Functions
- Environment variables from Cloudflare (removed
.dev.varsscript) are migrated to Vercel's environment configuration
| "dev": "vc dev --listen 8787", | ||
| "build": "vc build", | ||
| "deploy": "vc deploy" |
There was a problem hiding this comment.
🧩 Analysis chain
Verify the vc package name and version (Line 23).
The scripts have been updated to use vc dev, vc build, and vc deploy commands, but the vc package at version ^0.1.4 appears suspicious. The standard Vercel CLI is typically the vercel package (not vc), and 0.1.4 suggests an early beta version.
Confirm that:
vcis the intended package (not a typo forvercel)- The package is stable and suitable for CI/CD deployments
- The CLI commands match what's in the scripts
🌐 Web query:
What is the vc npm package and what version should be used with Vercel functions?
💡 Result:
Short answer
- The npm package named "vc" is an old/unrelated utility (years-old tool to bump/change a package.json version). [1]
- For working with Vercel (local dev, deploy, and Functions) use the official Vercel CLI (the vc commands used in Vercel docs/templates). Install the official CLI per Vercel docs (examples show using vc dev, vc deploy, vc link) rather than the unrelated "vc" npm package. [2]
Sources
- npm entry for the unrelated "vc" package. [1]
- Vercel docs/templates using the official CLI (vc dev / vc deploy / vc link). [2]
Remove the vc npm package from devDependencies (Line 23) and install the official Vercel CLI instead.
The npm package vc is an old/unrelated utility for version bumping, not the Vercel CLI. Your scripts reference vc dev, vc build, and vc deploy commands, which are part of the official Vercel CLI, but installing this unrelated vc package will cause those commands to fail. The official CLI should be installed per Vercel docs, not via this unrelated package. Verify the correct installation method in Vercel's documentation to ensure your deployment pipeline works as intended.
🤖 Prompt for AI Agents
In apps/api/package.json around lines 5 to 7 (and devDependencies at line 23),
the project mistakenly installs the unrelated "vc" npm package while scripts use
Vercel CLI commands; remove "vc" from devDependencies and install the official
Vercel CLI instead (e.g., add "vercel" as a devDependency or follow Vercel docs
for global/CI installation), then verify the existing scripts ("vc dev", "vc
build", "vc deploy") are either updated to use "vercel" or that package.json
scripts map to the installed CLI (e.g., use "vercel dev", "vercel build",
"vercel deploy") so the commands run correctly in local and CI environments.
| "@upstash/ratelimit": "^2.0.5", | ||
| "@upstash/redis": "^1.34.4", | ||
| "hono": "^4.10.3", | ||
| "hono": "^4.10.6", |
There was a problem hiding this comment.
🧩 Analysis chain
Verify Hono 4.10.6 compatibility with Node.js/Vercel runtime.
The hono dependency was bumped from ^4.10.3 to ^4.10.6 as part of the Wrangler → Vercel migration. Ensure this version is compatible with the Node.js Vercel Functions runtime and the new entry point setup in src/index.ts.
🌐 Web query:
Hono 4.10.6 Node.js Vercel Functions compatibility
💡 Result:
Short answer: Yes — Hono 4.10.6 is compatible with Vercel Node.js Functions, but you must use the appropriate Hono adapter/handler and follow Vercel/Next.js runtime guidance.
Key points and actions
- Use the Vercel adapter shipped with Hono: import and export the handler from hono/vercel (App Router) or use @hono/node-server/vercel for Next.js Pages Router. [1][2][3]
- For Next.js App Router set export const runtime = 'nodejs' and export GET/POST = handle(app) (example in docs). [1]
- For Pages Router install @hono/node-server and use its vercel handle; also set NODEJS_HELPERS=0 in your Vercel environment or .env as the docs instruct. [2][3]
- Ensure Node.js version >= 18 (Hono documents minimum Node requirements and specific minima per minor Node release). [4]
- If deploying ESM/TypeScript to Vercel functions, confirm package.json "type":"module" and proper module settings to avoid CJS/ESM mismatch (common Vercel issue). Community guidance suggests using ES module config. [5]
Sources
- Hono Vercel guide / examples. [1]
- Hono + Next.js / Pages Router instructions (use @hono/node-server + NODEJS_HELPERS=0). [2][3]
- Hono Node.js runtime requirements (Node >=18 and specific minimum releases). [4]
- Vercel community thread re: Node/ESM issues when deploying Hono on Vercel. [5]
If you want, I can: 1) show a minimal working example for App Router or Pages Router on Vercel using Hono 4.10.6, or 2) inspect your project config (package.json, Next.js router type, Node version) to point out any required changes.
🏁 Script executed:
#!/bin/bash
# Check the entry point setup in src/index.ts
echo "=== src/index.ts ==="
cat src/index.ts 2>/dev/null || echo "File not found"
echo -e "\n=== package.json (relevant fields) ==="
# Extract relevant fields from package.json
python3 << 'EOF'
import json
try:
with open('apps/api/package.json', 'r') as f:
pkg = json.load(f)
print(f"Type: {pkg.get('type', 'not specified')}")
print(f"Engines: {pkg.get('engines', {})}")
print(f"Scripts (main): {pkg.get('scripts', {}).get('build', 'N/A')}")
if 'hono' in pkg.get('dependencies', {}):
print(f"Hono version: {pkg['dependencies']['hono']}")
except Exception as e:
print(f"Error: {e}")
EOFLength of output: 154
🏁 Script executed:
#!/bin/bash
# Search for entry points and Vercel configuration
echo "=== Directory structure in apps/api ==="
ls -la apps/api/ 2>/dev/null | head -20
echo -e "\n=== Search for src or api directories ==="
find apps/api -type d -name "src" -o -name "api" 2>/dev/null
echo -e "\n=== Look for Vercel config and entry points ==="
find apps/api -maxdepth 2 -type f \( -name "vercel.json" -o -name "*.ts" -o -name "*.js" \) 2>/dev/null | grep -v node_modules | head -15
echo -e "\n=== Check main/exports in package.json ==="
python3 << 'EOF'
import json
try:
with open('apps/api/package.json', 'r') as f:
pkg = json.load(f)
print(f"Main: {pkg.get('main', 'not specified')}")
print(f"Exports: {pkg.get('exports', 'not specified')}")
except Exception as e:
print(f"Error: {e}")
EOFLength of output: 725
🏁 Script executed:
#!/bin/bash
echo "=== apps/api/src/index.ts ==="
cat apps/api/src/index.ts
echo -e "\n=== apps/api/tsconfig.json ==="
cat apps/api/tsconfig.jsonLength of output: 2974
Update src/index.ts to use Hono's Vercel adapter for proper Node.js Vercel Functions compatibility.
While Hono 4.10.6 is compatible with Vercel, the entry point in apps/api/src/index.ts exports a raw Hono app instance (export default app) instead of using the required Vercel adapter. For Vercel Node.js Functions, the handler must be exported via hono/vercel. Update the export to use the appropriate Vercel adapter (e.g., export const GET = handle(app) with the Vercel adapter) to ensure compatibility.
🤖 Prompt for AI Agents
In apps/api/src/index.ts (entry export portion), the file currently exports the
raw Hono app instance which is incompatible with Vercel Node.js Functions;
import the Vercel adapter from 'hono/vercel' (handle) and replace the direct app
export with Vercel-compatible handler exports — e.g., export the adapter-wrapped
handlers such as export const GET = handle(app) and export const POST =
handle(app) (and any other HTTP methods your app supports, e.g., OPTIONS) so
Vercel will invoke the app correctly.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/api/package.json (1)
5-7: Minor: Consider usingvercelinstead ofvcin script commands for explicitness.The scripts currently use
vc dev,vc build, andvc deploy. Whilevcis an alias in the official Vercel CLI package and should work in npm scripts, it's more conventional and explicit to use the full command namevercel.Consider updating the scripts to use the explicit command name:
- "dev": "vc dev --listen 8787", - "build": "vc build", - "deploy": "vc deploy" + "dev": "vercel dev --listen 8787", + "build": "vercel build", + "deploy": "vercel deploy"This makes it clearer which package provides the commands and aligns with Vercel's official documentation examples.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (1)
apps/api/package.json(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Workers Builds: marble-api
🔇 Additional comments (2)
apps/api/package.json (2)
20-23: Excellent: Critical issue resolved with official Vercel CLI.The critical package issue flagged in the past review has been fixed. The package now correctly installs
vercel(the official CLI at v48.10.10) instead of the unrelatedvcutility. The addition of@types/node,tsx, andtypescriptalso aligns well with the Node.js/Vercel environment.
3-3: Verify Vercel runtime compatibility with Hono 4.10.6.While the Hono version bump (4.10.3 → 4.10.6) is reasonable, ensure that the entry point in
src/index.tsuses Hono's Vercel adapter (hono/vercel) for proper Node.js Vercel Functions compatibility, as noted in the past review. The raw app export will not work with Vercel's function handler invocation model.Please confirm that
src/index.tsexports handlers using the Vercel adapter (e.g.,export const GET = handle(app)fromhono/vercel), or open a follow-up issue if the adapter integration is still pending.Also applies to: 15-15
|
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/api/src/routes/authors.ts (1)
9-13: Extract database client initialization to reduce duplication.The same environment check and client initialization pattern appears in both route handlers. This creates maintenance overhead and redundant runtime checks on every request.
Consider extracting this logic into a helper function or initializing the client once at module level (with an early check that fails fast if DATABASE_URL is missing).
Example helper approach:
function getDbClient(c: Context) { if (!process.env.DATABASE_URL) { throw new Error("DATABASE_URL not configured"); } return createClient(process.env.DATABASE_URL); } // Then in handlers: authors.get("/", async (c) => { try { const db = getDbClient(c); // ... rest of handler } catch (error) { return c.json({ error: "Internal server error" }, 500); } });Alternatively, initialize at module level:
if (!process.env.DATABASE_URL) { throw new Error("DATABASE_URL must be configured"); } const db = createClient(process.env.DATABASE_URL); // Then handlers can use `db` directlyAlso applies to: 129-134
apps/api/src/middleware/analytics.ts (1)
12-15: Remove redundant DATABASE_URL check inside task function.The DATABASE_URL check appears twice: once at line 12 (preventing task execution) and again at lines 30-32 (inside the task). Since the outer check at line 12 already prevents the task from being scheduled when DATABASE_URL is missing, the inner check is redundant.
Apply this diff to remove the redundant check:
const task = async () => { try { - if (!process.env.DATABASE_URL) { - return; - } const db = createClient(process.env.DATABASE_URL);Also applies to: 30-32
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/api/src/middleware/analytics.ts(4 hunks)apps/api/src/middleware/authorization.ts(2 hunks)apps/api/src/routes/authors.ts(2 hunks)apps/api/src/routes/categories.ts(3 hunks)apps/api/src/routes/posts.ts(3 hunks)apps/api/src/routes/tags.ts(2 hunks)packages/db/package.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/api/src/routes/posts.ts
- apps/api/src/middleware/authorization.ts
- apps/api/src/routes/categories.ts
- apps/api/src/routes/tags.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{ts,tsx,js,jsx}: Don't useaccessKeyattribute on any HTML element
Don't setaria-hidden="true"on focusable elements
Don't add ARIA roles, states, and properties to elements that don't support them
Don't use distracting elements like<marquee>or<blink>
Only use thescopeprop on<th>elements
Don't assign non-interactive ARIA roles to interactive HTML elements
Make sure label elements have text content and are associated with an input
Don't assign interactive ARIA roles to non-interactive HTML elements
Don't assigntabIndexto non-interactive HTML elements
Don't use positive integers fortabIndexproperty
Don't include "image", "picture", or "photo" in img alt prop
Don't use explicit role property that's the same as the implicit/default role
Make static elements with click handlers use a valid role attribute
Always include atitleelement for SVG elements
Give all elements requiring alt text meaningful information for screen readers
Make sure anchors have content that's accessible to screen readers
AssigntabIndexto non-interactive HTML elements witharia-activedescendant
Include all required ARIA attributes for elements with ARIA roles
Make sure ARIA properties are valid for the element's supported roles
Always include atypeattribute for button elements
Make elements with interactive roles and handlers focusable
Give heading elements content that's accessible to screen readers (not hidden witharia-hidden)
Always include alangattribute on the html element
Always include atitleattribute for iframe elements
AccompanyonClickwith at least one of:onKeyUp,onKeyDown, oronKeyPress
AccompanyonMouseOver/onMouseOutwithonFocus/onBlur
Include caption tracks for audio and video elements
Use semantic elements instead of role attributes in JSX
Make sure all anchors are valid and navigable
Ensure all ARIA properties (aria-*) are valid
Use valid, non-abstract ARIA roles for elements with ARIA roles
Use valid AR...
Files:
apps/api/src/routes/authors.tsapps/api/src/middleware/analytics.ts
🧠 Learnings (1)
📚 Learning: 2025-11-24T22:28:24.921Z
Learnt from: CR
Repo: usemarble/marble PR: 0
File: .cursor/rules/ultracite.mdc:0-0
Timestamp: 2025-11-24T22:28:24.921Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Enforce strict type safety using Ultracite's linter and formatter
Applied to files:
apps/api/src/routes/authors.ts
🧬 Code graph analysis (2)
apps/api/src/routes/authors.ts (1)
apps/api/src/types/env.ts (1)
Env(1-7)
apps/api/src/middleware/analytics.ts (3)
packages/db/src/index.ts (1)
db(29-29)apps/api/src/lib/polar.ts (1)
createPolarClient(3-8)apps/cms/src/lib/polar/client.ts (1)
createPolarClient(5-18)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Workers Builds: marble-api
🔇 Additional comments (5)
apps/api/src/routes/authors.ts (1)
1-1: LGTM! Import path updated correctly.The import path change from
"@marble/db/workers"to"@marble/db"correctly aligns with the package.json export updates.packages/db/package.json (1)
5-12: Breaking change: "./workers" export removed.The
"./workers"export has been removed, which is a breaking change for any consumers using that import path. The PR correctly updates internal usages (authors.ts, analytics.ts, etc.) from"@marble/db/workers"to"@marble/db".This is intentional and all internal references have been updated consistently across the codebase.
apps/api/src/middleware/analytics.ts (3)
64-97: LGTM! Conditional Polar client creation handles missing token gracefully.The Polar client creation properly checks for
POLAR_ACCESS_TOKENavailability and logs when skipping Polar ingestion. The error handling for Polar API failures (which can return 500 in dev but still process events) is appropriate.
1-1: LGTM! Import path updated correctly.The import path change from
"@marble/db/workers"to"@marble/db"correctly aligns with the package.json export updates.
103-103: Code uses correct Cloudflare Workers API; Vercel deployment assumption appears incorrect.The code correctly uses
c.executionCtx?.waitUntil()which is the standard Cloudflare Workers pattern for background tasks. Evidence indicates this app is deployed to Cloudflare Workers, not Vercel:
- Hono initialized as
new Hono<{ Bindings: Env }>()— theBindingstype parameter is specific to Cloudflare Workers adapter- No Vercel configuration (vercel.json) found in apps/api
- No @vercel/functions dependency in apps/api/package.json
- The executionCtx API accessed via Hono context is the correct pattern for Cloudflare Workers
The review comment assumes Vercel deployment and recommends checking @vercel/functions, but the codebase design indicates Cloudflare Workers as the deployment target. For Cloudflare Workers, the current implementation is correct. If the actual deployment target is Vercel, the entire app structure and adapter would need to change (which appears unlikely given the current setup).
Likely an incorrect or invalid review comment.
| ".": { | ||
| "types": "./dist/index.d.ts", | ||
| "default": "./dist/index.js" | ||
| }, | ||
| "./client": { | ||
| "types": "./dist/client.d.ts", | ||
| "default": "./dist/client.js" | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Ensure build artifacts exist before package consumption.
The exports now point to ./dist/* paths, but there's no guarantee these files exist. The "build": "tsc" script is added but not automatically run before publish or when the package is consumed by workspace dependents.
Consider adding a "prepublishOnly" script to ensure the build runs before publishing, or document that consumers must run the build step manually.
Run the following to verify the build process and check if dist files are generated:
🏁 Script executed:
#!/bin/bash
# Verify build process and dist output
# Check if dist directory exists
if [ -d "packages/db/dist" ]; then
echo "✓ dist directory exists"
fd . packages/db/dist --type f --max-depth 2
else
echo "✗ dist directory missing"
fi
# Check for build-related scripts in root or workspace
rg -n "\"build\".*@marble/db|turbo.*build" package.json apps/api/package.json -A 2 -B 2
# Verify tsconfig for db package
cat packages/db/tsconfig.json | jq '.compilerOptions | {outDir, rootDir, declaration}'Length of output: 375
I'll continue verification to understand the complete build setup and the declaration configuration issue.
---
🏁 Script executed:
```shell
#!/bin/bash
# Deeper verification of build and tsconfig configuration
# Get full packages/db/package.json
echo "=== packages/db/package.json (scripts section) ==="
cat packages/db/package.json | jq '.scripts'
# Get full tsconfig to understand declaration setting
echo -e "\n=== packages/db/tsconfig.json (full compilerOptions) ==="
cat packages/db/tsconfig.json | jq '.compilerOptions'
# Check turbo.json for build task configuration
echo -e "\n=== turbo.json build configuration ==="
if [ -f "turbo.json" ]; then
cat turbo.json | jq '.pipeline.build // "not found"'
else
echo "turbo.json not found"
fi
# Check if @marble/db is a workspace dependency elsewhere
echo -e "\n=== Checking workspace dependents of @marble/db ==="
rg -l "@marble/db" --type json packages/*/package.json apps/*/package.json 2>/dev/null | head -10
Length of output: 337
I need to re-run the verification with repository access enabled.
---
🏁 Script executed:
```shell
#!/bin/bash
# Deeper verification of build and tsconfig configuration
# Get full packages/db/package.json
echo "=== packages/db/package.json (scripts section) ==="
cat packages/db/package.json | jq '.scripts'
# Get full tsconfig to understand declaration setting
echo -e "\n=== packages/db/tsconfig.json (full compilerOptions) ==="
cat packages/db/tsconfig.json | jq '.compilerOptions'
# Check turbo.json for build task configuration
echo -e "\n=== turbo.json build configuration ==="
if [ -f "turbo.json" ]; then
cat turbo.json | jq '.pipeline.build // "not found"'
else
echo "turbo.json not found"
fi
# Check if @marble/db is a workspace dependency elsewhere
echo -e "\n=== Checking workspace dependents of @marble/db ==="
rg -l "@marble/db" --type json packages/*/package.json apps/*/package.json 2>/dev/null | head -10
Length of output: 597
Final verification to confirm workspace setup and build orchestration:
---
🏁 Script executed:
```shell
#!/bin/bash
# Final verification of workspace setup
# Check root turbo.json
echo "=== Root turbo.json ==="
ls -la turbo.json 2>&1
# Check apps/cms and apps/api dependency declarations
echo -e "\n=== apps/cms/package.json @marble/db reference ==="
cat apps/cms/package.json | jq '.dependencies["@marble/db"], .devDependencies["@marble/db"]'
echo -e "\n=== apps/api/package.json @marble/db reference ==="
cat apps/api/package.json | jq '.dependencies["@marble/db"], .devDependencies["@marble/db"]'
# Check if dist is in .gitignore
echo -e "\n=== Checking .gitignore for dist ==="
rg "dist" .gitignore 2>/dev/null || echo "No dist pattern found in .gitignore"
# Verify full tsconfig declaration setting
echo -e "\n=== Full packages/db/tsconfig.json ==="
cat packages/db/tsconfig.json
Length of output: 534
Let me verify the extended tsconfig configuration:
---
🏁 Script executed:
```shell
#!/bin/bash
# Check extended tsconfig for declaration setting
echo "=== Checking for @marble/tsconfig package ==="
find . -name "base.json" -path "*/tsconfig/*" 2>/dev/null | head -5
echo -e "\n=== Root turbo.json build pipeline ==="
cat turbo.json | jq '.pipeline'
Length of output: 120
---
🏁 Script executed:
```shell
#!/bin/bash
# Final checks on tsconfig and turbo configuration
echo "=== packages/tsconfig/base.json compilerOptions ==="
cat packages/tsconfig/base.json | jq '.compilerOptions | {declaration, declarationMap, emitDeclarationOnly}'
echo -e "\n=== Full turbo.json ==="
cat turbo.json
Length of output: 1652
Add prepublishOnly script to ensure build artifacts exist.
The exports reference ./dist/* files, but dist is gitignored and won't exist on fresh checkout or when the package is consumed via npm. While turbo orchestrates builds in the monorepo context, there's no guarantee build runs before consumption, especially if this package is ever published independently.
Add to packages/db/package.json:
"prepublishOnly": "tsc"Alternatively, document that consumers must run the root build step first.
🤖 Prompt for AI Agents
In packages/db/package.json around lines 5 to 12, the package exports point to
./dist/* files but dist is gitignored and may not exist when the package is
installed or published; add a "prepublishOnly" script that runs the TypeScript
build (e.g., "prepublishOnly": "tsc") so dist artifacts are produced before
publishing, or alternatively document that consumers must run the monorepo root
build step first.
Summary by CodeRabbit
Chores
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.