feat(preview-server): add --clients option to filter email client compatibility warnings #2797
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@ReemX is attempting to deploy a commit to the resend Team on Vercel. A member of the Team first needs to authorize it. |
|
commit: |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
2 issues found across 9 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/react-email/src/utils/preview/start-dev-server.ts">
<violation number="1" location="packages/react-email/src/utils/preview/start-dev-server.ts:35">
P2: Port retry drops the new --clients option: compatibilityClients isn’t passed when startDevServer recurses after EADDRINUSE, so retries ignore the user’s client filter.</violation>
</file>
<file name="packages/react-email/src/utils/preview/get-env-variables-for-preview-app.ts">
<violation number="1" location="packages/react-email/src/utils/preview/get-env-variables-for-preview-app.ts:16">
P2: Undefined compatibilityClients overwrites existing COMPATIBILITY_EMAIL_CLIENTS when merging env, breaking user-provided env var support.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| PREVIEW_SERVER_LOCATION: previewServerLocation, | ||
| USER_PROJECT_LOCATION: cwd, | ||
| RESEND_API_KEY: resendApiKey, | ||
| COMPATIBILITY_EMAIL_CLIENTS: compatibilityClients, |
Contributor
There was a problem hiding this comment.
P2: Undefined compatibilityClients overwrites existing COMPATIBILITY_EMAIL_CLIENTS when merging env, breaking user-provided env var support.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/react-email/src/utils/preview/get-env-variables-for-preview-app.ts, line 16:
<comment>Undefined compatibilityClients overwrites existing COMPATIBILITY_EMAIL_CLIENTS when merging env, breaking user-provided env var support.</comment>
<file context>
@@ -5,12 +5,14 @@ export const getEnvVariablesForPreviewApp = (
PREVIEW_SERVER_LOCATION: previewServerLocation,
USER_PROJECT_LOCATION: cwd,
RESEND_API_KEY: resendApiKey,
+ COMPATIBILITY_EMAIL_CLIENTS: compatibilityClients,
} as const;
};
</file context>
Suggested change
| COMPATIBILITY_EMAIL_CLIENTS: compatibilityClients, | |
| ...(compatibilityClients !== undefined | |
| ? { COMPATIBILITY_EMAIL_CLIENTS: compatibilityClients } | |
| : {}), |
…patibility warnings Adds a new `--clients` CLI option to the `dev` command that allows users to specify which email clients to show compatibility warnings for. This is useful for teams that only target specific email clients (e.g., corporate environments that only use Outlook). Usage: email dev --clients outlook email dev --clients "outlook,gmail,apple-mail" If not specified, defaults to ['gmail', 'apple-mail', 'outlook', 'yahoo']. Also supports the COMPATIBILITY_EMAIL_CLIENTS environment variable for configuration without CLI flags. Closes resend#2796
ReemX
force-pushed
the
feat/configurable-email-clients
branch
from
82d77b5 to
866fa3f
Compare
Contributor
There was a problem hiding this comment.
2 issues found across 21 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/preview-server/src/actions/email-validation/check-compatibility.ts">
<violation number="1" location="packages/preview-server/src/actions/email-validation/check-compatibility.ts:15">
P2: COMPATIBILITY_EMAIL_CLIENTS is no longer normalized/validated; invalid/empty values now suppress all compatibility warnings instead of falling back to defaults.</violation>
</file>
<file name="packages/preview-server/src/actions/get-email-path-from-slug.ts">
<violation number="1" location="packages/preview-server/src/actions/get-email-path-from-slug.ts:10">
P1: User-controlled slug is joined without containment checks, allowing path traversal/absolute-path override outside EMAILS_DIR_ABSOLUTE_PATH before file access.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| export const getEmailPathFromSlug = cache(async (slug: string) => { | ||
| if (['.tsx', '.jsx', '.ts', '.js'].includes(path.extname(slug))) | ||
| return path.join(emailsDirectoryAbsolutePath, slug); | ||
| return path.join(env.EMAILS_DIR_ABSOLUTE_PATH, slug); |
Contributor
There was a problem hiding this comment.
P1: User-controlled slug is joined without containment checks, allowing path traversal/absolute-path override outside EMAILS_DIR_ABSOLUTE_PATH before file access.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/preview-server/src/actions/get-email-path-from-slug.ts, line 10:
<comment>User-controlled slug is joined without containment checks, allowing path traversal/absolute-path override outside EMAILS_DIR_ABSOLUTE_PATH before file access.</comment>
<file context>
@@ -3,13 +3,13 @@
export const getEmailPathFromSlug = cache(async (slug: string) => {
if (['.tsx', '.jsx', '.ts', '.js'].includes(path.extname(slug)))
- return path.join(emailsDirectoryAbsolutePath, slug);
+ return path.join(env.EMAILS_DIR_ABSOLUTE_PATH, slug);
- const pathWithoutExtension = path.join(emailsDirectoryAbsolutePath, slug);
</file context>
| getUsedStyleProperties, | ||
| } from '../../utils/caniemail/ast/get-used-style-properties'; | ||
| import { | ||
| relevantEmailClients, |
Contributor
There was a problem hiding this comment.
P2: COMPATIBILITY_EMAIL_CLIENTS is no longer normalized/validated; invalid/empty values now suppress all compatibility warnings instead of falling back to defaults.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/preview-server/src/actions/email-validation/check-compatibility.ts, line 15:
<comment>COMPATIBILITY_EMAIL_CLIENTS is no longer normalized/validated; invalid/empty values now suppress all compatibility warnings instead of falling back to defaults.</comment>
<file context>
@@ -13,8 +12,7 @@ import {
import {
- allEmailClients,
- type EmailClient,
+ relevantEmailClients,
type SupportEntry,
} from '../../utils/caniemail/email-clients';
</file context>
Contributor
There was a problem hiding this comment.
1 issue found across 7 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/preview-server/src/app/env.ts">
<violation number="1" location="packages/preview-server/src/app/env.ts:22">
P2: `skipValidation: true` disables all env validation and default application, allowing missing/invalid env vars to reach runtime</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| .optional() | ||
| .default('false'), | ||
| }, | ||
| skipValidation: true, |
Contributor
There was a problem hiding this comment.
P2: skipValidation: true disables all env validation and default application, allowing missing/invalid env vars to reach runtime
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/preview-server/src/app/env.ts, line 22:
<comment>`skipValidation: true` disables all env validation and default application, allowing missing/invalid env vars to reach runtime</comment>
<file context>
@@ -10,12 +10,16 @@ export const env = createEnv({
+ .optional()
+ .default('false'),
},
+ skipValidation: true,
experimental__runtimeEnv: {
NEXT_PUBLIC_IS_BUILDING: process.env.NEXT_PUBLIC_IS_BUILDING,
</file context>
Suggested change
| skipValidation: true, | |
| - skipValidation: true, | |
| + skipValidation: process.env.SKIP_ENV_VALIDATION === 'true', |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
--clientsCLI option to thedevcommand that allows users to specify which email clients to show compatibility warnings forCOMPATIBILITY_EMAIL_CLIENTSenvironment variableUsage
Details
--clientsis not specified, defaults to['gmail', 'apple-mail', 'outlook', 'yahoo'](existing behavior)gmail,outlook,yahoo,apple-mail,aol,thunderbird,microsoft,samsung-email,sfr,orange,protonmail,hey,mail-ru,fastmail,laposte,t-online-de,free-fr,gmx,web-de,ionos-1and1,rainloop,wp-plChanges
packages/react-email/src/index.ts- Added--clientsCLI optionpackages/react-email/src/commands/dev.ts- Pass clients option throughpackages/react-email/src/utils/preview/start-dev-server.ts- Pass to env builderpackages/react-email/src/utils/preview/get-env-variables-for-preview-app.ts- AddCOMPATIBILITY_EMAIL_CLIENTSenv varpackages/preview-server/src/app/env.ts- Export new env varpackages/preview-server/src/utils/caniemail/email-clients.ts- NEW: Extracted types and constants from server actionpackages/preview-server/src/actions/email-validation/check-compatibility.ts- Use configurable email clientsTest plan
--clients outlookto verify only Outlook warnings shownCloses #2796
Summary by cubic
Adds a --clients option to the dev and build commands plus a COMPATIBILITY_EMAIL_CLIENTS env var to filter compatibility warnings to chosen email clients. Keeps default clients unchanged so teams can focus on targets without noise.
New Features
Refactors
Written for commit 1692826. Summary will update on new commits.