Skip to content

Conversation

@charliecreates
Copy link
Contributor

Component / Package Name:

jsx-email, create-mail, test-cli, apps/preview, docs

This PR contains:

  • bugfix
  • feature
  • refactor
  • documentation
  • other

Are tests included?

  • yes (bugfixes and features will not be merged without tests)
  • no

Breaking Changes?

  • yes (breaking changes will not be merged unless absolutely necessary)
  • no

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:

Description

Combines #385, #386, #387, #388, and #389 into a single PR targeting next/v3 (branch + base).

Included changes:

Additional small follow-ups required to keep CI green when combining:

  • create-mail Moon copy task is now idempotent across multiple invocations
  • Updated config-related snapshots to match the stabilized test environment

Note: This PR intentionally does not close any of the individual PRs above.

Verification

# Build: success
$ pnpm moon jsx-email:build
$ pnpm moon create-mail:build
$ pnpm moon run :build --query "project~plugin-*"

# Lint: 0 errors, 31 warnings
$ pnpm moon run repo:lint

# Unit tests: 188 passed (45 files)
$ FORCE_COLOR=1 pnpm moon run jsx-email:test

# TypeScript: success
$ pnpm moon run jsx-email:tsc

# CLI tests: 5 passed (5 files)
$ pnpm moon test-cli:test.run

Self-review notes:

  • reviewChanges flagged a few broader repo concerns (e.g., pnpm-workspace.yaml, shared/tsconfig.base.json, smoke tests). These files are unchanged in this PR relative to origin/next/v3.
  • reviewChanges suggested tightening assertions in test/cli/check.test.ts; kept as-is to match existing CLI test style and avoid making the test brittle.

Copy link
Contributor Author

@charliecreates charliecreates bot left a 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 when baseDir is not actually under outputBasePath (e.g. misconfiguration or differing realpaths). That would make writePath escape the intended output directory (path traversal via build configuration), potentially overwriting arbitrary locations under out!.

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
    Using resolve(originalCwd, path) fixes the previous incorrect resolve('/', path), but this still assumes esbuild’s metafile.outputs keys are relative to originalCwd. If outdir is absolute or esbuild returns absolute output keys in some cases, resolve(originalCwd, absolutePath) will ignore originalCwd on 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 dist makes copy idempotent for templates, but it still leaves dist/generators itself potentially stale across runs (e.g. removed files outside templates). 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.
  • create-mail scaffold + build tasks

    • Added react-dom to scaffold devDependencies.
    • Made Moon copy task more idempotent by removing dist/generators/templates before copying.
  • CLI improvements

    • email check now supports --use-preview-props and passes it through to buildTemplates.
    • email preview --build-path now supports relative paths (resolved from originalCwd) and includes a Windows drive-letter guard for Vite.
    • build command output path logic now uses relative() instead of string replacement.
  • Rendering pipeline refactor (Conditional + Raw)

    • Conditional component now renders a marker element (<jsx-email-cond ...>) and relies on a new rehype plugin (getConditionalPlugin) to emit conditional comments.
    • Introduced getRawPlugin to replace <jsx-email-raw> wrappers with HAST raw nodes, and moved raw handling into the rehype phase.
    • render() now dynamically imports rehype/rehype-stringify and runs moverawconditional plugins.
  • 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.
  • Docs

    • Small doc alignment (import ordering, spacing) and examples.

@charliecreates charliecreates bot removed the request for review from CharlieHelps December 10, 2025 03:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants