Email templates with logo #852
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughEmail rendering now accepts a unified Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant API as /emails/send-email (POST)
participant Renderer as renderEmailWithTemplate
participant Theme as EmailTheme (Light/Dark)
participant Template as EmailTemplate
Client->>API: Send email request
API->>Renderer: renderEmailWithTemplate(template, theme, { variables, themeProps:{ unsubscribeLink, projectDisplayName, logoUrl, fullLogoUrl }, previewMode })
Renderer->>Theme: Render with {...themeProps}
Theme->>Theme: Conditionally render header (fullLogo → logo + name → none)
Theme->>Template: Wrap template content with themed layout
Renderer-->>API: { html, text, subject?, notificationCategory? }
API-->>Client: Return/send result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (9)
✨ 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 (
|
|
Claude finished @fomalhautb's task —— View job Pull Request ReviewReviewing the "Email templates with logo" changes for code quality, security, performance, and potential issues. Todo List
Review SummaryThis PR adds logo support to email templates by extending the theme props to include ✅ Positive Aspects
|
|
Claude finished @fomalhautb's task —— View job Code Review Complete ✅I've thoroughly reviewed PR #852 that adds logo support to email templates. Here's my comprehensive feedback: Todo Checklist
🔍 Review SummaryThe implementation correctly adds logo support to email templates with proper type safety and consistent patterns. The changes are well-structured and follow existing codebase conventions. ✅ Strengths
|
Review by RecurseML🔍 Review performed on 933c60d..7bce171 ✨ No bugs found, your code is sparkling clean ✅ Files analyzed, no issues (5)• |
| @@ -43,15 +43,24 @@ export const emptyEmailTheme = deindent` | |||
| `; | |||
|
|
|||
| export const LightEmailTheme = `import { Html, Head, Tailwind, Body, Container, Link } from '@react-email/components'; | |||
| import { Html, Head, Tailwind, Body, Container, Link, Img } from '@react-email/components'; | |||
There was a problem hiding this comment.
Duplicate import in LightEmailTheme string literal: remove the redundant import and combine required modules (include Img in a single import).
There was a problem hiding this comment.
Greptile Summary
This review covers only the changes made since the last review (commit 7bce171), not the entire PR.
This PR adds logo support to Stack Auth's email template system, enabling projects to display their branding in email communications. The implementation introduces a new themeProps object structure that replaces the simple unsubscribeLink parameter across the email rendering pipeline.
Key changes include:
-
Email Rendering Pipeline: Updated
renderEmailWithTemplate()inemail-rendering.tsxto accept a comprehensivethemePropsobject containingunsubscribeLink,projectDisplayName,logoUrl, andfullLogoUrl. This props object is properly spread to the EmailTheme component during rendering. -
API Route Updates: Both
render-email/route.tsxandsend-email/route.tsxnow constructthemePropsfrom the tenancy project configuration, extracting logo URLs and display names fromtenancy.project.logo_url,tenancy.project.full_logo_url, andtenancy.project.display_name. -
Email Theme Templates: The
LightEmailThemeandDarkEmailThemeinemails.tsnow include conditional logo rendering logic. They prioritize displaying a full logo (fullLogoUrl) if available, otherwise show a smaller logo alongside the project name (logoUrl+projectDisplayName). -
Type Safety: Extended the
ThemePropstype definition incode-editor.tsxto include the new optional logo properties, ensuring proper TypeScript intellisense for developers writing custom email templates.
The implementation maintains backward compatibility by making all logo properties optional and using null coalescing operators to handle missing values. The changes integrate well with the existing email rendering architecture, following established patterns for prop passing and theme customization.
Confidence score: 3/5
- This PR has implementation issues that should be addressed before merging, particularly duplicate imports in email themes
- Score reflects code quality concerns and potential runtime issues with malformed template strings
- Pay close attention to
packages/stack-shared/src/helpers/emails.tswhich contains duplicate import statements that will cause compilation errors
5 files reviewed, 1 comment
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
|
Claude finished @fomalhautb's task —— View job Pull Request ReviewReviewing the "Email templates with logo" changes for code quality, security, performance, and potential issues. Todo List
Review SummaryThis PR adds logo support to email templates by extending the theme props to include ✅ Positive Aspects
|
Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
|
Claude finished @fomalhautb's task —— View job Pull Request ReviewReviewing the "Email templates with logo" changes for code quality, security, performance, and potential issues. Todo List
Review SummaryThis PR adds logo support to email templates by extending the theme props to include ✅ Positive Aspects
|
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
packages/stack-shared/src/helpers/emails.ts (1)
45-46: Duplicate imports inside LightEmailTheme string cause syntax errorThere are two import lines for @react-email/components inside the template string. This will break bundling.
Apply this dedupe:
-export const LightEmailTheme = `import { Html, Head, Tailwind, Body, Container, Link } from '@react-email/components'; -import { Html, Head, Tailwind, Body, Container, Link, Img } from '@react-email/components'; +export const LightEmailTheme = `import { Html, Head, Tailwind, Body, Container, Link, Img } from '@react-email/components';apps/backend/src/lib/email-rendering.tsx (1)
95-101: Fix: spreading possibly undefined themeProps throws at runtime; also set correct merge precedenceIf options.themeProps is undefined, JSON.stringify(options.themeProps) becomes undefined and {...undefined} throws. Also, PreviewProps should provide defaults that call-site props can override, not the other way around.
Apply:
- const themeProps = { - ...${JSON.stringify(options.themeProps)}, - ...${previewMode ? "EmailTheme.PreviewProps" : "{}"}, - } + const themeProps = { + ...${previewMode ? "EmailTheme.PreviewProps" : "{}"}, + ...${JSON.stringify(options.themeProps || {})}, + }This unblocks first-pass renders (which don’t pass themeProps) in send-email and prevents PreviewProps from clobbering explicit props.
🧹 Nitpick comments (5)
packages/stack-shared/src/helpers/emails.ts (5)
47-47: Prefer type-only import for ThemeProps to avoid runtime importUsing a value import for a type may keep an unnecessary import after transpilation depending on settings.
-import { ThemeProps } from "@stackframe/emails" +import type { ThemeProps } from "@stackframe/emails"
56-63: Add text fallback and better alt text for accessibilityIf neither logo is set, nothing renders; also alt text should be descriptive.
- {fullLogoUrl ? - <Img src={fullLogoUrl} alt="Full Logo" className="h-16" /> : - logoUrl ? - <div className="flex gap-2 items-center"> - <Img src={logoUrl} alt="Logo" className="h-8" /> - <h2>{projectDisplayName}</h2> - </div> - : null} + {fullLogoUrl ? ( + <Img src={fullLogoUrl} alt={(projectDisplayName ? projectDisplayName + " " : "") + "full logo"} className="h-16" /> + ) : logoUrl ? ( + <div className="flex gap-2 items-center"> + <Img src={logoUrl} alt={(projectDisplayName ? projectDisplayName + " " : "") + "logo"} className="h-8" /> + <h2>{projectDisplayName}</h2> + </div> + ) : ( + projectDisplayName ? <h2>{projectDisplayName}</h2> : null + )}
79-80: Consider enriching PreviewProps to showcase branding in previewOptional: add sample projectDisplayName/logoUrl to improve preview experience in the editor.
EmailTheme.PreviewProps = { - unsubscribeLink: "https://example.com", + unsubscribeLink: "https://example.com", + projectDisplayName: "Acme Inc.", + // logoUrl: "https://example.com/logo.png", + // fullLogoUrl: "https://example.com/full-logo.png", } satisfies Partial<ThemeProps>
84-85: Prefer type-only import for ThemeProps in Dark themeSame rationale as Light theme.
-const DarkEmailTheme = `import { Html, Head, Tailwind, Body, Container, Link, Img } from '@react-email/components'; -import { ThemeProps } from "@stackframe/emails" +const DarkEmailTheme = `import { Html, Head, Tailwind, Body, Container, Link, Img } from '@react-email/components'; +import type { ThemeProps } from "@stackframe/emails"
94-101: Dark theme: add text fallback and descriptive alt textMirror Light theme’s accessibility improvements.
- {fullLogoUrl ? - <Img src={fullLogoUrl} alt="Full Logo" className="h-16" /> : - logoUrl ? - <div className="flex gap-2 items-center"> - <Img src={logoUrl} alt="Logo" className="h-8" /> - <h2 className="text-white">{projectDisplayName}</h2> - </div> - : null} + {fullLogoUrl ? ( + <Img src={fullLogoUrl} alt={(projectDisplayName ? projectDisplayName + " " : "") + "full logo"} className="h-16" /> + ) : logoUrl ? ( + <div className="flex gap-2 items-center"> + <Img src={logoUrl} alt={(projectDisplayName ? projectDisplayName + " " : "") + "logo"} className="h-8" /> + <h2 className="text-white">{projectDisplayName}</h2> + </div> + ) : ( + projectDisplayName ? <h2 className="text-white">{projectDisplayName}</h2> : null + )}
📜 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)
apps/backend/src/app/api/latest/emails/render-email/route.tsx(1 hunks)apps/backend/src/app/api/latest/emails/send-email/route.tsx(2 hunks)apps/backend/src/lib/email-rendering.tsx(3 hunks)apps/dashboard/src/components/vibe-coding/code-editor.tsx(1 hunks)packages/stack-shared/src/helpers/emails.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
apps/backend/src/app/api/latest/**/*
📄 CodeRabbit Inference Engine (CLAUDE.md)
apps/backend/src/app/api/latest/**/*: Main API routes are located in /apps/backend/src/app/api/latest
The project uses a custom route handler system in the backend for consistent API responses
Files:
apps/backend/src/app/api/latest/emails/render-email/route.tsxapps/backend/src/app/api/latest/emails/send-email/route.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). (8)
- GitHub Check: docker
- GitHub Check: lint_and_build (latest)
- GitHub Check: setup-tests
- GitHub Check: build (22.x)
- GitHub Check: build (22.x)
- GitHub Check: restart-dev-and-test
- GitHub Check: all-good
- GitHub Check: Security Check
🔇 Additional comments (7)
apps/backend/src/app/api/latest/emails/render-email/route.tsx (2)
58-62: Branding props wiring looks goodPassing projectDisplayName, logoUrl, and fullLogoUrl via themeProps aligns with the new renderer signature and EmailTheme expectations. No issues here.
58-62: Sanity-check logo URLs are absolute and reachableIf tenancy.project.logo_url/full_logo_url can be relative or point to private hosts, many email clients will fail to display them. Ensure these values are absolute, public URLs, or normalize/validate before passing through.
Would you like a guard that validates these URLs and drops invalid ones to avoid broken images?
apps/dashboard/src/components/vibe-coding/code-editor.tsx (1)
96-99: Type augmentation matches backend themePropsAdding projectDisplayName, logoUrl, and fullLogoUrl to ThemeProps keeps Monaco typings in sync with the renderer and themes. Looks good.
apps/backend/src/app/api/latest/emails/send-email/route.tsx (3)
158-163: Good: themeProps passed with unsubscribe and brandingThis aligns with the updated renderEmailWithTemplate signature and new EmailTheme props.
107-115: First-pass render doesn’t pass themeProps; relies on renderer to handle undefinedThe first pass to compute notification category omits themeProps. With the current renderer implementation, object-spreading an undefined themeProps causes a runtime error. Ensure the renderer defaults themeProps to {} when absent (see email-rendering.tsx review).
Do you want me to open a follow-up to harden renderEmailWithTemplate and/or pass branding themeProps in the first pass too?
6-6:KnownErrorscorrectly re-exported at package rootThe
packages/stack-shared/src/index.tsfile includes:export { KnownError, KnownErrors } from "./known-errors";so importing
KnownErrorsfrom"@stackframe/stack-shared"will resolve successfully at runtime.apps/backend/src/lib/email-rendering.tsx (1)
52-57: Public API extension is coherentAdding themeProps with unsubscribeLink, projectDisplayName, logoUrl, and fullLogoUrl matches consuming routes and theme templates.
Important
Add logo support to email templates by passing logo URLs and project display names to email rendering functions and themes.
themePropswithlogoUrl,fullLogoUrl, andprojectDisplayNametorenderEmailWithTemplate()inrender-email/route.tsxandsend-email/route.tsx.renderEmailWithTemplate()inemail-rendering.tsxto acceptthemePropsand pass them to theEmailThemecomponent.LightEmailThemeandDarkEmailThemeinemails.tsto display logos usingImgcomponent.fullLogoUrlandlogoUrlinEmailThemecomponents.ThemePropsincode-editor.tsxto includelogoUrl,fullLogoUrl, andprojectDisplayName.This description was created by
for 7bce171. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Style