fix(jsx-email): port build/compile path fixes to v3

[charliecreates]

Component / Package Name:

jsx-email (next/v3)

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 #382

Description

Ports two output-path correctness fixes from main to next/v3:

• email build: replace the string replace hack (baseDir.replace(outputBasePath, '')) with path.relative(outputBasePath, baseDir) so nested templates write under --out correctly.
• compile(): resolve esbuild metafile output keys against the original CWD (not resolve('/', key)), keeping .meta.json next to outputs and preserving importability.

Also adds a small, non-snapshot unit test that compiles a nested entrypoint and dynamically imports it via the returned CompileResult.path.

Verification

# Lint (warnings only)
$ pnpm moon run repo:lint.js

# Plugin builds (required for jsx-email config tests)
$ unset NO_COLOR && nvm exec 20.19.0 pnpm moon run :build --query "project~plugin-*"

# Unit tests
$ unset NO_COLOR && FORCE_COLOR=1 nvm exec 20.19.0 pnpm moon run jsx-email:test

# CLI tests
$ unset NO_COLOR && FORCE_COLOR=1 nvm exec 20.19.0 pnpm moon run test-cli:test.run

Self-review notes

• The self-review tool surfaced a number of pre-existing next/v3 vs main diffs (e.g. pnpm-workspace.yaml, shared/tsconfig.*, smoke tests) that are not modified by this PR and are intentionally left out of scope for charlie: port main build/compile path fixes to next/v3 #382.

[charliecreates]

The build path change correctly avoids brittle string replacement, but it introduces a potential directory traversal/escape if path.relative() yields .. segments when baseDir is not actually under outputBasePath. The new compile-path test is valuable, but it currently validates only importability and not that outputs are placed under outDir, leaving room for regressions in the path/location guarantee. Addressing these would harden the CLI behavior and make the test better reflect the reported bug.

Additional notes (1)

• Maintainability | packages/jsx-email/src/renderer/compile.ts:78-84
  This uses .filter<CompileResult>(Boolean as any), which works but suppresses type safety and can hide accidental truthy/falsey issues in the future. Since this function is already doing a structural mapping, it’s easy to avoid any entirely by building the array with flatMap or by using a typed predicate.

Given the project’s emphasis on correctness, removing as any would improve maintainability without changing runtime behavior.

Summary of changes

What changed

email build output path handling

• Updated packages/jsx-email/src/cli/commands/build.ts to compute a relativeBaseDir using path.relative(outputBasePath, baseDir).
• Replaced the prior string-based path manipulation (baseDir.replace(outputBasePath, '')) with join(out!, relativeBaseDir, templateName) to correctly place nested templates under --out.

compile() output path resolution

• Updated packages/jsx-email/src/renderer/compile.ts to resolve esbuild metafile.outputs keys against originalCwd (resolve(originalCwd, path)) instead of resolve('/', path), keeping returned CompileResult.path importable and aligned with where outputs are actually written.

Added coverage

• Added packages/jsx-email/test/render/compile-path.test.ts to compile a nested TSX entrypoint and verify the returned CompileResult.path can be dynamically imported (with a Windows file:// workaround).

[charliecreates]

path.relative(outputBasePath, baseDir) can return paths like .. when baseDir is outside outputBasePath (or when one is realpathed and the other isn’t). That would cause join(out!, relativeBaseDir, templateName) to write outside of out (directory traversal) or into unexpected locations. The old replace hack was also flawed, but it typically wouldn’t introduce .. segments.

Given this is a CLI that writes to disk based on user-provided inputs/flags, it’s worth hardening: either ensure baseDir is actually under outputBasePath (after normalizing/realpathing both), or clamp/fallback to '' when relativeBaseDir escapes.

Suggestion

Consider validating that the computed relative path stays within out.

For example:

import { relative, isAbsolute, sep, resolve as resolvePath } from 'node:path';

const relativeBaseDir = outputBasePath
  ? relative(outputBasePath, baseDir)
  : '';

// Prevent writing outside of outDir due to `..` segments or absolute rels
const safeRelativeBaseDir =
  relativeBaseDir &&
  !relativeBaseDir.startsWith(`..${sep}`) &&
  relativeBaseDir !== '..' &&
  !isAbsolute(relativeBaseDir)
    ? relativeBaseDir
    : '';

const writePath = outputBasePath
  ? join(out!, safeRelativeBaseDir, templateName)
  : join(out!, templateName);

If you’d like, reply with "@CharlieHelps yes please" and I can add a commit with this guard (plus a small unit test around the escaping case if the code is easily exercised).

[charliecreates]

The new test verifies importability, but it never asserts that the compiled output landed under outDir (the core behavior this change is meant to preserve/fix). A regression could still return an importable path while writing meta/output somewhere else (especially if originalCwd changes in the future).

Also, dynamically importing result.path without ensuring it’s a URL on Windows is handled, but on non-Windows this assumes Node will always accept the absolute filesystem path string. That’s generally fine in modern Node, but the test would be more robust if it always used pathToFileURL (and kept the assertion focused on the returned value being correct).

Suggestion

Tighten the test so it asserts both location and importability.

Example adjustments:

import { pathToFileURL } from 'node:url';
import { resolve, relative, sep } from 'node:path';

// ...
const results = await compile({ files: [entryPoint], outDir });
const result = results[0];
if (!result) throw new Error('Expected compile to return at least one output');

// Assert it is inside outDir (no traversal)
const relToOut = relative(outDir, result.path);
expect(relToOut === '' || (!relToOut.startsWith(`..${sep}`) && relToOut !== '..')).toBe(true);

// Always import via file URL for cross-platform consistency
const mod = await import(pathToFileURL(result.path).toString());
expect(typeof mod.Template).toBe('function');

Reply with "@CharlieHelps yes please" if you want me to add a commit that strengthens the test in this way.

[charliecreates]

The test creates a .tsx file containing JSX (<h1>...) but doesn’t include an explicit React import or JSX runtime directive. If your compile pipeline relies on project defaults it may be fine, but it makes this test more brittle across config changes (e.g. switching jsx/jsxImportSource or esbuild jsx settings).

To keep the test focused on path correctness (not JSX configuration), consider using a no-JSX module or an explicit runtime import/directive so the test fails only for the intended reason.

Suggestion

Make the entrypoint independent of JSX settings by exporting plain JS/TS:

await writeFile(
  entryPoint,
  `export const Template = ({ name }: { name: string }) => "Hello " + name;\n`,
  'utf8'
);

If you specifically want JSX coverage, add an explicit directive/import appropriate to your expected runtime (e.g. /** @jsxImportSource react */) so it’s resilient to config drift.

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