-
Notifications
You must be signed in to change notification settings - Fork 38
Reorganize third-party integrations #234
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
WalkthroughAdded Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
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.
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 (7)
conversions/sales/segment.mdx (1)
106-126
: Fix undefined vars and missing import in the Segment Node example.
id
is undefined;cookies()
is used without import and top-level await. Wrap in an async fn, importcookies
, and pass a knownuser
object.Apply:
- ```tsx - import { Analytics } from "@segment/analytics-node"; - - const segment = new Analytics({ - writeKey: "<YOUR_SEGMENT_WRITE_KEY>", - }); - - segment.track({ - userId: id, - event: "Order Completed", - properties: { - payment_processor: "stripe", - order_id: "ORD_123", - currency: "USD", - revenue: 1000, - }, - integrations: { - All: true, - }, - }); - ``` + ```tsx + import { Analytics } from "@segment/analytics-node"; + import { cookies } from "next/headers"; + + const segment = new Analytics({ writeKey: "<YOUR_SEGMENT_WRITE_KEY>" }); + + export async function trackOrderCompleted(user: { id: string }) { + const cookieStore = await cookies(); + // optional: read click id if you also map it in Actions + const clickId = cookieStore.get("dub_id")?.value; + await segment.track({ + userId: user.id, + event: "Order Completed", + properties: { + payment_processor: "stripe", + order_id: "ORD_123", + currency: "USD", + revenue: 1000, + clickId, + }, + integrations: { All: true }, + }); + } + ``` </blockquote></details> <details> <summary>conversions/leads/clerk.mdx (1)</summary><blockquote> `195-227`: **API route example: missing imports and undefined identifiers.** `dub`, `clerkClient`, and body fields (`id`, `name`, `email`, `avatar`) are not defined/imported. ```diff -```tsx /api/track-lead/route.ts +```tsx /api/track-lead/route.ts // This is an API route import { NextRequest, NextResponse } from "next/server"; +import { dub } from "@/lib/dub"; +import { clerkClient } from "@clerk/nextjs/server"; export async function POST(req: NextRequest) { // read dub_id from the request cookies const dubId = req.cookies.get("dub_id")?.value; + const { id, name, email, avatar } = await req.json(); if (dubId) { // Send lead event to Dub await dub.track.lead({ clickId: dubId, eventName: "Sign Up", customerExternalId: id, customerName: name, customerEmail: email, customerAvatar: avatar, }); } const clerk = await clerkClient(); await clerk.users.updateUser(id, { publicMetadata: { dubClickId: dubId || "n/a", }, }); const res = NextResponse.json({ ok: true }); // Delete the dub_id cookie res.cookies.set("dub_id", "", { expires: new Date(0), }); return res; }conversions/leads/segment.mdx (1)
106-131
: Make the example complete: import cookies and define inputs.
cookies()
isn’t imported;id
,name
,avatar
are undefined; top‑levelawait
shown without an async context.- ```tsx - import { Analytics } from "@segment/analytics-node"; - - const segment = new Analytics({ - writeKey: "<YOUR_SEGMENT_WRITE_KEY>", - }); - - const cookieStore = await cookies(); - const clickId = cookieStore.get("dub_id")?.value; - - segment.track({ - userId: id, - event: "Sign Up", - context: { - traits: { - name, - email, - avatar, - clickId, - }, - }, - integrations: { - All: true, - }, - }); - ``` + ```tsx + import { Analytics } from "@segment/analytics-node"; + import { cookies } from "next/headers"; + + const segment = new Analytics({ writeKey: "<YOUR_SEGMENT_WRITE_KEY>" }); + + export async function trackSignUp(user: { + id: string; + name?: string | null; + email?: string | null; + avatar?: string | null; + }) { + const clickId = (await cookies()).get("dub_id")?.value; + await segment.track({ + userId: user.id, + event: "Sign Up", + context: { + traits: { name: user.name, email: user.email, avatar: user.avatar, clickId }, + }, + integrations: { All: true }, + }); + } + ``` </blockquote></details> <details> <summary>conversions/leads/next-auth.mdx (2)</summary><blockquote> `33-66`: **Use `message.user` and persist cookie deletion correctly.** `user` is undefined; reference `message.user`. Also, deleting cookies via `cookies()` inside options may not persist; prefer deleting in the route handler response. ```diff - async signIn(message) { + async signIn(message) { // if it's a new sign up if (message.isNewUser) { const cookieStore = await cookies(); // check if dub_id cookie is present const dub_id = cookieStore.get("dub_id")?.value; if (dub_id) { // send lead event to Dub await dub.track.lead({ clickId: dub_id, eventName: "Sign Up", - customerExternalId: user.id, - customerName: user.name, - customerEmail: user.email, - customerAvatar: user.image, + customerExternalId: message.user.id, + customerName: message.user.name ?? undefined, + customerEmail: message.user.email ?? undefined, + customerAvatar: (message.user as any).image ?? undefined, }); - // delete the cookies - cookieStore.delete("dub_id"); - cookieStore.delete("dub_partner_data"); + // Tip: delete cookies in your route handler using NextResponse cookies API } } },Optionally add to the App Router route handler (index.ts):
+import { cookies } from "next/headers"; +// after successful auth: +cookies().delete("dub_id"); +cookies().delete("dub_partner_data");
68-97
: Pages Router example: referencemessage.user
.Same undefined
user
issue here.- customerExternalId: user.id, - customerName: user.name, - customerEmail: user.email, - customerAvatar: user.image, + customerExternalId: message.user.id, + customerName: message.user.name ?? undefined, + customerEmail: message.user.email ?? undefined, + customerAvatar: (message.user as any).image ?? undefined,conversions/leads/appwrite.mdx (1)
76-97
: Place DubAnalytics inside the body.
<DubAnalytics />
is outside<body>
, which is invalid.- <html lang="en"> - <body>{children}</body> - <DubAnalytics /> - </html> + <html lang="en"> + <body> + <DubAnalytics /> + {children} + </body> + </html>conversions/leads/supabase.mdx (1)
56-71
: App Router: cookie deletion won’t persist—delete on the response, not via request cookies().Mutating
cookies()
here doesn’t attach a Set-Cookie header to the redirect. Delete on theNextResponse
instance so thedub_id
cookie is actually cleared.Apply this diff:
- const { user } = data; - const dub_id = cookies().get("dub_id")?.value; + const { user } = data; + const dub_id = cookies().get("dub_id")?.value; @@ - if (dub_id && isNewUser) { - waitUntil( - dub.track.lead({ - clickId: dub_id, - eventName: "Sign Up", - customerExternalId: user.id, - customerName: user.user_metadata.name, - customerEmail: user.email, - customerAvatar: user.user_metadata.avatar_url, - }) - ); - // delete the clickId cookie - cookies().delete("dub_id"); - } - return NextResponse.redirect(`${origin}${next}`); + const res = NextResponse.redirect(`${origin}${next}`); + if (dub_id && isNewUser) { + waitUntil( + dub.track.lead({ + clickId: dub_id, + eventName: "Sign Up", + customerExternalId: user.id, + customerName: user.user_metadata.name, + customerEmail: user.email, + customerAvatar: user.user_metadata.avatar_url, + }) + ); + // delete the clickId cookie on the response + res.cookies.delete("dub_id"); + } + return res;
🧹 Nitpick comments (11)
conversions/leads/clerk.mdx (1)
22-32
: JSX attribute casing on iframe.Use React/JSX-cased props so attributes are applied.
-<iframe +<iframe width="100%" height="469px" className="rounded-xl" src="https://www.loom.com/embed/7338589f0c0c4ee1b71c9f2aa28aac87?sid=04c67f3b-1bec-468a-b0c7-5b24d24cd96e" title="Loom video player" - frameborder="0" + frameBorder={0} allow="accelerometer; autoplay; clipboard-write; encrypted-media; gyroscope; picture-in-picture; web-share" - referrerpolicy="strict-origin-when-cross-origin" - allowfullscreen + referrerPolicy="strict-origin-when-cross-origin" + allowFullScreen />conversions/leads/auth0.mdx (1)
29-63
: Auth0 Session fields: use stable properties.In
nextjs-auth0
, the unique id is typicallysession.user.sub
and avatar issession.user.picture
. Adjust to avoid confusion.- customerExternalId: session.user.id, - customerName: session.user.name, - customerEmail: session.user.email, - customerAvatar: session.user.image, + customerExternalId: session.user.sub, + customerName: session.user.name, + customerEmail: session.user.email, + customerAvatar: session.user.picture,conversions/leads/appwrite.mdx (2)
63-68
: Env var naming consistency.Elsewhere you use
DUB_API_KEY
; here it’sNEXT_DUB_API_KEY
. Consider standardizing across docs to reduce confusion.
173-229
: Undefined import in example text.The page references
getLoggedInUser()
but it’s not defined in earlier snippets from@/lib/server/appwrite
. Add or remove to avoid copy‑paste errors.conversions/sales/shopify.mdx (4)
9-12
: Grammar tweak.“Conversion tracking require” → “requires”.
- Conversion tracking require a [Business plan](https://dub.co/pricing) + Conversion tracking requires a [Business plan](https://dub.co/pricing)
16-22
: Alt text mismatch.Image alt mentions “lead events” on a sales page. Update for accuracy.
- alt="A diagram showing how lead events are tracked in the conversion funnel" + alt="A diagram showing how sale events are tracked in the conversion funnel"
26-36
: JSX attribute casing on iframe.Use JSX-cased props.
- frameborder="0" + frameBorder={0} - referrerpolicy="strict-origin-when-cross-origin" + referrerPolicy="strict-origin-when-cross-origin" - allowfullscreen + allowFullScreen
78-89
: Broken link (double slash).Fix extra slash in
/conversions//quickstart
.-With the Shopify app, you can also create [conversion-enabled links](/conversions//quickstart#step-1%3A-enable-conversion-tracking-for-your-links) directly from your Shopify store: +With the Shopify app, you can also create [conversion-enabled links](/conversions/quickstart#step-1%3A-enable-conversion-tracking-for-your-links) directly from your Shopify store:conversions/leads/supabase.mdx (1)
93-126
: Pages Router cookie deletion: consider mirroring original cookie attributes.Deletion succeeds best when path/domain match the original. If the original set a Domain or SameSite/HttpOnly/Secure, add them here as appropriate.
conversions/sales/stripe.mdx (2)
16-16
: Grammar nit: agreement.“listens … and track” → “listens … and tracks”.
Apply this diff:
-Dub comes with a powerful Stripe integration that automatically listens to payment events on Stripe and track them as sales on Dub. +Dub comes with a powerful Stripe integration that automatically listens to payment events from Stripe and tracks them as sales on Dub.
141-175
: Clarify source of dub_id in examples.Some snippets use
client-reference-id
/client_reference_id
, others showreq.headers.get("dub_id")
without context. Consider adding a one-liner per snippet clarifying how to read the Dub click ID in:
- Next.js App Router (cookies().get("dub_id")?.value)
- Next.js Pages Router (
req.cookies.dub_id
)- Generic Node/Express (
req.cookies.dub_id
if using cookie-parser)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
conversions/leads/appwrite.mdx
(1 hunks)conversions/leads/auth0.mdx
(1 hunks)conversions/leads/better-auth.mdx
(1 hunks)conversions/leads/clerk.mdx
(1 hunks)conversions/leads/next-auth.mdx
(1 hunks)conversions/leads/segment.mdx
(1 hunks)conversions/leads/supabase.mdx
(1 hunks)conversions/sales/segment.mdx
(1 hunks)conversions/sales/shopify.mdx
(1 hunks)conversions/sales/stripe.mdx
(1 hunks)docs.json
(6 hunks)
🔇 Additional comments (16)
conversions/sales/segment.mdx (1)
2-2
: Consistent taxonomy update — looks good.Title change aligns with the new "(analytics)" suffix pattern.
conversions/leads/clerk.mdx (1)
2-2
: Title suffix update approved.Matches the "(auth)" convention introduced in this PR.
conversions/leads/auth0.mdx (1)
2-2
: Title taxonomy LGTM.Auth pages consistently use "(auth)".
conversions/leads/better-auth.mdx (1)
2-2
: Front‑matter update confirmed.No further issues in this doc.
conversions/leads/segment.mdx (1)
2-2
: Title change OK.Aligns with "(analytics)" convention.
conversions/leads/next-auth.mdx (1)
2-2
: Title suffix change looks good.Matches the reorg.
conversions/leads/appwrite.mdx (1)
2-2
: Front‑matter title change approved.Consistent with other auth docs.
conversions/sales/shopify.mdx (1)
2-2
: Payments taxonomy suffix LGTM.Matches Stripe/Shopify pattern.
conversions/leads/supabase.mdx (1)
2-2
: Title suffix LGTM.Matches the new “Third-party integrations” grouping and improves scanability.
conversions/sales/stripe.mdx (3)
2-2
: Title suffix LGTM.Consistent with the reorg and clearer in the new dropdown.
151-169
: Keys in docs: confirm these are test publishable keys.Looks like test keys, but please double‑check no live credentials slipped in.
103-106
: Anchor IDs with encoded colons: verify they match generated headings.The hash links include
%3A
for “Option X: …”. Ensure the site’s heading ID generator matches these slugs to avoid broken in‑page links.Also applies to: 258-264, 291-315
docs.json (4)
75-85
: Rename to “Third-party integrations”: good consolidation.Grouping lead integrations under a single submenu improves discoverability.
93-101
: Sales integrations grouped: good parity with leads.Stripe/Shopify/Segment are now consistently discoverable.
12-20
: Contextual options list: confirm UI supports the added providers.If these populate a share menu, ensure icons/labels exist for “Claude”, “Cursor”, and “VS Code”.
28-31
: All slugs resolve to files — no action required.
Ran the provided verification script; output: PASS: All slugs resolved to files.
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 (2)
conversions/leads/supabase.mdx (2)
90-93
: Avoid deriving origin from headers directly; validate the next path.
x-forwarded-proto
/host
are spoofable. Prefer a trusted base URL and ensurenext
starts with “/”.- const { code, next = "/" } = req.query; - const origin = `${req.headers["x-forwarded-proto"]}://${req.headers.host}`; + const { code, next = "/" } = req.query; + const baseUrl = + process.env.NEXT_PUBLIC_APP_URL || `https://${req.headers.host}`; + const nextPath = + typeof next === "string" && next.startsWith("/") ? next : "/";And later:
- return res.redirect(`${origin}${next}`); + return res.redirect(`${baseUrl}${nextPath}`);
94-98
: Do not use the Supabase service role key in this handler.
SUPABASE_SERVICE_ROLE_KEY
is over‑privileged for an auth code exchange. Use the anon key.- const supabase = createClient( - process.env.NEXT_PUBLIC_SUPABASE_URL!, - process.env.SUPABASE_SERVICE_ROLE_KEY! - ); + const supabase = createClient( + process.env.NEXT_PUBLIC_SUPABASE_URL!, + process.env.NEXT_PUBLIC_SUPABASE_ANON_KEY! + );
🧹 Nitpick comments (3)
conversions/leads/segment.mdx (1)
146-151
: Verified: example link is live (HTTP 200). Consider pinning to a commit SHA for stability.
conversions/leads/segment.mdx (lines 146–151) — GitHub URL returned 200.conversions/leads/supabase.mdx (2)
68-70
: More reliable cookie deletion (App Router).Deleting by name may fail if the cookie was set with a specific path or attributes. Expire it explicitly with secure flags.
- // delete the clickId cookie - cookies().delete("dub_id"); + // delete the clickId cookie (explicit expiry + attributes) + cookies().set({ + name: "dub_id", + value: "", + expires: new Date(0), + path: "/", + httpOnly: true, + secure: true, + sameSite: "lax", + });
123-126
: Harden cookie deletion (Pages Router).Add HttpOnly, Secure, and SameSite to the deletion header.
- res.setHeader( - "Set-Cookie", - `dub_id=; Path=/; Expires=Thu, 01 Jan 1970 00:00:00 GMT` - ); + res.setHeader( + "Set-Cookie", + `dub_id=; Path=/; Expires=Thu, 01 Jan 1970 00:00:00 GMT; HttpOnly; Secure; SameSite=Lax` + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
conversions/leads/appwrite.mdx
(1 hunks)conversions/leads/auth0.mdx
(1 hunks)conversions/leads/better-auth.mdx
(1 hunks)conversions/leads/clerk.mdx
(1 hunks)conversions/leads/next-auth.mdx
(1 hunks)conversions/leads/segment.mdx
(1 hunks)conversions/leads/supabase.mdx
(1 hunks)conversions/sales/segment.mdx
(1 hunks)conversions/sales/shopify.mdx
(1 hunks)conversions/sales/stripe.mdx
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (8)
- conversions/leads/better-auth.mdx
- conversions/leads/next-auth.mdx
- conversions/sales/shopify.mdx
- conversions/leads/auth0.mdx
- conversions/sales/stripe.mdx
- conversions/leads/clerk.mdx
- conversions/sales/segment.mdx
- conversions/leads/appwrite.mdx
🔇 Additional comments (3)
conversions/leads/segment.mdx (1)
3-3
: LGTM — sidebarTitle consistent; confirm docs nav consumes it.sidebarTitle is set across MDX pages (e.g., conversions/leads/segment.mdx, conversions/sales/stripe.mdx, sdks/client-side/features/conversion-tracking.mdx) and the "(Analytics)/(Auth)/(Payments)" suffix taxonomy is consistent. I did not find code that explicitly consumes sidebarTitle — confirm your docs build/sidebars config reads frontmatter.sidebarTitle for sidebar rendering.
conversions/leads/supabase.mdx (2)
149-153
: External GitHub example link verified — no action required.
Referenced GitHub file returns HTTP 200, so the docs link is valid.
3-3
: Front‑matter addition OK — verify sidebarTitle naming consistencySupabase (Auth) front‑matter looks good. Confirm all integration pages and nav entries use the "Name (Type)" pattern and identical capitalization for the parenthetical type.
third-party integrations
dropdown(<integration type>)
to each integrationSummary by CodeRabbit