fix(web): improve stack builder logic - #737
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThis PR consolidates stack compatibility logic into a centralized Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
apps/web/src/app/(home)/_components/stats-section.tsx (1)
72-77: Consider a clearer fallback for missinglastUpdated.Displaying the current date when
lastUpdatedis null could mislead users into thinking data was recently updated. Consider showing "—" or "N/A" instead to indicate missing data.🔎 Suggested change
<span className="truncate font-mono text-accent"> - {lastUpdated || - new Date().toLocaleDateString("en-US", { - month: "short", - day: "numeric", - year: "numeric", - })} + {lastUpdated || "—"} </span>apps/web/src/app/(home)/new/_components/utils.ts (3)
39-43: Usetypealias instead ofinterfaceper coding guidelines.As per coding guidelines, TypeScript type aliases are preferred over interface declarations.
🔎 Suggested change
-interface CompatibilityResult { - adjustedStack: StackState | null; - notes: Record<string, { notes: string[]; hasIssue: boolean }>; - changes: Array<{ category: string; message: string }>; -} +type CompatibilityResult = { + adjustedStack: StackState | null; + notes: Record<string, { notes: string[]; hasIssue: boolean }>; + changes: Array<{ category: string; message: string }>; +};
50-50: Consider using standard function declaration per coding guidelines.The coding guidelines specify using standard function declarations instead of arrow functions. However, given this is a large function and the pattern is consistent throughout the file, this is a low-priority refactor.
490-516: Minor redundancy in Polar payment checks.Multiple independent conditions can set
payments = "none"and push changes. If multiple conditions are true, duplicate change messages could be added. Consider consolidating these checks.🔎 Suggested consolidation
if (nextStack.payments === "polar") { - if (nextStack.auth !== "better-auth") { - nextStack.payments = "none"; - changed = true; - changes.push({ - category: "payments", - message: "Payments set to 'None' (Polar requires Better Auth)", - }); - } - if (nextStack.backend === "convex") { - nextStack.payments = "none"; - changed = true; - changes.push({ - category: "payments", - message: "Payments set to 'None' (Polar incompatible with Convex)", - }); - } const hasWebFrontend = nextStack.webFrontend.some((f) => f !== "none"); - if (!hasWebFrontend) { + const polarIncompatible = + nextStack.auth !== "better-auth" || + nextStack.backend === "convex" || + !hasWebFrontend; + + if (polarIncompatible) { nextStack.payments = "none"; changed = true; - changes.push({ - category: "payments", - message: "Payments set to 'None' (Polar requires web frontend)", - }); + const reason = nextStack.auth !== "better-auth" + ? "Polar requires Better Auth" + : nextStack.backend === "convex" + ? "Polar incompatible with Convex" + : "Polar requires web frontend"; + changes.push({ category: "payments", message: `Payments set to 'None' (${reason})` }); } }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web/src/app/(home)/_components/stats-section.tsx(4 hunks)apps/web/src/app/(home)/new/_components/utils.ts(3 hunks)apps/web/src/app/(home)/page.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/better-t-stack-repo.mdc)
Define functions using the standard function declaration syntax, not arrow functions
Files:
apps/web/src/app/(home)/_components/stats-section.tsxapps/web/src/app/(home)/page.tsxapps/web/src/app/(home)/new/_components/utils.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/better-t-stack-repo.mdc)
**/*.{ts,tsx}: Use TypeScript type aliases instead of interface declarations
Do not use explicit return types
Files:
apps/web/src/app/(home)/_components/stats-section.tsxapps/web/src/app/(home)/page.tsxapps/web/src/app/(home)/new/_components/utils.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc)
**/*.{ts,tsx,js,jsx}: Usebun <file>instead ofnode <file>orts-node <file>for running TypeScript/JavaScript files
Bun automatically loads .env files, so don't use the dotenv package
UseBun.serve()which supports WebSockets, HTTPS, and routes instead ofexpress
Usebun:sqlitemodule for SQLite instead ofbetter-sqlite3
UseBun.redisfor Redis instead ofioredis
UseBun.sqlfor Postgres instead ofpgorpostgres.js
Use built-inWebSocketinstead of thewspackage
PreferBun.fileovernode:fsreadFile/writeFile methods
UseBun.$template literal syntax instead ofexecafor shell command execution
Import .css files directly in TypeScript/JavaScript files; Bun's CSS bundler will handle bundling
Run server withbun --hot <file>to enable hot reloading during development
Files:
apps/web/src/app/(home)/_components/stats-section.tsxapps/web/src/app/(home)/page.tsxapps/web/src/app/(home)/new/_components/utils.ts
**/*.{ts,tsx,js,jsx,css}
📄 CodeRabbit inference engine (.cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc)
Use
bun build <file>instead ofwebpackoresbuildfor bundling TypeScript, JavaScript, and CSS files
Files:
apps/web/src/app/(home)/_components/stats-section.tsxapps/web/src/app/(home)/page.tsxapps/web/src/app/(home)/new/_components/utils.ts
**/*.{html,tsx,ts,jsx,js}
📄 CodeRabbit inference engine (.cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc)
Use HTML imports with
Bun.serve()for frontend instead of Vite
Files:
apps/web/src/app/(home)/_components/stats-section.tsxapps/web/src/app/(home)/page.tsxapps/web/src/app/(home)/new/_components/utils.ts
🔇 Additional comments (11)
apps/web/src/app/(home)/page.tsx (1)
24-24: Good fix removing the duplicatemx-autoclass.apps/web/src/app/(home)/_components/stats-section.tsx (2)
21-23: Verify the average calculation logic.The calculation
totalProjects / dailyStats.lengthassumesdailyStatscontains one entry per day. IfdailyStatshas gaps (non-consecutive days) or represents something other than daily entries, this average may be misleading.
45-65: LGTM!The UI bindings correctly use the new inline computed values, and the label change accurately reflects the removal of the 30-day scope.
apps/web/src/app/(home)/new/_components/utils.ts (8)
73-94: LGTM!The Convex backend constraints correctly override dependent options and track changes appropriately.
153-187: LGTM!The "no backend" constraints correctly cascade to disable all backend-dependent options.
189-229: LGTM!Fullstack backend constraints correctly enforce runtime/serverDeploy to none and ensure the appropriate frontend is selected.
236-262: LGTM!The Workers runtime constraints correctly enforce Hono backend, server deployment, and handle MongoDB incompatibility by switching to SQLite with D1.
285-362: LGTM!The database and ORM constraints correctly handle the interdependencies, including MongoDB requiring Prisma/Mongoose and relational DBs requiring Drizzle/Prisma.
364-457: LGTM!The DB Setup constraints correctly enforce database requirements for each setup option and handle incompatibilities appropriately.
609-922: Well-structured disabled reason logic with clear documentation.The function correctly implements the philosophy of only disabling truly incompatible options while allowing selections that trigger auto-adjustments. The sectioned comments improve readability.
924-933: LGTM!Clean wrapper function that correctly handles YOLO mode bypass and delegates to
getDisabledReason.
Summary by CodeRabbit
Release Notes
Bug Fixes
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.