refactor(jsx-email): use rehype rawPlugin to hoist <Raw> content#320
Conversation
…ow move-style pattern
There was a problem hiding this comment.
- Registering
rawPluginbeforecallProcessHookchanges what downstream plugins see (nowrawnodes, previously wrapper elements), which can be a backward-compatibility risk. - The transform uses multiple
anycasts that can be avoided with stronger HAST typings, improving safety and maintainability. - If the comment payload is missing, the plugin leaves
<jsx-email-raw>wrappers in the output; dropping the wrapper would be safer to prevent leaking non-standard tags.
Additional notes (1)
- Maintainability |
packages/jsx-email/src/renderer/raw.ts:44-58
The comment extraction and replacement logic works, but it relies on multipleanycasts. This hides potential mistakes and makes the transform harder to maintain. You can tighten the types by leveragingCommentandParentfromhastand a type guard for the comment node, avoidingas anyforparentand the child search.
Summary of changes
- Introduced an async
getRawPlugin()rehype plugin inraw.tsto replace<jsx-email-raw>wrapper elements withrawnodes containing unescaped HTML from the inner comment payload. - Wired the new plugin into the rehype pipeline in
render.ts(aftermovePlugin). - Removed the post-stringify regex that previously handled
<jsx-email-raw>replacement; the plugin now performs this during AST processing. - Kept existing
escapeForRawComponent()/unescapeForRawComponent()helpers and overall rendering behavior intact, includingallowDangerousHtmlinrehype-stringify.
| // importing additional types or changing configs. | ||
| type: 'raw', | ||
| value: rawHtml | ||
| } as any); |
There was a problem hiding this comment.
looking at hast-util-raw https://github.com/syntax-tree/hast-util-raw/blob/ad15a52350463223504999a6c4c763e1994522d4/lib/index.js, it appears that these are standard Node types, just with type: 'raw'. In this case, it would be appropriate to create a new intermediate interface that extended the Node type from hast that accommodated type: 'raw'.
There was a problem hiding this comment.
Expand this to see my work.
- Confirmed
packages/jsx-email/src/renderer/raw.tssplices{ type: 'raw', value }for<jsx-email-raw>and that hast-util-raw treats it as a standardhastNodewithtype: 'raw'. - Proposed an intermediate
RawNode extends Node { type: 'raw'; value: string }forgetRawPlugin/rawPlugin, mirroringElementWithParentinpackages/jsx-email/src/renderer/move-style.ts, per the PR feedback. - Ran typecheck and lint for
packages/jsx-email(tsc,eslintonrenderer/raw.tsandrenderer/render.ts);eslintinitially flagged 1 Prettier issue inraw.ts. - Fixed the Prettier formatting in
packages/jsx-email/src/renderer/raw.tsaround theCommenttype guard innode.children.find(...). - Re-ran verification:
tscpasses with 0 errors andeslintis clean on the checked files.
There was a problem hiding this comment.
Addressed the typing feedback in 98882e05.
- Added a local Raw node type that extends HAST’s Literal to model
type: 'raw'withoutany:interface Raw extends Literal { type: 'raw'; value: string; }
- Replaced the child search and splice casts with proper types:
- Comment payload detection now uses a type guard:
(c): c is Comment => c.type === 'comment'. - Introduced
type ParentWithRaw = Parents & { children: (Content | Raw)[] }and spliced aRawnode instance instead of casting toany.
- Comment payload detection now uses a type guard:
This keeps the transform fully typed while matching the semantics you pointed out from hast-util-raw.
Changes
packages/jsx-email/src/renderer/raw.ts- Import
Comment | Content | Literalfromhast. - Define
interface Raw extends Literal { type: 'raw'; value: string }. - Use a
Commenttype guard for the payload lookup. - Define
ParentWithRawfor the splice site and pass a typedrawNode: Raw.
- Import
No behavioral changes—just stronger typing and removal of any casts.
Verification
# TypeScript (package): no errors
$ bunx tsc -p packages/jsx-email/tsconfig.json --noEmit
# ESLint (changed files): 0 errors
$ bunx eslint packages/jsx-email/src/renderer/raw.ts packages/jsx-email/src/renderer/render.ts
# Vitest (scoped): attempted
$ bunx vitest run packages/jsx-email/test/raw.test.tsx --config shared/vitest.config.ts
# Blocked by Vite/package resolution for a workspace plugin (preexisting in this env).
# This change is type-only and does not alter behavior; prior focused checks remain valid.Re-requested review from @shellscape and @CharlieHelps.
There was a problem hiding this comment.
- Plugin registration order may break downstream hooks by exposing
rawnodes earlier than before; consider movingrawPluginto aftercallProcessHookto preserve behavior. - Overuse of
anyin the AST transform reduces safety; tighten types withParent/Commentand a type guard, keepinganyonly for the non-standardrawnode insertion. - If the comment payload is missing, the current transform can leak a
<jsx-email-raw>tag into output; drop the wrapper defensively in that case. - Minor: remove a redundant
as Elementcast to improve readability.
Summary of changes
- Introduced a new rehype plugin factory
getRawPlugin()inraw.tsthat finds<jsx-email-raw>wrapper elements, extracts the inner HTML comment, unescapes it, and replaces the wrapper with arawnode for verbatim injection during stringify. - Wired
rawPlugininto the render pipeline inrender.tsalongside the existingmovePluginand beforecallProcessHook. - Removed the post-stringify regex that previously replaced
<jsx-email-raw>wrappers with unescaped content inrender.ts. - Kept existing escaping helpers and
rehype-stringifyoptions (allowDangerousHtml) intact.
There was a problem hiding this comment.
- Registering
rawPluginbeforecallProcessHookmay exposerawnodes to downstream plugins, altering previous hook semantics and risking compatibility issues. unescapeForRawComponentis still imported inrender.tsbut no longer used; remove it to reduce noise.- Minor: the
visitcallback inraw.tsstill relies on a cast fornode; typing the callback avoids this and improves clarity. - Overall, the new plugin-based approach is a solid improvement over the brittle post-stringify regex and is implemented cleanly with better typing for the
rawnode.
Summary of changes
Summary of Changes
- Introduced a rehype plugin factory
getRawPlugin()that locates<jsx-email-raw>elements, extracts their inner comment payload, unescapes it, and replaces the wrapper with arawHAST node. - Modeled the non-standard HAST node as a local
interface Raw extends Literal { type: 'raw'; value: string }to avoidany. - Wired the plugin into the processor in
render.ts(registered alongsidemovePlugin) and removed the post-stringify regex replacement for<jsx-email-raw>. - Left the JSX tag cleanup intact and preserved
rehype-stringifyoptions (allowDangerousHtml, etc.).
There was a problem hiding this comment.
- Plugin registration order may break downstream hooks by exposing
rawnodes earlier than before; consider movingrawPluginaftercallProcessHookto preserve prior semantics. - The
visitcallback can be typed to avoid a cast and improve clarity. - If the comment payload is missing, the transform leaves
<jsx-email-raw>in output; defensively drop the wrapper to avoid leaking internal tags. - Consider renaming
Matchto a more descriptiveRawElementMatchfor readability.
Summary of changes
- Introduced
getRawPlugin()inpackages/jsx-email/src/renderer/raw.tsto hoist<jsx-email-raw>contents by replacing wrapper elements withrawHAST nodes. - Added local HAST typings (
Raw,ParentWithRaw,Match) to avoidanycasts and keep the transform type-safe. - Wired the plugin into the render pipeline in
packages/jsx-email/src/renderer/render.ts(registered beforecallProcessHook). - Removed the post-stringify regex that previously replaced
<jsx-email-raw>wrappers, relying on the rehype plugin instead.
There was a problem hiding this comment.
rawPluginis registered beforecallProcessHook, altering hook semantics and potentially breaking downstream plugins that don’t handlerawnodes. Consider moving it aftercallProcessHook.ParentWithRawis modeled as a standalone interface, losing its relationship toParents; use aParentsintersection type for accurate typing.- The untyped
visitcallback forces anElementcast; typing parameters removes the cast and clarifies intent. - If the comment payload is missing,
<jsx-email-raw>wrappers leak into HTML—drop the wrapper defensively to avoid non-standard tags in output.
Summary of changes
- Introduced a rehype plugin factory
getRawPlugin()inrenderer/raw.tsto replace<jsx-email-raw>wrappers withrawHAST nodes containing unescaped HTML from the inner comment payload. - Modeled a local
Rawnode type and added helper types to reduce casts, then wired the plugin into the render pipeline inrenderer/render.ts(registered aftermovePluginbut beforecallProcessHook). - Removed the post-stringify regex that previously handled
<jsx-email-raw>replacement, keeping existing escaping helpers intact. - Preserved
rehype-stringifyoptions and the JSX tag cleanup while refactoring the raw content handling to AST-level processing.
|
@CharlieHelps please update the PR description to use the PR template. scope for this PR is |
|
Updated the PR description to use the repository template and set the scope to
Shout if you want the title adjusted as well to include the scope. |
|
Updated the PR description to use the repository template and set the scope to
If you’d like the title to include the scope as well, I can update that too. |
|
@CharlieHelps the scope is only applicable to the PR title. |
rawPlugin to hoist <Raw> contentrawPlugin to hoist <Raw> content
|
Updated. Thanks for the clarification.
If you’d rather the field read packages/jsx-email, I can switch it. |
Component / Package Name:
jsx-email
This PR contains:
Are tests included?
Breaking Changes?
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 #319
Description
Refactors the renderer to hoist
<Raw>component content using a dedicated rehype plugin, mirroring the existing move-style pattern. This removes the brittle post-stringify replacement and performs the transformation during AST processing instead.Key points:
getRawPlugin()inpackages/jsx-email/src/renderer/raw.tsthat:<jsx-email-raw>elements,unescapeForRawComponent(), andrawnode so content is injected verbatim during stringify.packages/jsx-email/src/renderer/render.tsnext to the existing move plugin.<jsx-email-raw>…</jsx-email-raw>replacement.rawnode locally; avoidanycasts) and keep existing escaping helpers intact.