-
Notifications
You must be signed in to change notification settings - Fork 0
refactor #10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThe pull request removes Husky pre-commit hook and simplifies the commit-msg hook, deletes multiple documentation and database seed/reset scripts, adds a Cloudflare environment type definition, introduces a /dashboard proxy middleware, refactors the tRPC client to direct client initialization, updates auth cookie/session settings, removes DATABASE_URL from env, updates commitlint rules, removes Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🔇 Additional comments (1)
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 |
Cloudflare DeploymentStatus: ❌ Deployment failed The deployment to Cloudflare failed. Please check the workflow logs for more details. Last updated: 12/02/2025, 21:07:10 +05:30 GMT |
Summary of ChangesHello @prdai, 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 focuses on a comprehensive maintenance and refinement effort across the project. It updates numerous dependencies to their latest versions, streamlines development workflows by adjusting Git hooks and commit linting rules, and enhances Cloudflare integration with improved type generation. The changes also include significant updates to authentication configurations for better security and session management, alongside a simplification of the project's documentation and database management scripts. The overall goal is to improve the project's stability, developer experience, and alignment with modern best practices. Highlights
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
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request includes a wide range of changes, from dependency updates and configuration adjustments to the removal of several documentation and script files. While many of the dependency updates are welcome, there are several critical issues that need to be addressed.
Key issues identified:
- Broken tRPC Client: The tRPC client setup in
src/trpc/client.tsis broken and will prevent the application from fetching data. - Broken npm Scripts: The
db:seedanddb:resetscripts inpackage.jsonare now broken as the files they point to have been removed. - Removed Pre-commit Hook: The pre-commit hook for linting has been removed, which could impact code quality.
- Hardcoded Resource ID: A hardcoded Cloudflare KV namespace ID has been committed in
wrangler.jsonc. - Documentation Removal: A significant amount of documentation (
INSTALLATION.md,PROJECT_SUMMARY.md,QUICKSTART.md) has been removed without explanation.
These issues should be resolved before merging to ensure the project remains functional and maintainable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/trpc/client.ts (1)
3-52: TRPC client/provider is currently broken—missing imports, undefined identifiers, and incomplete configurationThis file has several correctness issues that will prevent compilation and runtime execution:
- The file is named
client.tsbut contains JSX (<api.Provider>,<QueryClientProvider>). Withjsx: preservein tsconfig, JSX is only allowed in.tsx/.jsxfiles.apiis referenced in the JSX on line 50 but is never defined or imported.QueryClientProvideris used on line 51 but never imported (onlyQueryClientis imported on line 4).createTRPCClient<AppRouter>({})on line 47 is called with an empty config; TRPC v11 requires at least one link (e.g.,httpBatchLink) and typically a transformer likesuperjson.getBaseUrl,httpBatchLink, andsuperjsonare all imported but unused, indicating the client wiring is incomplete.To fix this, switch to
createTRPCReact(already available in package.json as@trpc/react-query@^11.7.2) and properly configure the links and transformer:+"use client"; + +import { AppRouter } from "@/server/routers/_app"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { httpBatchLink } from "@trpc/client"; +import { createTRPCReact } from "@trpc/react-query"; +import { useState } from "react"; +import superjson from "superjson"; + +export const api = createTRPCReact<AppRouter>(); export function TRPCProvider({ children }: { children: React.ReactNode }) { const queryClient = getQueryClient(); const [trpcClient] = useState(() => - createTRPCClient<AppRouter>({}), + api.createClient({ + transformer: superjson, + links: [ + httpBatchLink({ + url: `${getBaseUrl()}/api/trpc`, + }), + ], + }), ); + return ( - <api.Provider client= { trpcClient } queryClient = { queryClient } > - <QueryClientProvider client={ queryClient }> { children } </QueryClientProvider> - </api.Provider> + <QueryClientProvider client={queryClient}> + <api.Provider client={trpcClient} queryClient={queryClient}> + {children} + </api.Provider> + </QueryClientProvider> ); }Also rename this file to
client.tsxso JSX parses cleanly.
🧹 Nitpick comments (2)
src/lib/auth.ts (1)
13-19: Cookie/session options look good; confirm they don’t break HTTP dev
cookiePrefixand enablingsession.cookieCachewithstrategy: "jwt"make sense. Withadvanced.useSecureCookies: true, cookies won’t be sent over plain HTTP, so if you ever run the app overhttp://localhost(not behind HTTPS/CF), auth may silently fail. Consider gating this on environment if you expect non-HTTPS local runs, or confirm everything is always served over HTTPS.package.json (1)
18-92: Align scripts/deps with actual tooling and DB filesA couple of things to double‑check around these script and dependency changes:
db:seed/db:resetstill point atsrc/server/db/seed.tsandreset.ts. If those files were removed in this PR, these scripts will now just error; either restore the scripts or delete/repurpose these commands.- You’ve added
prepare: "husky install"and still depend onhusky/lint-staged. If you intended to drop Git hooks from this repo, consider removing thepreparescript and those devDeps; otherwise, make sure there’s a.huskydirectory with actual hooks sopreparedoes something useful.- With all the major version bumps (Next 15, React 19, TRPC 11, TanStack Query 5, ESLint 9, TypeScript 5.9, Wrangler 3.x, etc.), it’s worth re-running
bun run type-check,bun run lint, and your test suite to catch any incompatibilities. Also make sure your@types/react*versions are compatible with the chosen React version.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (15)
.husky/commit-msg(0 hunks).husky/pre-commit(0 hunks)INSTALLATION.md(0 hunks)PROJECT_SUMMARY.md(0 hunks)QUICKSTART.md(0 hunks)cloudflare-env.d.ts(1 hunks)commitlint.config.js(1 hunks)package.json(1 hunks)src/env.ts(0 hunks)src/lib/auth.ts(1 hunks)src/proxy.ts(1 hunks)src/server/db/reset.ts(0 hunks)src/server/db/seed.ts(0 hunks)src/trpc/client.ts(2 hunks)wrangler.jsonc(1 hunks)
💤 Files with no reviewable changes (8)
- .husky/pre-commit
- src/env.ts
- INSTALLATION.md
- src/server/db/reset.ts
- QUICKSTART.md
- src/server/db/seed.ts
- .husky/commit-msg
- PROJECT_SUMMARY.md
🧰 Additional context used
🧬 Code graph analysis (1)
src/trpc/client.ts (1)
src/server/routers/_app.ts (1)
AppRouter(8-8)
🪛 Biome (2.1.2)
src/trpc/client.ts
[error] 50-50: expected > but instead found client
Remove client
(parse)
[error] 50-50: Invalid assignment to <api.Provider client
This expression cannot be assigned to
(parse)
[error] 50-50: expected ) but instead found queryClient
Remove queryClient
(parse)
[error] 51-51: expected > but instead found client
Remove client
(parse)
[error] 50-51: Invalid assignment to { queryClient } > <QueryClientProvider client
This expression cannot be assigned to
(parse)
[error] 51-51: unterminated regex literal
...but the line ends here
a regex literal starts there...
(parse)
[error] 52-52: unterminated regex literal
...but the line ends here
a regex literal starts there...
(parse)
[error] 50-50: Can't assign queryClient because it's a constant.
This is where the variable is defined as constant.
Unsafe fix: Replace const with let if you assign it to a new value.
(lint/correctness/noConstAssign)
🔇 Additional comments (5)
commitlint.config.js (1)
2-22: Commitlint rules relaxed; only subject/body checks now disabled
- Line 2 and the
type-enumblock (Lines 4-20) only change quote style; behavior stays the same and looks fine.- Lines 21-22 set
subject-caseandbody-max-line-lengthto[0], which fully disables those checks. That means commit subjects can use any casing and body lines can be arbitrarily long.If the goal is to reduce friction around commit formatting, this config is consistent with that. If you still want some guardrails, you might instead use non‑zero levels (e.g., warnings) and keep a max body length.
src/proxy.ts (2)
17-19: Remove or correct the middleware matcher—the protected/dashboardroute does not exist in the application.The matcher currently protects a non-existent
/dashboardroute, making the middleware ineffective. The actual application routes are/(home),/api/auth/[...all], and/api/trpc/[trpc]. Additionally, the proxy function itself includes a comment warning "THIS IS NOT SECURE!" and recommends handling auth checks in each page/route rather than via middleware. Either remove the middleware configuration if auth is handled per-route, or update the matcher to protect the routes that actually require authentication (e.g.,/api/trpcor/).Likely an incorrect or invalid review comment.
4-15: Clarify the middleware comment—the cookie presence check pattern is correct.The proxy middleware is correctly implemented according to better-auth recommendations. Middleware should only check for cookie presence using
getSessionCookie()for optimistic routing; authoritative session validation happens server-side (as evidenced by yourprotectedProcedurein tRPC that checksctx.session?.userId).The comment "THIS IS NOT SECURE!" is misleading. The middleware pattern is secure when paired with server-side validation—which you already have. Consider updating the comment to clarify that this middleware provides UX-level routing only and that actual authorization checks occur in server components and procedures.
Do not replace
getSessionCookie()withauth.api.getSession()in middleware; that would cause Edge runtime errors since full session validation requires database/server calls.Likely an incorrect or invalid review comment.
wrangler.jsonc (1)
8-35: Wrangler bindings + vars are consistent with CloudflareEnvThe added
R2_PUBLIC_URLand updated commas look fine, and the bindings (DB,R2_STORAGE,CACHE_KV,ASSETS) align with the newCloudflareEnvinterface. Just keep this file andcloudflare-env.d.tsin sync whenever you add/change bindings.cloudflare-env.d.ts (1)
1-10: Generated CloudflareEnv matches Wrangler configThe generated
CloudflareEnvinterface lines up with the bindings and vars inwrangler.jsonc(CACHE_KV,R2_STORAGE,DB,ASSETS,ENVIRONMENT,R2_PUBLIC_URL). This looks good as a type source of truth—just avoid hand-editing and rely on thecf-typegen/wrangler typescommand when config changes.
Summary by CodeRabbit
New Features
Chores
Refactor
Documentation
Style/Config
✏️ Tip: You can customize this high-level summary in your review settings.