fix(jsx-email): implement Conditional via rehype (no regex); preserve <Raw>; forbid nested <Conditional>

[charliecreates]

Summary

• Replace regex-based handling in Conditional renderer with a rehype-based transform.
• Ensure content is preserved verbatim inside Conditional blocks (including line breaks/whitespace).
• Disallow nested usage and surface a clear error message.

Why

• Addresses review feedback on fix(jsx-email): preserve Raw within Conditional via rehype; forbid nested Conditional #323: regular expressions are brittle and incorrect for HTML/AST transformation.
• Using rehype aligns with the rest of the pipeline and guarantees correct escaping/serialization behavior for complex children.

What changed

• packages/jsx-email/src/renderer/conditional.ts
  • Removed regex parsing/manipulation.
  • Implemented a rehype transform to construct the MSO conditional comment wrapper and pass through children unescaped.
  • Added a guard to detect nested and throw with a descriptive message.
• Tests
  • Added minimal snapshot for inside .
  • Added coverage for correct <![endif]--> closer emission.
  • Added a negative test ensuring nested throws.

Behavior

• For non-Raw children, behavior remains unchanged aside from more robust serialization via rehype.
• For inside , content is now preserved exactly as authored (no escaping, line breaks maintained).
• Nested now throws an error with guidance.

Migration

• If you previously nested components, refactor to a single Conditional with combined logic or adjacent conditionals; nested usage is no longer supported and will throw at render time.

Notes

• This directly addresses the review comment in fix(jsx-email): preserve Raw within Conditional via rehype; forbid nested Conditional #323 regarding the removal of regex usage in the Conditional renderer.

[charliecreates]

• Nested-conditional detection is incomplete: when the outer <Conditional> renders as a single comment node (e.g., mso={true}), inner <Conditional>s are stringified into comment text and bypass the rehype guard, contradicting the stated “forbid nested” behavior.
• The nested-check in the conditional plugin performs redundant traversals, which can be simplified to a single DFS that carries an inCond flag.
• The “group-by-parent then splice descending” logic is duplicated in two plugins and should be factored into a shared helper to reduce maintenance risk.
• The legacy conditional parsing looks solid and avoids regex; no issues found with the raw payload preservation path for the mso case; the !mso downlevel-revealed case is correctly handled via unwrap + prior raw lifting.

Additional notes (3)

• Maintainability | packages/jsx-email/src/components/conditional.tsx:52-53
  The nested-conditional guard only runs at the rehype stage by scanning for child <jsx-email-cond> elements. When the outer <Conditional> renders as a single comment wrapper (e.g., mso={true}), any inner <Conditional> is already stringified into the comment text and won’t exist as an element in the HAST. That means the current guard won’t fire for this common case, and nested usage will slip through—contradicting the PR’s stated guarantee that nested <Conditional> is disallowed.

Add a React-level pre-serialization check here (before jsxToString) to catch nested <Conditional> in all cases (including comment-only shapes).

• Maintainability | packages/jsx-email/src/renderer/raw.ts:53-64
  The “group-by-parent then splice in descending index order” pattern is duplicated here and in renderer/conditional.ts. This repetition makes future fixes easy to miss in one location and hardens maintenance.

• Maintainability | packages/jsx-email/src/renderer/raw.ts:80-87
  Nice improvement to splice by parent in descending index order. As a small enhancement for debuggability, consider including node position info (if available) in the warning to aid users in locating the problematic <Raw> wrapper during development.

Summary of changes

• components/conditional.tsx
  • Minor refactor: reordered prop destructuring, removed unnecessary fragment around Suspense.
  • Preserved legacy jsxToString path for children serialization and retained comment/string helpers.
• renderer/conditional.ts
  • Added a rehype plugin to unwrap <jsx-email-cond> wrappers and forbid nested conditionals.
  • Implemented parseLegacyConditional to convert the legacy single-comment placeholder shape into a raw node, preserving <Raw> payload verbatim.
  • Grouped splices by parent and applied in descending index order to avoid index invalidation.
• renderer/raw.ts
  • Aligned mutation strategy with conditional plugin: group by parent and splice in descending index. Added a dev-time fallback warning when the expected comment child is missing.
• renderer/render.ts
  • Integrated getConditionalPlugin() after the Raw plugin; removed regex-based <jsx-email-cond> cleanup.
• tests
  • Updated snapshot to assert that the final HTML preserves the <Raw> content inside the MSO conditional block.

[charliecreates]

The nested-check performs a full recursive scan (hasNestedConditional) for every matched wrapper and also walks the tree separately, which is O(N^2) in the worst case. You can make a single DFS that tracks whether you’re already inside a <jsx-email-cond> and throw immediately if you encounter another—simpler and more efficient.

Suggestion

You can replace hasNestedConditional + walk with a single-pass visitor that carries an inCond flag and collects matches:

const matches: Array<{ index: number; node: Element; parent: Parents }> = [];
const visit = (parent: Parents, inCond: boolean): void => {
  const children = ((parent as any).children || []) as Content[];
  for (let i = 0; i < children.length; i++) {
    const child = children[i];
    if (isElement(child)) {
      const isCond = child.tagName === 'jsx-email-cond';
      if (inCond && isCond) {
        throw new RangeError(
          'jsx-email: Nested <Conditional> is not supported. Flatten your conditionals into a single block.'
        );
      }
      if (isCond) matches.push({ index: i, node: child as Element, parent });
      visit(child as Parents, inCond || isCond);
    }
  }
};

visit(tree as unknown as Parents, false);

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