-
Notifications
You must be signed in to change notification settings - Fork 53
fix(jsx-email,create-mail): bundle next/v3 fixes #393
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
base: next/v3
Are you sure you want to change the base?
Conversation
Reorder imports and adjust spacing in documentation examples to match the updated create-mail scaffold conventions, keeping import groupings consistent across docs. Refs: #384
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.
The rehype plugins (getConditionalPlugin, getRawPlugin) have a real correctness bug: collecting indices and then mutating parents with splice() can invalidate later indices, causing incorrect replacements. The Windows drive-letter guard in preview.ts appears to check the wrong path (or an undefined one), making the error handling unreliable. The Conditional component introduces a module augmentation plus @ts-ignore in a runtime module, which is brittle for consumers. Some tests are non-hermetic (mixing dist and src) and include stray logging that should be removed.
Additional notes (3)
- Security |
packages/jsx-email/src/cli/commands/build.ts:121-127
relative(outputBasePath, baseDir)is directionally correct vs string replacement, but it can yield..segments whenbaseDiris not actually underoutputBasePath(e.g. misconfiguration or differing realpaths). That would makewritePathescape the intended output directory (path traversal via build configuration), potentially overwriting arbitrary locations underout!.
This is a correctness + safety issue; you should enforce that baseDir is within outputBasePath before using the relative path, and fail fast otherwise.
- Compatibility |
packages/jsx-email/src/renderer/compile.ts:78-83
Usingresolve(originalCwd, path)fixes the previous incorrectresolve('/', path), but this still assumes esbuild’smetafile.outputskeys are relative tooriginalCwd. Ifoutdiris absolute or esbuild returns absolute output keys in some cases,resolve(originalCwd, absolutePath)will ignoreoriginalCwdon POSIX but behaves differently on Windows edge cases.
This can create non-importable result.path values and reintroduce platform-specific breakage.
- Maintainability |
packages/create-mail/moon.yml:36-40
rm -rf dist/generators/templates && cp -r generators distmakescopyidempotent for templates, but it still leavesdist/generatorsitself potentially stale across runs (e.g. removed files outsidetemplates). That can cause subtle generator drift in published artifacts.
Given you already have a copy.templates task, it may be cleaner/safer to fully replace dist/generators in copy and let copy.templates repopulate templates.
Summary of changes
Summary
This PR bundles several next/v3-targeted fixes across jsx-email, create-mail, the preview app, CLI tests, and docs.
Notable changes
-
Preview app path/casing fixes
- Updated imports to lowercase paths (
./logo,../components/sidebar) to avoid case-sensitive FS issues.
- Updated imports to lowercase paths (
-
create-mailscaffold + build tasks- Added
react-domto scaffolddevDependencies. - Made Moon
copytask more idempotent by removingdist/generators/templatesbefore copying.
- Added
-
CLI improvements
email checknow supports--use-preview-propsand passes it through tobuildTemplates.email preview --build-pathnow supports relative paths (resolved fromoriginalCwd) and includes a Windows drive-letter guard for Vite.buildcommand output path logic now usesrelative()instead of string replacement.
-
Rendering pipeline refactor (
Conditional+Raw)Conditionalcomponent now renders a marker element (<jsx-email-cond ...>) and relies on a new rehype plugin (getConditionalPlugin) to emit conditional comments.- Introduced
getRawPluginto replace<jsx-email-raw>wrappers with HASTrawnodes, and moved raw handling into the rehype phase. render()now dynamically importsrehype/rehype-stringifyand runsmove→raw→conditionalplugins.
-
Tests & snapshots
- Added new unit/CLI tests covering conditional closers, raw/conditional interplay, compile path importability,
email check --use-preview-props, and preview build-path behavior. - Updated snapshots to reflect MSO closer changes and new conditional/raw processing.
- Added new unit/CLI tests covering conditional closers, raw/conditional interplay, compile path importability,
-
Docs
- Small doc alignment (import ordering, spacing) and examples.
Component / Package Name:
jsx-email,create-mail,test-cli,apps/preview,docsThis PR contains:
Are tests included?
Breaking Changes?
If yes, please include "BREAKING CHANGES:" in the first commit message body, followed by a description of what is breaking.
List any relevant issue numbers:
mainwithnext/v3#379Description
Combines #385, #386, #387, #388, and #389 into a single PR targeting
next/v3(branch + base).Included changes:
create-mailscaffold addsreact-domto devDeps + docs alignmentapps/preview+test-cli)Conditional/Rawrendering behavior (rehype-based)email checksupports--use-preview-props+ tests + config snapshot stabilizationAdditional small follow-ups required to keep CI green when combining:
create-mailMooncopytask is now idempotent across multiple invocationsNote: This PR intentionally does not close any of the individual PRs above.
Verification
Self-review notes:
reviewChangesflagged a few broader repo concerns (e.g.,pnpm-workspace.yaml,shared/tsconfig.base.json, smoke tests). These files are unchanged in this PR relative toorigin/next/v3.reviewChangessuggested tightening assertions intest/cli/check.test.ts; kept as-is to match existing CLI test style and avoid making the test brittle.