fix: no cname for domains #979
Conversation
WalkthroughUpdated VerifyStep’s CNAME record visibility condition to additionally require the domain to be a subdomain. The flag showCNAMERecord now depends on hasRecommendedCNAME, not being already configured, and isSubdomain(domain). No exported APIs changed. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant UI as VerifyStep Component
participant H as Helpers
U->>UI: Open Custom Domain Verify Step
UI->>H: Compute hasRecommendedCNAME, cnameConfigured, isSubdomain(domain)
H-->>UI: booleans
alt Show CNAME section
Note over UI: hasRecommendedCNAME && !cnameConfigured && isSubdomain(domain)
UI-->>U: Render CNAME record instructions
else Hide CNAME section
UI-->>U: CNAME section omitted
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
✨ 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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)
apps/web/app/(org)/dashboard/settings/organization/components/CustomDomainDialog/VerifyStep.tsx (1)
382-386: Fix CNAME “Name” for multi-label subdomains (e.g., foo.bar.example.co.uk).Using only the first label is incorrect; use the full subdomain via tldts.
Apply:
- {domain.split(".").length > 2 - ? domain.split(".")[0] - : "@"} + {(() => { + const { subdomain } = parse(domain); + return subdomain || "@"; + })()}
🧹 Nitpick comments (3)
apps/web/app/(org)/dashboard/settings/organization/components/CustomDomainDialog/VerifyStep.tsx (3)
53-55: Avoid mutating props-derived arrays when sorting IPv4 recommendations.Sort a copy to prevent side effects.
- const sortedIPv4 = recommendedIPv4.sort((a, b) => a.rank - b.rank); + const sortedIPv4 = [...recommendedIPv4].sort((a, b) => a.rank - b.rank);
391-394: Don’t mutate recommendedCnames during render.Sort a cloned array.
-{recommendedCnames - .sort((a, b) => a.rank - b.rank) +{[...recommendedCnames] + .sort((a, b) => a.rank - b.rank) .map((cname, index) => {
113-127: Tighten interval typing and effect deps to avoid stale closures.Use ReturnType for browser/node compatibility and include checkVerification in deps.
- let interval: NodeJS.Timeout; + let interval: ReturnType<typeof setInterval> | undefined; @@ - return () => { - clearInterval(interval); - }; - }, [activeOrganization?.organization.customDomain, isVerified]); + return () => { + if (interval) clearInterval(interval); + }; + }, [activeOrganization?.organization.customDomain, isVerified, checkVerification]);
📜 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)
apps/web/app/(org)/dashboard/settings/organization/components/CustomDomainDialog/VerifyStep.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/**/*.{ts,tsx}: Use TanStack Query v5 for client-side server state and data fetching in the web app
Mutations should call Server Actions and perform precise cache updates with setQueryData/setQueriesData, avoiding broad invalidations
Prefer Server Components for initial data and pass initialData to client components for React Query hydration
Files:
apps/web/app/(org)/dashboard/settings/organization/components/CustomDomainDialog/VerifyStep.tsx
{apps/web,packages/ui}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
{apps/web,packages/ui}/**/*.{ts,tsx}: Use Tailwind CSS exclusively for styling in the web app and shared React UI components
Component naming: React components in PascalCase; hooks in camelCase starting with 'use'
Files:
apps/web/app/(org)/dashboard/settings/organization/components/CustomDomainDialog/VerifyStep.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript and avoid any; prefer shared types from packages
Files:
apps/web/app/(org)/dashboard/settings/organization/components/CustomDomainDialog/VerifyStep.tsx
⏰ 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). (3)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Analyze (rust)
🔇 Additional comments (2)
apps/web/app/(org)/dashboard/settings/organization/components/CustomDomainDialog/VerifyStep.tsx (2)
98-99: CNAME visibility correctly limited to subdomains — LGTM.This aligns with DNS rules (no CNAME at the apex) and matches the A-record gating above.
1-4: Confirm this module is a Client Component.It uses hooks; add 'use client' if not already present.
+'use client'; import { LoadingSpinner } from "@cap/ui";
Summary by CodeRabbit