Skip to content

Conversation

@charliecreates
Copy link
Contributor

@charliecreates charliecreates bot commented Dec 1, 2025

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

Description

Ports the Windows static-asset path fix from #365 onto the next/v3 branch so email preview serves files correctly on Windows when --assets globs include backslash-separated paths.

On main, the fix normalized the globs passed to globby using Vite's normalizePath() helper. This PR applies the same normalization in the next/v3 CLI implementation:

  • Updates packages/jsx-email/src/cli/vite-static.ts to import normalizePath from vite.
  • Normalizes the configured paths before passing them to globby, matching the behavior from vite-static.mts on main.

Runtime behavior for non-Windows platforms is unchanged; on Windows, static asset paths are now consistently treated as POSIX-style globs that globby can match.

Verification

# Build plugins used by jsx-email
$ pnpm moon run plugin-inline:build plugin-minify:build plugin-pretty:build

# Build jsx-email (v3 branch)
$ pnpm moon run jsx-email:build

# Lint (warnings only; no errors)
$ pnpm moon run repo:lint

# Tests for packages/jsx-email (v3 config)
$ pnpm moon run jsx-email:test

# TypeScript: jsx-email (preexisting failure)
$ pnpm moon run jsx-email:tsc
  • plugin-inline:build, plugin-minify:build, and plugin-pretty:build all completed successfully.
  • jsx-email:build completed successfully.
  • repo:lint completed with 31 existing warnings (in apps, docs, and tests) but no errors; none are related to vite-static.ts.
  • jsx-email:test passed: 46 test files, 190 tests.
  • jsx-email:tsc currently fails with a preexisting TS2717 error in packages/jsx-email/src/components/conditional.tsx about duplicate 'jsx-email-cond' declarations. This failure reproduces on origin/next/v3 before applying the vite-static.ts change and is unchanged by this PR.

Self-review notes

The automated self-review surfaced a number of maintainability items (pnpm workspace flags, shared tsconfig changes, CLI test reshaping, Playwright/diffable-html versions, smoke snapshots, etc.) that do not appear in this branch's diff when compared to origin/next/v3.

  • git diff --stat origin/next/v3...HEAD shows only one touched file:
    • packages/jsx-email/src/cli/vite-static.ts
  • No changes were made to pnpm-workspace.yaml, shared/tsconfig.base.json, shared/tsconfig.eslint.json, tests, or lockfiles as part of this PR.

Those self-review items correspond to earlier next/v3 alignment work (for example #355, #358, #359, #360, #361) and are intentionally left as-is here to keep this change narrowly scoped to porting the Windows asset path fix from #365.

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.

No issues were found in the modified code. The change is minimal, aligned with the stated goal, and follows the existing patterns and repository instructions (notably [R1] about keeping diffs small and scoped). The use of normalizePath directly in the globby call is appropriate for cross-platform glob handling and does not introduce obvious performance or maintainability concerns.

Additional notes (1)
  • Maintainability | packages/jsx-email/src/cli/vite-static.ts:50-53
    Using normalizePath on each configured glob before passing them to globby is the right way to make these patterns portable across Windows and POSIX, and it mirrors the existing pattern on main. The transformation is local, side-effect free, and doesn’t alter runtime behavior for already-POSIX-style globs.

One potential follow-up (not required here per [R1]) would be to encapsulate the normalization into a small helper to make it obvious to future readers that paths are always treated as Vite-normalized POSIX patterns when used with globby, but given this PR’s narrow scope, keeping the inline map is acceptable and consistent with the existing minimal style.

Summary of changes

Summary of Changes

  • Updated the Vite import in packages/jsx-email/src/cli/vite-static.ts to include normalizePath as a value import while keeping PluginOption and ViteDevServer as type imports.
  • Normalized configured paths before passing them to globby by mapping each path through vite's normalizePath() helper.
  • This aligns the next/v3 CLI static asset handling with the existing behavior on main, improving Windows compatibility for backslash-separated asset globs while leaving non-Windows behavior unchanged.

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