Skip to content

Conversation

@charliecreates
Copy link
Contributor

Component / Package Name:

create-mail / jsx-email / preview / test-cli

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:

Resolves #384
Resolves #381
Resolves #366
Refs #379
Refs #390

Supersedes #385
Supersedes #387

Description

Combines the work from #385 and #387 into a single PR targeting next/v3.

  • create-mail: scaffolded projects now include react-dom in devDependencies.
  • Docs: updated examples to match the create-mail scaffolds.
  • email preview --deploy: ported --build-path / static asset fixes and added a CLI regression test for a relative --build-path.

Verification

# Install
$ pnpm install --frozen-lockfile

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

# Lint: 0 errors (31 warnings)
$ pnpm exec moon run repo:lint

# Vitest (packages/jsx-email): 184 tests passed (41 files)
$ cd packages/jsx-email && PATH="$HOME/.local/share/mise/installs/node/20.19.0/bin:$PATH" FORCE_COLOR=1 pnpm exec vitest --config ../../shared/vitest.config.ts

# CLI tests (test/cli): 4 tests passed (4 files)
$ pnpm install --frozen-lockfile
$ PATH="$HOME/.local/share/mise/installs/node/20.19.0/bin:$PATH" FORCE_COLOR=1 pnpm exec moon test-cli:test.run
Self-review notes

The self-review tool surfaced maintainability suggestions in files not modified by this PR; leaving them unchanged to keep scope limited to combining #385 and #387.

  • scripts/ci-preview-setup.sh:14-32
  • shared/tsconfig.base.json:1-16
  • shared/tsconfig.eslint.json:1-16 (file is already absent on next/v3)
  • shared/vitest.config.ts:1-10
  • test/cli/create-mail.test.ts:1-34, test/cli/create-mail.test.ts:36-47, test/cli/create-mail.test.ts:49-48
  • test/smoke/tests/smoke.test.ts:1-76
  • pnpm-workspace.yaml:1-9
  • recipes/import-css/package.json:1-20

Reorder imports and adjust spacing in documentation examples to match the updated create-mail scaffold conventions, keeping import groupings consistent across docs.

Refs: #384
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 Windows drive-letter guard in preview.ts appears to check the wrong path (CLI --build-path instead of the temp build directory), which can make the workaround ineffective or produce false failures. The new CLI regression test also includes unconditional logging that will add noise to CI output. Addressing these will improve correctness and maintainability of the preview deploy path fixes.

Additional notes (2)
  • Maintainability | packages/jsx-email/src/cli/commands/preview.ts:53-53
    The Vite config resolve.alias still points @jsxemailbuild at buildPath (the temp preview build dir), but --build-path now writes the deploy bundle to outDir. This is likely intentional (alias is for runtime preview), but it creates an easy-to-miss divergence: users may expect @jsxemailbuild to correspond to the deploy output when running with --build-path. If any code paths (or future changes) reuse this config for deploy builds, it can accidentally reference the temp directory instead of the deploy directory.

  • Compatibility | packages/jsx-email/src/cli/vite-static.ts:50-56
    Path normalization is applied when finding files (globby(paths.map(normalizePath))), but later request handling compares URL paths against filesystem paths. If files contains normalized POSIX paths on Windows, ensure downstream comparisons (e.g., endsWith, includes, or direct equality) also normalize consistently, otherwise requests may fail to match static files on Windows.

Summary of changes

Summary of changes

  • Preview app imports

    • Normalized component import casing in the preview app (Logo/Sidebar) by switching to ./logo and ../components/sidebar.
  • Docs updates

    • Reordered imports and spacing in multiple docs to align examples with the create-mail scaffold conventions.
  • create-mail scaffold

    • Added react-dom to scaffolded devDependencies in packages/create-mail/generators/package.json.mustache.
  • email preview --deploy/--build-path fixes

    • --build-path now defaults to ./.deploy and relative paths are resolved from the original working directory via originalCwd.
    • Added Windows-specific guard to fail fast when Vite root and temp build directory are on different drive letters.
  • Static asset handling

    • Normalized paths before globby() in vite-static.ts using Vite’s normalizePath().
  • Tests

    • Extended create-mail CLI test to assert react and react-dom are in devDependencies and not in dependencies.
    • Added a new CLI regression test for relative --build-path behavior (and a Windows-specific error case).

Comment on lines +23 to +35
await execa({ cwd: __dirname, shell: true })`email create BatmanEmail --out ${templatePath} `;
const { stdout } = await execa({
cwd: __dirname,
shell: true
})`email preview ${templatePath} --build-path ${outRelational}`;

console.log(stdout);

await access(join(outAbsolute, 'index.html'));

const html = await readFile(join(outAbsolute, 'index.html'), 'utf8');
expect(html).toContain('<div id="root"></div>');
}, 60e3);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new CLI test prints command output unconditionally via console.log(stdout). This adds noise to test runs/CI logs and can mask failures by making logs harder to read. Tests should only emit output when failing, or behind a debug flag.

Suggestion

Remove the console.log(stdout) line, or only print it when an assertion fails (e.g., wrap assertions in a try/catch and log on catch), or gate it behind an env var like DEBUG_CLI_TESTS.

Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion.

@charliecreates charliecreates bot removed the request for review from CharlieHelps December 10, 2025 02:40
@shellscape shellscape closed this Dec 10, 2025
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