Conversation
…tip text accordingly
fix: og image rendering
fix: cache, oh my god
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
zero-staging | 276e4ac | May 22 2025, 08:42 PM |
WalkthroughThis update introduces several refactors and feature adjustments. It streamlines font loading in Open Graph API endpoints, updates the cache burst key, and removes the Vercel deploy badge from the README. Query client management is decoupled from session state, relying on Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Loader (root.tsx)
participant tRPC Server
participant ServerProviders
participant QueryProvider
Client->>Loader (root.tsx): Request page
Loader (root.tsx)->>tRPC Server: trpc.connections.getDefault.query()
tRPC Server-->>Loader (root.tsx): { id: connectionId }
Loader (root.tsx)-->>Client: { connectionId }
Client->>ServerProviders: Render with connectionId
ServerProviders->>QueryProvider: Pass connectionId prop
QueryProvider->>QueryProvider: Create/get QueryClient for connectionId
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
apps/mail/providers/query-provider.tsx (1)
82-99: 🛠️ Refactor suggestion
⚠️ Potential issueGlobal
trpcClientreferenceswindow, breaking SSR
createTRPCClientis instantiated at module scope, and its
fetchoverride unconditionally accesseswindow.location. When this module
is imported during server-side rendering (wherewindowis undefined) the very
first query will throw, crashing the request.A defensive guard avoids the issue:
- fetch: (url, options) => - fetch(url, { ...options, credentials: 'include' }).then((res) => { - const currentPath = new URL(window.location.href).pathname; - const redirectPath = res.headers.get('X-Zero-Redirect'); - if (!!redirectPath && redirectPath !== currentPath) window.location.href = redirectPath; - return res; - }), + fetch: (url, options) => + fetch(url, { ...options, credentials: 'include' }).then((res) => { + if (typeof window !== 'undefined') { + const currentPath = new URL(window.location.href).pathname; + const redirectPath = res.headers.get('X-Zero-Redirect'); + if (redirectPath && redirectPath !== currentPath) window.location.href = redirectPath; + } + return res; + }),Alternatively, instantiate the client lazily in
useEffecton the client only.
🧹 Nitpick comments (2)
apps/mail/providers/query-provider.tsx (2)
62-78: SingletonbrowserQueryClientleaks when multiple connections are opened
browserQueryClientstores a singleQueryClient&activeConnectionId.
If a user switches between two connections (A → B → A), the second switch will
reuse the client built for B while the hash function still prefixes keys with
connection A, leading to cache pollution.Consider caching per connection id instead of mutating a singleton:
-let browserQueryClient = { - queryClient: null, - activeConnectionId: null, -} as { queryClient: QueryClient | null; activeConnectionId: string | null }; +const browserQueryClients = new Map<string | null, QueryClient>();…and update
getQueryClientaccordingly.This guarantees isolated caches and avoids subtle cross-connection data
bleeding.
125-142:threadQueryKeymissesconnectionIdcomponent used in custom hashBecause
queryKeyHashFnprepends{connectionId}before hashing, look-ups
insideonSuccessshould mirror that logic; otherwise the sliced data might not
match the intended queries when multiple connections are active.-const threadQueryKey = [['mail', 'listThreads'], { type: 'infinite' }]; +const threadQueryKey = [['mail', 'listThreads'], { type: 'infinite' }]; // (then pass `queryKey` plus meta so the hash function prefixes connectionId)Better: extract a small helper
buildQueryKey(['mail','listThreads'])that
always applies the same hashing rule to avoid divergence.
📜 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 (12)
README.md(0 hunks)apps/mail/app/og-api/create.tsx(2 hunks)apps/mail/app/og-api/home.tsx(1 hunks)apps/mail/app/root.tsx(3 hunks)apps/mail/components/ui/ai-sidebar.tsx(1 hunks)apps/mail/components/ui/nav-user.tsx(3 hunks)apps/mail/components/ui/switch.tsx(2 hunks)apps/mail/lib/constants.tsx(1 hunks)apps/mail/lib/trpc.server.ts(1 hunks)apps/mail/package.json(0 hunks)apps/mail/providers/query-provider.tsx(5 hunks)apps/mail/providers/server-providers.tsx(1 hunks)
💤 Files with no reviewable changes (2)
- apps/mail/package.json
- README.md
🧰 Additional context used
🧬 Code Graph Analysis (7)
apps/mail/components/ui/ai-sidebar.tsx (1)
apps/mail/components/ui/gauge.tsx (1)
Gauge(1-84)
apps/mail/components/ui/switch.tsx (1)
apps/mail/lib/utils.ts (1)
cn(53-53)
apps/mail/providers/server-providers.tsx (1)
apps/mail/providers/query-provider.tsx (1)
QueryProvider(107-149)
apps/mail/app/og-api/home.tsx (1)
apps/mail/app/og-api/create.tsx (1)
loader(4-97)
apps/mail/app/root.tsx (2)
apps/mail/lib/trpc.server.ts (1)
getServerTrpc(8-17)apps/mail/providers/server-providers.tsx (1)
ServerProviders(7-20)
apps/mail/lib/trpc.server.ts (1)
apps/server/src/trpc/index.ts (1)
AppRouter(31-31)
apps/mail/providers/query-provider.tsx (1)
apps/mail/lib/constants.tsx (1)
CACHE_BURST_KEY(13-13)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Workers Builds: zero-staging
- GitHub Check: Workers Builds: zero-server-staging
🔇 Additional comments (13)
apps/mail/components/ui/switch.tsx (3)
3-3: Package import updated to use consolidated Radix UI package.The import statement has been updated to use the consolidated
radix-uipackage instead of the individual@radix-ui/react-switchpackage. This aligns with the dependency changes in the project and helps standardize the component imports.
14-14: LGTM: CSS class ordering adjustment.The reordering of Tailwind CSS classes with
data-[state=checked]:bg-[#0B84FE]moved to the end maintains the same visual appearance while potentially improving readability.
22-22: LGTM: Improved organization of Tailwind classes.The class string has been reorganized with better grouping - positioning classes are now together, transform classes are grouped, and theme-specific class (
dark:bg-white) is moved to the end. This improves code readability while maintaining the same functionality.apps/mail/app/og-api/create.tsx (2)
1-1: Good refactoring of font loading functionalityThe import now properly leverages the built-in
loadGoogleFontfunction from theworkers-ogpackage, eliminating the need for a custom implementation.
20-21: Improved font loading with standardized parameter objectThe updated call signature using an object with
familyandweightproperties follows better practices than separate string arguments. This change aligns with the implementation in thehome.tsxfile, ensuring consistency across OG API endpoints.apps/mail/app/og-api/home.tsx (3)
1-2: Improved imports with direct asset referenceGood refactoring to:
- Import the standard
loadGoogleFontutility fromworkers-og- Use modern asset imports with the
?urlsuffix for the SVG iconThis eliminates runtime fetching and base64 encoding, which is more efficient.
6-7: Standardized font loading implementationThe updated call signature using an object with
familyandweightproperties matches the implementation increate.tsx, ensuring consistency across OG API endpoints.
14-14: Simplified asset usageUsing the imported SVG URL directly eliminates the need for runtime fetching and base64 encoding, improving performance and reducing potential network failures.
apps/mail/lib/constants.tsx (1)
13-13: Cache burst key incremented to invalidate stale caches.The version update to v0.0.3 will trigger client cache invalidation, ensuring users get fresh data following the query client refactoring changes.
apps/mail/components/ui/nav-user.tsx (3)
23-23: Consolidated imports from 'react-router'.The import statement has been cleaned up to group related imports from the same package.
68-68: Using useRevalidator instead of useThreads.The component now uses the more appropriate
useRevalidatorhook to access the revalidation function, which aligns with the broader refactoring of connection and session management.
102-103: Added data revalidation step after account switching.The added
revalidate()call ensures that route or query data is refreshed after switching accounts, maintaining data consistency with the new connection context. This complements the cache invalidation strategies implemented elsewhere.apps/mail/providers/server-providers.tsx (1)
11-12: Decoupled query provider from session state.The ServerProviders component now accepts a
connectionIdprop and passes it directly to QueryProvider. This architectural improvement:
- Makes the data flow more explicit
- Reduces unnecessary recomputation
- Aligns with the refactoring in query-provider.tsx to rely solely on connectionId for cache keys
This change is part of a broader effort to simplify cache management and query client behavior across the application.
Also applies to: 16-16
| <TooltipTrigger asChild className="md:h-fit md:px-2"> | ||
| <div> | ||
| <Gauge value={50 - chatMessages.remaining!} size="small" showValue={true} /> | ||
| <Gauge value={20 - chatMessages.remaining!} size="small" showValue={true} /> |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Reduced usage limit for non-pro users.
The maximum allowed chat messages for non-pro users has been reduced from 50 to 20. This change affects both the gauge calculation and the tooltip text.
Note that while this change correctly updates the values in the sidebar, the Gauge component in apps/mail/components/ui/gauge.tsx uses a hardcoded value of 50 for circumference calculations. Consider checking if the gauge visualization will still appear correctly with this new value range:
Also applies to: 157-157
🏁 Script executed:
#!/bin/bash
# Check if Gauge component calculations need updating
cat apps/mail/components/ui/gauge.tsx | grep -n 'value / 50'Length of output: 126
Gauge max value is still hardcoded; make it dynamic
The Gauge component in apps/mail/components/ui/gauge.tsx still uses a hardcoded 50 for its circumference calculation even though non-pro usage has dropped to 20. We should:
• apps/mail/components/ui/gauge.tsx
– Change
js const valueInCircumference = (value / 50) * circumference;
to
js const valueInCircumference = (value / max) * circumference;
– Add a max prop (defaulting to 50) in the component signature and PropTypes/TS defs.
• apps/mail/components/ui/ai-sidebar.tsx (lines 153 & 157)
– Pass the new max prop to Gauge, e.g.
tsx <Gauge value={20 - chatMessages.remaining!} max={20} size="small" showValue />
This ensures the gauge scales correctly for the new 20-message limit while preserving the old 50-message default elsewhere.
🤖 Prompt for AI Agents
In apps/mail/components/ui/gauge.tsx around the lines where circumference is
calculated, replace the hardcoded divisor 50 with a dynamic max prop by adding a
max prop to the component signature with a default value of 50 and updating the
calculation to use (value / max) * circumference. Then in
apps/mail/components/ui/ai-sidebar.tsx at lines 153 and 157, update the Gauge
component usage to pass max={20} along with the value prop to reflect the new
20-message limit, ensuring the gauge visualization scales correctly.
| const trpc = getServerTrpc(request); | ||
|
|
||
| const connectionId = await trpc.connections.getDefault | ||
| .query() | ||
| .then((res) => res?.id) | ||
| .catch(() => null); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Always normalise connectionId to the declared string | null type
res?.id returns undefined when the server responds with no default connection.
Because the loader’s return type specifies connectionId: string | null, returning
undefined breaks the contract and will surface as a type-error or, worse, a
runtime “cannot read … of undefined” further down the tree.
- const connectionId = await trpc.connections.getDefault
- .query()
- .then((res) => res?.id)
- .catch(() => null);
+ const connectionId = await trpc.connections.getDefault
+ .query()
+ .then((res) => res?.id ?? null)
+ .catch(() => null);This tiny change guarantees the value is either a string or explicit null.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const trpc = getServerTrpc(request); | |
| const connectionId = await trpc.connections.getDefault | |
| .query() | |
| .then((res) => res?.id) | |
| .catch(() => null); | |
| const trpc = getServerTrpc(request); | |
| const connectionId = await trpc.connections.getDefault | |
| .query() | |
| .then((res) => res?.id ?? null) | |
| .catch(() => null); |
🤖 Prompt for AI Agents
In apps/mail/app/root.tsx around lines 41 to 47, the connectionId is assigned
using res?.id which can be undefined if no default connection exists, but the
declared type expects string or null. To fix this, explicitly normalize the
connectionId by returning res?.id if it exists, otherwise null, ensuring the
value always matches the string | null type contract and prevents type errors or
runtime issues.
| import { createTRPCClient, httpBatchLink } from '@trpc/client'; | ||
| import type { AppRouter } from '@zero/server/trpc'; | ||
| import { env } from 'cloudflare:workers'; | ||
| import superjson from 'superjson'; | ||
|
|
||
| const getUrl = () => env.VITE_PUBLIC_BACKEND_URL + '/api/trpc'; | ||
|
|
||
| export const getServerTrpc = (req: Request) => | ||
| createTRPCClient<AppRouter>({ | ||
| links: [ | ||
| httpBatchLink({ | ||
| url: getUrl(), | ||
| transformer: superjson, | ||
| headers: req.headers, | ||
| }), | ||
| ], | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Mis-placed transformer & fragile header forwarding
-
In tRPC v10 the
transformermust be passed tocreateTRPCClient, not to
httpBatchLink. At runtime the current code silently ignores the
superjson transformer and falls back toJSON.stringify/JSON.parse. -
req.headersis aHeadersinstance;httpBatchLinkexpects
HeadersInit | () => HeadersInit. While most runtimes accept the native
Headersobject, Node’s polyfill does not ‑ converting to a plain record
is safer. -
env.VITE_PUBLIC_BACKEND_URLis assumed to be set. If it is missing the
URL becomes “undefined/api/trpc” – extremely hard to trace in production.
-const getUrl = () => env.VITE_PUBLIC_BACKEND_URL + '/api/trpc';
+const getUrl = () => {
+ if (!env.VITE_PUBLIC_BACKEND_URL)
+ throw new Error('VITE_PUBLIC_BACKEND_URL is not defined in the worker env');
+ return `${env.VITE_PUBLIC_BACKEND_URL}/api/trpc`;
+};
export const getServerTrpc = (req: Request) =>
- createTRPCClient<AppRouter>({
- links: [
- httpBatchLink({
- url: getUrl(),
- transformer: superjson,
- headers: req.headers,
- }),
- ],
- });
+ createTRPCClient<AppRouter>({
+ transformer: superjson,
+ links: [
+ httpBatchLink({
+ url: getUrl(),
+ headers: () => Object.fromEntries(req.headers),
+ }),
+ ],
+ });These tweaks restore superjson serialisation, harden header handling, and fail
fast when the backend URL is missing.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { createTRPCClient, httpBatchLink } from '@trpc/client'; | |
| import type { AppRouter } from '@zero/server/trpc'; | |
| import { env } from 'cloudflare:workers'; | |
| import superjson from 'superjson'; | |
| const getUrl = () => env.VITE_PUBLIC_BACKEND_URL + '/api/trpc'; | |
| export const getServerTrpc = (req: Request) => | |
| createTRPCClient<AppRouter>({ | |
| links: [ | |
| httpBatchLink({ | |
| url: getUrl(), | |
| transformer: superjson, | |
| headers: req.headers, | |
| }), | |
| ], | |
| }); | |
| import { createTRPCClient, httpBatchLink } from '@trpc/client'; | |
| import type { AppRouter } from '@zero/server/trpc'; | |
| import { env } from 'cloudflare:workers'; | |
| import superjson from 'superjson'; | |
| const getUrl = () => { | |
| if (!env.VITE_PUBLIC_BACKEND_URL) | |
| throw new Error('VITE_PUBLIC_BACKEND_URL is not defined in the worker env'); | |
| return `${env.VITE_PUBLIC_BACKEND_URL}/api/trpc`; | |
| }; | |
| export const getServerTrpc = (req: Request) => | |
| createTRPCClient<AppRouter>({ | |
| transformer: superjson, | |
| links: [ | |
| httpBatchLink({ | |
| url: getUrl(), | |
| headers: () => Object.fromEntries(req.headers), | |
| }), | |
| ], | |
| }); |
🤖 Prompt for AI Agents
In apps/mail/lib/trpc.server.ts lines 1 to 17, move the superjson transformer
from the httpBatchLink options to the createTRPCClient options to ensure it is
applied correctly. Convert req.headers from a Headers instance to a plain object
before passing it to httpBatchLink to avoid compatibility issues with Node
polyfills. Add a check to verify that env.VITE_PUBLIC_BACKEND_URL is defined and
throw an error or fail fast if it is missing to prevent constructing an invalid
URL.
2 participants
Summary by CodeRabbit
Bug Fixes
New Features
Refactor
Chores