-
Notifications
You must be signed in to change notification settings - Fork 619
Add Google Click ID tracking to Stripe checkout #8180
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
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
WalkthroughAdds gclid capture and propagation: middleware parses gclid from query and sets a cookie with options; Stripe action reads gclid from cookies and conditionally includes it as metadata when creating a Checkout Session. Middleware refactors cookie-setting to support value+options tuples and updates related function signatures and calls. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant MW as Next.js Middleware
participant B as Browser
participant SA as Stripe Action
U->>MW: HTTP request with ?gclid=...
MW->>B: Response with Set-Cookie gclid=... (7d, options)
Note over MW,B: Cookie options: httpOnly=false, sameSite=lax, secure=true
U->>SA: Initiate checkout (request includes cookies)
SA->>SA: Read gclid via cookies()
alt gclid present
SA->>Stripe: Create Checkout Session (metadata.gclid)
else gclid absent
SA->>Stripe: Create Checkout Session (no gclid metadata)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8180 +/- ##
=======================================
Coverage 56.07% 56.07%
=======================================
Files 905 905
Lines 59228 59228
Branches 4124 4124
=======================================
Hits 33212 33212
Misses 25912 25912
Partials 104 104
🚀 New features to boost your workflow:
|
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/dashboard/src/middleware.ts (3)
46-76: Remove redundant null check.
cookiesToSetis initialized at line 46, so the check at lines 69-71 will always be false and can be removed.Apply this diff:
// if its already set - don't set it again if (!request.cookies.get(key)) { - if (!cookiesToSet) { - cookiesToSet = {}; - } - cookiesToSet[key] = [value]; }
175-182: Add type safety for optional cookie options.The code assumes
entry[1][1](cookie options) is always present, butCookiesToSetallows tuples with just[value]. This could passundefinedas the third argument tocookies.set().Apply this diff to handle both tuple formats safely:
if (cookiesToSet) { const defaultResponse = NextResponse.next(); for (const entry of Object.entries(cookiesToSet)) { - defaultResponse.cookies.set(entry[0], entry[1][0], entry[1][1]); + const [value, options] = entry[1]; + if (options) { + defaultResponse.cookies.set(entry[0], value, options); + } else { + defaultResponse.cookies.set(entry[0], value); + } } return defaultResponse; }
191-233: Apply the same type safety fix in helper functions.The same issue with optional cookie options exists in both
rewrite()(line 202) andredirect()(line 228). Apply the same pattern as suggested for the main flow.Apply this diff:
function rewrite( request: NextRequest, relativePath: string, cookiesToSet: CookiesToSet, ) { const url = request.nextUrl.clone(); url.pathname = relativePath; const res = NextResponse.rewrite(url); if (cookiesToSet) { for (const entry of Object.entries(cookiesToSet)) { - res.cookies.set(entry[0], entry[1][0], entry[1][1]); + const [value, options] = entry[1]; + if (options) { + res.cookies.set(entry[0], value, options); + } else { + res.cookies.set(entry[0], value); + } } } return res; } function redirect( request: NextRequest, relativePath: string, options: | { searchParams?: string; permanent?: boolean; cookiesToSet?: CookiesToSet; } | undefined, ) { const permanent = options?.permanent ?? false; const url = request.nextUrl.clone(); url.pathname = relativePath; url.search = options?.searchParams ? `?${options.searchParams}` : ""; const res = NextResponse.redirect(url, permanent ? 308 : undefined); if (options?.cookiesToSet) { for (const entry of Object.entries(options.cookiesToSet)) { - res.cookies.set(entry[0], entry[1][0], entry[1][1]); + const [value, options] = entry[1]; + if (options) { + res.cookies.set(entry[0], value, options); + } else { + res.cookies.set(entry[0], value); + } } } return res; }
🧹 Nitpick comments (1)
apps/dashboard/src/middleware.ts (1)
10-22: Consider aligningCookiesToSetoptions with Next.js cookie API.The current type definition only includes
httpOnly,sameSite,secure, andmaxAge, but Next.jscookies().set()supports additional options likepath,domain, andexpires. This could limit flexibility if you need to set cookies with specific paths or domains in the future.Consider expanding the type to include all standard cookie options:
type CookiesToSet = Record< string, | [string] | [ string, { httpOnly: boolean; sameSite?: "lax" | "strict" | "none"; secure: boolean; maxAge: number; + path?: string; + domain?: string; + expires?: Date; }, ] >;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
apps/dashboard/src/@/actions/stripe-actions.ts(3 hunks)apps/dashboard/src/middleware.ts(7 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Write idiomatic TypeScript with explicit function declarations and return types
Limit each file to one stateless, single-responsibility function for clarity
Re-use shared types from@/typesor localtypes.tsbarrels
Prefer type aliases over interface except for nominal shapes
Avoidanyandunknownunless unavoidable; narrow generics when possible
Choose composition over inheritance; leverage utility types (Partial,Pick, etc.)
Comment only ambiguous logic; avoid restating TypeScript in prose
**/*.{ts,tsx}: Use explicit function declarations and explicit return types in TypeScript
Limit each file to one stateless, single‑responsibility function
Re‑use shared types from@/typeswhere applicable
Prefertypealiases overinterfaceexcept for nominal shapes
Avoidanyandunknownunless unavoidable; narrow generics when possible
Prefer composition over inheritance; use utility types (Partial, Pick, etc.)
Lazy‑import optional features and avoid top‑level side‑effects to reduce bundle size
Files:
apps/dashboard/src/middleware.tsapps/dashboard/src/@/actions/stripe-actions.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Load heavy dependencies inside async paths to keep initial bundle lean (lazy loading)
Files:
apps/dashboard/src/middleware.tsapps/dashboard/src/@/actions/stripe-actions.ts
apps/{dashboard,playground-web}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/{dashboard,playground-web}/**/*.{ts,tsx}: Import UI primitives from@/components/ui/*(Button, Input, Select, Tabs, Card, Sidebar, Badge, Separator) in dashboard and playground apps
UseNavLinkfor internal navigation with automatic active states in dashboard and playground apps
Use Tailwind CSS only – no inline styles or CSS modules
Usecn()from@/lib/utilsfor conditional class logic
Use design system tokens (e.g.,bg-card,border-border,text-muted-foreground)
Server Components (Node edge): Start files withimport "server-only";
Client Components (browser): Begin files with'use client';
Always callgetAuthToken()to retrieve JWT from cookies on server side
UseAuthorization: Bearerheader – never embed tokens in URLs
Return typed results (e.g.,Project[],User[]) – avoidany
Wrap client-side data fetching calls in React Query (@tanstack/react-query)
Use descriptive, stablequeryKeysfor React Query cache hits
ConfigurestaleTime/cacheTimein React Query based on freshness (default ≥ 60s)
Keep tokens secret via internal API routes or server actions
Never importposthog-jsin server components
Files:
apps/dashboard/src/middleware.tsapps/dashboard/src/@/actions/stripe-actions.ts
apps/{dashboard,playground}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/{dashboard,playground}/**/*.{ts,tsx}: Import UI primitives from@/components/ui/_(e.g., Button, Input, Tabs, Card)
UseNavLinkfor internal navigation to get active state handling
Use Tailwind CSS for styling; no inline styles
Merge class names withcn()from@/lib/utilsfor conditional classes
Stick to design tokens (e.g., bg-card, border-border, text-muted-foreground)
Server Components must start withimport "server-only"; usenext/headers, server‑only env, heavy data fetching, andredirect()where appropriate
Client Components must start with'use client'; handle interactivity with hooks and browser APIs
Server-side data fetching: callgetAuthToken()from cookies, sendAuthorization: Bearer <token>header, and return typed results (avoidany)
Client-side data fetching: wrap calls in React Query with descriptive, stablequeryKeysand set sensiblestaleTime/cacheTime(≥ 60s default); keep tokens secret via internal routes or server actions
Do not importposthog-jsin server components (client-side only)
Files:
apps/dashboard/src/middleware.tsapps/dashboard/src/@/actions/stripe-actions.ts
🧠 Learnings (1)
📚 Learning: 2025-08-29T15:37:38.513Z
Learnt from: CR
PR: thirdweb-dev/js#0
File: AGENTS.md:0-0
Timestamp: 2025-08-29T15:37:38.513Z
Learning: Applies to apps/{dashboard,playground}/**/*.{ts,tsx} : Server Components must start with `import "server-only"`; use `next/headers`, server‑only env, heavy data fetching, and `redirect()` where appropriate
Applied to files:
apps/dashboard/src/@/actions/stripe-actions.ts
⏰ 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). (5)
- GitHub Check: E2E Tests (pnpm, vite)
- GitHub Check: Size
- GitHub Check: Unit Tests
- GitHub Check: Lint Packages
- GitHub Check: Analyze (javascript)
🔇 Additional comments (4)
apps/dashboard/src/middleware.ts (1)
41-41: LGTM!Destructuring
searchParamsupfront improves readability and reduces repetitive property access.apps/dashboard/src/@/actions/stripe-actions.ts (3)
3-3: LGTM!The addition of
cookiesimport fromnext/headersis appropriate for this server-side action and aligns with the coding guidelines for server components.Based on learnings.
91-92: LGTM!The gclid retrieval is implemented correctly with proper async handling and optional chaining to handle missing cookies gracefully.
123-124: LGTM! Verify metadata placement aligns with attribution requirements.The conditional metadata attachment is implemented correctly. The
gclidis being set onsubscription_data.metadata, which will attach it to the Stripe subscription object for long-term attribution tracking.If you need to track the gclid on the checkout session itself (for immediate conversion tracking), you might also want to add it at the session level:
const session = await stripe.checkout.sessions.create({ // ... existing config metadata: gclid ? { gclid } : undefined, subscription_data: { // ... existing config metadata: gclid ? { gclid } : undefined, }, });However, this is only necessary if your attribution logic needs session-level metadata. The current implementation on
subscription_datais sufficient for tracking which ad led to the subscription.
size-limit report 📦
|

TL;DR
Added Google Click ID (gclid) tracking to Stripe checkout sessions for conversion attribution.
What changed?
gclidfrom URL search parameters and store it in a cookie with a 7-day expirationgclidcookie value and include it in the Stripe checkout session metadatamaxAge,httpOnly, etc.How to test?
?gclid=test123)Why make this change?
This change enables proper attribution of conversions to Google Ads campaigns. By tracking the gclid through the user journey to payment, we can accurately measure which ad campaigns are driving successful conversions, improving our marketing analytics and ROI measurement.
Summary by CodeRabbit
New Features
Refactor