refactor: improve domain configuration experience #1848
Conversation
WalkthroughAdds a clipboard copy hook and CopyButton, integrates per-record copy controls into domain configuration with optional TXT verification rendering, removes the domain hyperlink in the domain card (renders plain text), and updates Vercel domain creation to the v10 JSON API with lowercase domain names. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant DomainConfig as DomainConfiguration
participant DnsRow as DnsRecordRow
participant CopyBtn as CopyButton
participant Hook as useCopyToClipboard
participant Clipboard as Navigator.clipboard
participant Toast as Toast
User->>DomainConfig: Open domain settings
DomainConfig->>DomainConfig: Compute records (A/CNAME + optional TXT)
DomainConfig->>DnsRow: Render rows with values and CopyButton(s)
User->>CopyBtn: Click copy (Type/Name/Value)
CopyBtn->>Hook: invoke copyToClipboard(value)
Hook->>Clipboard: writeText / write
Clipboard-->>Hook: success
Hook-->>CopyBtn: set copied = true
CopyBtn->>Toast: show success message
Note right of CopyBtn: Icon switches to Check until timeout
alt TXT verification present
DomainConfig->>DnsRow: Render TXT row with warning
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes ✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/domains.ts (1)
47-61: Await deletions; current function resolves before work completes.removeDomainFromVercel is async but doesn’t await the project/team deletions. Callers may proceed early and UI can desync.
export const removeDomainFromVercel = async ( domain: string, domainCount: number, ) => { if (domainCount > 1) { // the apex domain is being used by other domains // so we should only remove it from our Vercel project - removeDomainFromVercelProject(domain); + await removeDomainFromVercelProject(domain); } else { // this is the only domain using this apex domain // so we can remove it entirely from our Vercel team - removeDomainFromVercelProject(domain); - removeDomainFromVercelTeam(domain); + await removeDomainFromVercelProject(domain); + await removeDomainFromVercelTeam(domain); } };
🧹 Nitpick comments (10)
lib/domains.ts (3)
66-77: Harden error handling for fetch calls.Most functions blindly json() the response. Propagate non-2xx with actionable messages.
- ).then((res) => { - return res.json(); - }); + ).then(async (res) => { + const data = await res.json(); + if (!res.ok) { + const msg = data?.error?.message || `Vercel API error (${res.status})`; + throw new Error(msg); + } + return data; + });Apply similarly to other fetches.
Also applies to: 80-93, 95-108
115-129: Apex extraction is naive for multi-part TLDs (e.g., .co.uk).Consider tldts to robustly parse apex/subdomains; also punycode (toASCII) for IDNs before sending to Vercel.
131-134: Regex-based validation is limited.validDomainRegex won’t handle IDNs/public suffix edge-cases. Prefer library validation (e.g., tldts) and normalize with toASCII + toLowerCase before API calls.
components/domains/domain-card.tsx (2)
102-104: Confirm UX: domain no longer clickable.Switching from a link to plain text removes quick navigation. If unintentional, restore the anchor or add a contextual “Open” action.
- <div className="flex items-center gap-1.5 truncate text-sm font-medium sm:gap-2.5"> - {domain} + <div className="flex items-center gap-1.5 truncate text-sm font-medium sm:gap-2.5"> + <a + href={`http://${domain}`} + target="_blank" + rel="noopener noreferrer" + className="truncate hover:underline" + > + {domain} + </a>
146-148: Add accessible state for the disclosure (ARIA).Reflect expanded state and control relationship for screen readers.
- <Button + <Button variant="secondary" className={cn( "h-8 w-auto px-2 opacity-100 transition-opacity lg:h-9", !showDetails && !isInvalid && "sm:opacity-0 sm:group-hover:opacity-100", )} onClick={() => setShowDetails((s) => !s)} data-state={showDetails ? "open" : "closed"} + aria-expanded={showDetails} + aria-controls={`domain-details-${domain.replace(/\./g, "-")}`} > ... - <motion.div + <motion.div initial={false} animate={{ height: showDetails ? "auto" : 0 }} className="overflow-hidden" + id={`domain-details-${domain.replace(/\./g, "-")}`} >Also applies to: 213-216
lib/hooks/use-copy-to-clipboard.ts (2)
22-37: Feature-detect clipboard API for wider compatibility.Guard navigator.clipboard and fail fast with a clear error for HTTP/unsupported contexts.
- if (typeof value === "string") { + if (!("clipboard" in navigator) || !navigator.clipboard) { + throw new Error("Clipboard API unavailable"); + } + if (typeof value === "string") {
1-58: Consolidate clipboard logic underuseCopyToClipboard. Replace all existing calls to the legacycopyToClipboard(text, message)util with the hook’scopyToClipboard(value, { onSuccess: () => toast.success(message) }), then removelib/utils.ts’s copyToClipboard.components/domains/domain-configuration.tsx (3)
23-31: Safer optional chaining on conflicts.some.Minor guard to avoid .some on undefined in edge states.
- {configJson?.conflicts.some((x: any) => x.type === "A") + {configJson?.conflicts?.some((x: any) => x.type === "A") ? "A Record (recommended)" : "CNAME Record (recommended)"}
177-191: Optional: enable copy for Name (and TTL) for parity.Users often copy the host label. Add CopyButton next to Name (and TTL if present).
- <p key={record.name} className="font-mono"> - {record.name} - </p> + <p key={record.name} className="flex items-end gap-1 font-mono"> + {record.name} + <CopyButton variant="neutral" className="-mb-0.5" value={record.name} /> + </p>
141-143: dangerouslySetInnerHTML: ensure inputs are trusted/escaped.You interpolate domain names into HTML strings above. Domain chars are restricted, but consider escaping or switching to JSX to avoid future XSS risk.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
components/domains/domain-card.tsx(2 hunks)components/domains/domain-configuration.tsx(5 hunks)components/ui/copy-button.tsx(1 hunks)lib/domains.ts(1 hunks)lib/hooks/use-copy-to-clipboard.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
components/ui/copy-button.tsx (2)
lib/utils.ts (2)
copyToClipboard(269-278)cn(18-20)lib/hooks/use-copy-to-clipboard.ts (1)
useCopyToClipboard(3-58)
lib/hooks/use-copy-to-clipboard.ts (1)
lib/utils.ts (1)
copyToClipboard(269-278)
components/domains/domain-configuration.tsx (2)
lib/utils.ts (1)
cn(18-20)components/ui/copy-button.tsx (1)
CopyButton(25-58)
⏰ 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). (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
lib/domains.ts (1)
9-19: No changes required: API version usage aligns with Vercel’s official endpoints
Confirmed that:
- addDomainToVercel uses POST /v10/projects/{…}/domains
- getDomainResponse & verifyDomain use GET/POST /v9/projects/{…}/domains/{domain}
- getDomainConfig & removeDomain from team use GET/DELETE /v6/domains/{domain}
All versions match the latest Vercel docs.
There was a problem hiding this comment.
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 (1)
components/domains/domain-configuration.tsx (1)
28-31: Optional chaining on conflicts to prevent rare crash.Both .some and .map should be guarded to handle unexpected shapes from the API.
Apply:
- {configJson?.conflicts.some((x: any) => x.type === "A") + {(configJson?.conflicts?.some((x: any) => x.type === "A") ?? false) ? "A Record (recommended)" : "CNAME Record (recommended)"}- records={configJson?.conflicts.map( + records={(configJson?.conflicts ?? []).map( ({ name, type, value }: { name: string; type: string; value: string }) => ({ name, type, value, }), )}Also applies to: 35-49
♻️ Duplicate comments (2)
components/domains/domain-configuration.tsx (2)
106-111: Resolved: safe defaults for recommended values.The optional chaining + fallbacks address the earlier crash risk on missing config arrays.
114-127: TXT record label derivation is incorrect for apex and some subdomains.slice(...) yields wrong names (e.g., "example.co"). Use getSubdomain(...) and fallback to "@".
Apply:
- { - type: txtVerification.type, - name: txtVerification.domain.slice( - 0, - txtVerification.domain.length - - domainJson.apexName.length - - 1, - ), - value: txtVerification.value, - }, + { + type: txtVerification.type, + name: + getSubdomain(txtVerification.domain, domainJson.apexName) ?? + "@", + value: txtVerification.value, + },
🧹 Nitpick comments (2)
components/domains/domain-configuration.tsx (2)
188-193: Enhance copy UX (optional).Consider a more specific toast and adding copy for Name as well.
Apply:
<CopyButton variant="neutral" className="-mb-0.5" value={record.value} + successMessage={`Copied value for ${record.name}`} />If desired, mirror a CopyButton in the Name cell:
- <p key={record.name} className="font-mono"> - {record.name} - </p> + <p key={record.name} className="flex items-end gap-1 font-mono"> + {record.name} + <CopyButton variant="neutral" className="-mb-0.5" value={record.name} successMessage="Copied name" /> + </p>
55-61: Consistency: reuse recommended config in conflicts flow (optional).Mirror the main block’s recommended value logic for A/CNAME to avoid diverging guidance.
Apply:
- value: - recordType === "A" ? `76.76.21.21` : `cname.vercel-dns.com`, + value: + recordType === "A" + ? (configJson?.recommendedIPv4?.[0]?.value?.[0] ?? "76.76.21.21") + : (configJson?.recommendedCNAME?.[0]?.value ?? "cname.vercel-dns.com"),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
components/domains/domain-configuration.tsx(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
components/domains/domain-configuration.tsx (2)
lib/utils.ts (1)
cn(18-20)components/ui/copy-button.tsx (1)
CopyButton(25-58)
⏰ 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). (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (4)
components/domains/domain-configuration.tsx (4)
9-9: CopyButton integration looks good.Import is correct and aligns with the hook API.
101-101: Nice pluralization handling in instructions.Keeps copy accurate when TXT verification is present.
129-133: Warning copy reads clearly.Message is explicit and appropriately gated by txtVerification.
160-170: Layout tweak to max-content is appropriate.Improves column sizing and avoids cramped values; TTL-aware grid split is clean.
| const txtVerification = | ||
| status === "Pending Verification" | ||
| ? domainJson.verification.find((x: any) => x.type === "TXT") | ||
| : undefined; | ||
|
|
There was a problem hiding this comment.
Guard against missing domainJson.verification.
If verification is absent, .find will throw. Add optional chaining.
Apply:
- const txtVerification =
- status === "Pending Verification"
- ? domainJson.verification.find((x: any) => x.type === "TXT")
- : undefined;
+ const txtVerification =
+ status === "Pending Verification"
+ ? domainJson?.verification?.find((x: any) => x.type === "TXT")
+ : undefined;📝 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 txtVerification = | |
| status === "Pending Verification" | |
| ? domainJson.verification.find((x: any) => x.type === "TXT") | |
| : undefined; | |
| const txtVerification = | |
| status === "Pending Verification" | |
| ? domainJson?.verification?.find((x: any) => x.type === "TXT") | |
| : undefined; |
🤖 Prompt for AI Agents
In components/domains/domain-configuration.tsx around lines 75 to 79, the code
calls domainJson.verification.find(...) which will throw if verification is
missing; change the access to use optional chaining or a safe default (e.g.,
domainJson.verification?.find(...) or (domainJson.verification || []).find(...))
so the lookup returns undefined rather than throwing when verification is
absent.
Summary by CodeRabbit
New Features
Style
Refactor
Chores