Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR extends the MJML rendering system to support dual output formats, enabling AST-to-MJML rendering alongside existing AST-to-HTML functionality. This allows for MJML round-trip validation and supports generating Go code from MJML templates.
- Added
OutputFormatenum andRenderMJMLinterface methods for MJML output - Renamed existing
Rendermethods toRenderHTMLfor clarity - Introduced indentation configuration and XML attribute escaping for MJML output
Reviewed Changes
Copilot reviewed 36 out of 36 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| mjml/options/options.go | Added OutputFormat enum and IndentationConfig for MJML formatting |
| mjml/render.go | Introduced dual output support with OutputFormat option and RenderMJML methods |
| mjml/component.go | Added depth tracking and RenderMJML interface methods to component creation |
| mjml/components/base.go | Extended Component interface with RenderMJML and depth tracking methods |
| mjml/html/escape.go | Added XML attribute escaping function for security |
| Multiple component files | Implemented RenderMJML methods and renamed Render to RenderHTML |
| indent := "" | ||
| for i := 0; i < totalLevel; i++ { | ||
| indent += ic.Unit | ||
| } | ||
| return indent |
There was a problem hiding this comment.
Consider using strings.Repeat instead of a loop for better performance when generating indentation strings. Replace the loop with: return strings.Repeat(ic.Unit, totalLevel)
| indent := "" | |
| for i := 0; i < totalLevel; i++ { | |
| indent += ic.Unit | |
| } | |
| return indent | |
| return strings.Repeat(ic.Unit, totalLevel) |
|
|
||
| func (c *MJTextComponent) RenderMJML(w io.StringWriter) error { | ||
| // Write opening tag with indentation | ||
| if _, err := w.WriteString("\n" + c.RenderOpts.Indentation.GetIndent(4) + "<mj-text"); err != nil { |
There was a problem hiding this comment.
The hardcoded indentation level of 4 should be derived from the component's depth instead of being fixed. Use c.GetDepth() to maintain consistent indentation based on the actual component hierarchy.
| if _, err := w.WriteString("\n" + c.RenderOpts.Indentation.GetIndent(4) + "<mj-text"); err != nil { | |
| if _, err := w.WriteString("\n" + c.RenderOpts.Indentation.GetIndent(c.GetDepth()) + "<mj-text"); err != nil { |
|
|
||
| func (c *MJSectionComponent) RenderMJML(w io.StringWriter) error { | ||
| // Write opening tag with indentation | ||
| if _, err := w.WriteString("\n" + c.RenderOpts.Indentation.GetIndent(2) + "<mj-section"); err != nil { |
There was a problem hiding this comment.
The hardcoded indentation level of 2 should be derived from the component's depth instead of being fixed. Use c.GetDepth() to maintain consistent indentation based on the actual component hierarchy.
| if _, err := w.WriteString("\n" + c.RenderOpts.Indentation.GetIndent(2) + "<mj-section"); err != nil { | |
| if _, err := w.WriteString("\n" + c.RenderOpts.Indentation.GetIndent(c.GetDepth()) + "<mj-section"); err != nil { |
|
|
||
| func (c *MJColumnComponent) RenderMJML(w io.StringWriter) error { | ||
| // Write opening tag with indentation | ||
| if _, err := w.WriteString("\n" + c.RenderOpts.Indentation.GetIndent(3) + "<mj-column"); err != nil { |
There was a problem hiding this comment.
The hardcoded indentation level of 3 should be derived from the component's depth instead of being fixed. Use c.GetDepth() to maintain consistent indentation based on the actual component hierarchy.
| if _, err := w.WriteString("\n" + c.RenderOpts.Indentation.GetIndent(3) + "<mj-column"); err != nil { | |
| if _, err := w.WriteString("\n" + c.RenderOpts.Indentation.GetIndent(c.GetDepth()) + "<mj-column"); err != nil { |
|
|
||
| func (c *MJBodyComponent) RenderMJML(w io.StringWriter) error { | ||
| // Write opening tag with indentation | ||
| if _, err := w.WriteString("\n" + c.RenderOpts.Indentation.GetIndent(1) + "<mj-body"); err != nil { |
There was a problem hiding this comment.
The hardcoded indentation level of 1 should be derived from the component's depth instead of being fixed. Use c.GetDepth() to maintain consistent indentation based on the actual component hierarchy.
| if _, err := w.WriteString("\n" + c.RenderOpts.Indentation.GetIndent(1) + "<mj-body"); err != nil { | |
| if _, err := w.WriteString("\n" + c.RenderOpts.Indentation.GetIndent(c.GetDepth()) + "<mj-body"); err != nil { |
|
|
||
| func (c *MJHeadComponent) RenderMJML(w io.StringWriter) error { | ||
| // Opening tag with newline and indentation | ||
| if _, err := w.WriteString("\n" + c.RenderOpts.Indentation.GetIndent(1) + "<mj-head"); err != nil { |
There was a problem hiding this comment.
The hardcoded indentation level of 1 should be derived from the component's depth instead of being fixed. Use c.GetDepth() to maintain consistent indentation based on the actual component hierarchy.
| if _, err := w.WriteString("\n" + c.RenderOpts.Indentation.GetIndent(1) + "<mj-head"); err != nil { | |
| if _, err := w.WriteString("\n" + c.RenderOpts.Indentation.GetIndent(c.GetDepth()) + "<mj-head"); err != nil { |
|
|
||
| func (c *MJTitleComponent) RenderMJML(w io.StringWriter) error { | ||
| // Opening tag with newline and indentation | ||
| if _, err := w.WriteString("\n" + c.RenderOpts.Indentation.GetIndent(2) + "<mj-title"); err != nil { |
There was a problem hiding this comment.
The hardcoded indentation level of 2 should be derived from the component's depth instead of being fixed. Use c.GetDepth() to maintain consistent indentation based on the actual component hierarchy.
| if _, err := w.WriteString("\n" + c.RenderOpts.Indentation.GetIndent(2) + "<mj-title"); err != nil { | |
| if _, err := w.WriteString("\n" + c.RenderOpts.Indentation.GetIndent(c.GetDepth()) + "<mj-title"); err != nil { |
This PR extends the MJML rendering system to support dual output formats, enabling AST-to-MJML rendering alongside the existing AST-to-HTML functionality. This allows for MJML round-trip validation and opens possibilities for generating Go code from MJML templates.
OutputFormatenum with HTML and MJML optionsRenderMJMLmethod to all component interfacesRendermethods toRenderHTMLfor clarityMotivation
This addition extends the existing implementation with back-to-back rendering of MJML=>AST=>HTML/MJML.
Why do we want the AST to render MJML back? Isn't the MJML content sort of the input that we are given anyway?
In most cases, yes, but imagine the following: having the MJML AST in Go code opens up new possibilities, one of which is generating Go code out of some MJML, and using the Go code as the one and only way we render email templates. This would be great, but until now, it wouldn't be able to verify and validate the MJML code out of the Go code. We either had to store both versions (leads to duplication and possible inconsistencies) or rely on never changing the generated Go code. If you did want to change it, however (e.g., include some loops, conditional logic, etc.), you would need a way to see the MJML this new Go code would generate - it's just way easier to debug MJML code than having to debug its generated HTML output.