Conversation
…e website to `apps/website` and database logic to `packages/db`.
…Prisma to Drizzle, and update website configuration for new packages and serverless deployment.
…date README instructions for setup.
…setup instructions.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Summary of ChangesHello @iHildy, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request represents a significant architectural shift, transforming the project into a Cloudflare-native pnpm monorepo. The core changes involve migrating the database from Prisma/Supabase to Drizzle ORM with Cloudflare D1, integrating Cloudflare R2 for object storage, and adopting BetterAuth for authentication. The project structure is now organized into distinct Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This is an impressive and comprehensive refactoring to convert the project into a Cloudflare-native monorepo. The migration from Prisma/Supabase to Drizzle/D1 and the adoption of a pnpm workspace structure are well-executed and set a strong foundation for future development. The new packages for db, auth, and storage create clear separation of concerns, and the inclusion of a setup.sh script is a great addition for developer experience. I've identified a few areas for improvement to enhance robustness, type safety, and fix a minor bug in the setup script. Overall, this is an excellent pull request.
apps/website/src/server/trpc.ts
Outdated
| // This is a placeholder that will be replaced when running with wrangler dev | ||
| return createInnerTRPCContext({ | ||
| headers: opts.req.headers, | ||
| db: {} as DbClient, // Will be replaced by actual binding in runtime |
There was a problem hiding this comment.
Using a type assertion {} as DbClient compromises type safety, even in a local development context. If the runtime injection by Wrangler fails or doesn't behave as expected (e.g., when running pnpm dev without wrangler dev), this could lead to runtime errors that are hard to debug because they are not caught by TypeScript. Consider creating a lightweight mock DbClient that either throws informative errors for all methods or provides a basic in-memory implementation. This would make the local development setup more robust and type-safe.
packages/db/drizzle.config.ts
Outdated
| accountId: process.env.CLOUDFLARE_ACCOUNT_ID!, | ||
| databaseId: process.env.CLOUDFLARE_D1_DATABASE_ID!, | ||
| token: process.env.CLOUDFLARE_D1_TOKEN!, |
There was a problem hiding this comment.
Using non-null assertions (!) for environment variables can mask configuration issues and lead to runtime errors if the variables are not set. It's safer to validate these variables at the start of the script and throw a clear, informative error if they are missing. This fail-fast approach makes configuration problems much easier to diagnose.
For example, you could add a small helper function:
function getRequiredEnv(key: string): string {
const value = process.env[key];
if (!value) throw new Error(`Missing required environment variable: ${key}`);
return value;
}
// Then use it like this:
// accountId: getRequiredEnv('CLOUDFLARE_ACCOUNT_ID'),| read -p "Enter the existing database ID (or press Enter to skip): " DATABASE_ID | ||
| if [ -z "$DATABASE_ID" ]; then | ||
| print_error "Database ID is required." | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
There's a logical contradiction here. The prompt on line 169 says (or press Enter to skip), but if the user presses Enter, DATABASE_ID becomes empty, and the check on line 170 causes the script to exit with an error. This will be confusing for the user.
The script should either handle the 'skip' case gracefully by using a placeholder and warning the user, or remove the 'skip' option from the prompt to avoid confusion.
| read -p "Enter the existing database ID (or press Enter to skip): " DATABASE_ID | |
| if [ -z "$DATABASE_ID" ]; then | |
| print_error "Database ID is required." | |
| exit 1 | |
| fi | |
| read -p "Enter the existing database ID (or press Enter to skip manual entry): " DATABASE_ID | |
| if [ -z "$DATABASE_ID" ]; then | |
| print_warning "Skipping database ID entry. You will need to update wrangler.toml manually." | |
| DATABASE_ID="YOUR_DATABASE_ID_HERE" # Use a placeholder | |
| fi |
AGENTS.md
Outdated
| # AGENTS RULES OVERVIEW | ||
|
|
||
| This document condenses the most critical rules for agents working on this repository. Note that YOU doesn't have access to enviormental variables and cannot run commands like `pnpm build` because of this, instead use `pnpm lint` | ||
| This document condenses the most critical rules for agents working on this repository. Note that YOU doesn't have access to environmental variables and cannot run commands like `pnpm build` because of this, instead use `pnpm lint` |
There was a problem hiding this comment.
There's a small grammatical error in this line. When addressing YOU, it should be don't instead of doesn't.
| This document condenses the most critical rules for agents working on this repository. Note that YOU doesn't have access to environmental variables and cannot run commands like `pnpm build` because of this, instead use `pnpm lint` | |
| This document condenses the most critical rules for agents working on this repository. Note that YOU don't have access to environmental variables and cannot run commands like `pnpm build` because of this, instead use `pnpm lint` |
…ion-18255939110837213705 Fix review comments and add DB env validation
… Slack notifications, and improved onboarding.
…e-hildy-app into cloudflare-monorepo
1 participant
still kept
vercel-unorepoas a snapshot of the current main before this.