Skip to content

feat: upgrade to mrml v6.0 to allow conditional comments and fixes #179

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

rNoz
Copy link
Contributor

@rNoz rNoz commented Jun 29, 2025

Upgrade to mrml 6.0

This PR updates mjml_nif to use mrml 6.0, which introduces several improvements and breaking changes:

  • Updates mrml from v5 to v6.0 (currently tracked via branch — see two TODOs to update once released).
  • Adds a test verifying that conditional comments can now be handled (a feature I contributed upstream).
    • Removing comments option is just to show they don't interfere.
  • Reviewed all "API breaking" changes and I don't see anything affecting us (e.g., mrml::prelude::render::Options)
  • Validated against real templates in a separate sample project, including previously broken edge cases (e.g. mj-divider rendering, style fixes). See below tests to compare v5 with v6.

Once mrml 6.0 is officially released, only the two TODO lines in Cargo.toml need to be updated.

Feel free to change and merge it while I'm away, since I will take vacations soon.

Tests

Using the sample project, having these tests:

defmodule MjmlNifTest do
  use ExUnit.Case

  @valid_mjml """
  <mjml>
    <mj-body>
      <mj-section>
        <mj-column>
          <mj-text>Hello, world!</mj-text>
          <mj-divider border-color="#F45E43" width="50%" align="left"></mj-divider>
        </mj-column>
      </mj-section>
    </mj-body>
  </mjml>
  """

  @invalid_mjml "not valid mjml at all"

  @mjml_with_conditional_comment """
  <mjml>
    <mj-body>
      <mj-raw>
        <!--[if mso]>
          <p>This is a conditional comment for Outlook</p>
        <![endif]-->
      </mj-raw>
    </mj-body>
  </mjml>
  """

  test "renders valid MJML input into HTML with proper mj-divider styles" do
    assert {:ok, html} = Mjml.to_html(@valid_mjml)
    assert html =~ "<!doctype html>"
    assert html =~ "Hello, world!"
    
    # not using Floki because it fails with conditional comments
    # Match <p ... style="...border-top...">...</p>
    p_tag_regex = ~r/<p\s+[^>]*style="([^"]*border-top[^"]*)"[^>]*>.*?<\/p>/s

    case Regex.run(p_tag_regex, html) do
      [_, style_string] ->
        style_map =
          style_string
          |> String.split(";")
          |> Enum.map(&String.trim/1)
          |> Enum.reject(&(&1 == ""))
          |> Enum.map(fn kv ->
            [k, v] = String.split(kv, ":", parts: 2)
            {String.trim(k), String.trim(v)}
          end)
          |> Map.new()

        assert Map.get(style_map, "margin") == "0px"

      nil ->
        flunk("""
        Could not find a <p> tag with 'border-top' in the style attribute.
        HTML was:
        #{html}
        """)
      end
  end

  test "returns error tuple on invalid MJML input" do
    assert {:error, message} = Mjml.to_html(@invalid_mjml)
    assert is_binary(message)
    assert String.contains?(message, "Error") or String.length(message) > 0
  end

  test "renders conditional comments (added in mrml v6.0.0)" do
    assert {:ok, html} = Mjml.to_html(@mjml_with_conditional_comment)
    assert html =~ "<!--[if mso]>"
    assert html =~ "conditional comment for Outlook"
  end

  test "does not render conditional comments if comments are disabled (added in mrml v6.0.0)" do
    assert {:ok, html} = Mjml.to_html(@mjml_with_conditional_comment, keep_comments: false)
    assert html =~ "<!--[if mso]>"
    assert html =~ "conditional comment for Outlook"
  end
end

And using mjml_nif v5:

❯ mix test apps/mjml_app/test/mjml_nif_test.exs

  1) test does not render conditional comments if comments are disabled (added in mrml v6.0.0) (MjmlNifTest)
     apps/mjml_build/test/mjml_nif_test.exs:75
     match (=) failed
     code:  assert {:ok, html} = Mjml.to_html(@mjml_with_conditional_comment, keep_comments: false)
     left:  {:ok, html}
     right: {:error, "unexpected token in root template at position 38:51"}
     stacktrace:
       test/mjml_nif_test.exs:76: (test)


  2) test renders conditional comments (added in mrml v6.0.0) (MjmlNifTest)
     apps/mjml_build/test/mjml_nif_test.exs:69
     match (=) failed
     code:  assert {:ok, html} = Mjml.to_html(@mjml_with_conditional_comment)
     left:  {:ok, html}
     right: {:error, "unexpected token in root template at position 38:51"}
     stacktrace:
       test/mjml_nif_test.exs:70: (test)


  3) test renders valid MJML input into HTML with proper mj-divider styles (MjmlNifTest)
     apps/mjml_build/test/mjml_nif_test.exs:31
     Assertion with == failed
     code:  assert Map.get(style_map, "margin") == "0px"
     left:  "0px auto"
     right: "0px"
     stacktrace:
       test/mjml_nif_test.exs:52: (test)

.
Finished in 0.1 seconds (0.00s async, 0.1s sync)
4 tests, 3 failures

❯ mix deps.tree | rg '(mjml|mrml|rustler)'
==> mjml_app
mjml_app
│   ├── mjml ~> 5.0 (Hex package)
├── mjml ~> 5.0 (Hex package)
│   └── rustler_precompiled ~> 0.8.1 (Hex package)
│   ├── mjml ~> 5.0 (Hex package)
│   │   └── rustler_precompiled ~> 0.8.1 (Hex package)
│   ├── mjml ~> 5.0 (Hex package)
│   │   └── rustler_precompiled ~> 0.8.1 (Hex package)
│   ├── mjml ~> 5.0 (Hex package)
│   │   └── rustler_precompiled ~> 0.8.1 (Hex package)
│   ├── mjml ~> 5.0 (Hex package)
│   │   └── rustler_precompiled ~> 0.8.1 (Hex package)
│   ├── mjml ~> 5.0 (Hex package)
│   │   └── rustler_precompiled ~> 0.8.1 (Hex package)
│   ├── mjml ~> 5.0 (Hex package)
│   │   └── rustler_precompiled ~> 0.8.1 (Hex package)

And in this PR / mjml v6

❯ mix test test/mjml_nif_test.exs 
===> Analyzing applications...
===> Compiling hackney
Compiling 2 files (.ex)
Running ExUnit with seed: 383087, max_cases: 20

....
Finished in 0.04 seconds (0.00s async, 0.04s sync)
4 tests, 0 failures

❯ mix deps.tree | rg '(mjml|mrml|rustler)'
├── mjml (/Users/rNoz/projects/mjml_nif)
│   ├── rustler >= 0.0.0 (Hex package)
│   └── rustler_precompiled ~> 0.8.1 (Hex package)
├── rustler >= 0.0.0 (Hex package)
└── rustler_precompiled ~> 0.8.0 (Hex package)
    └── rustler ~> 0.23 (Hex package)
    
❯ mix test test/mjml_nif_test.exs   # using my branch
===> Analyzing applications...
===> Compiling hackney
==> mjml
Compiling 3 files (.ex)
Compiling crate mjml_nif in release mode (native/mjml_nif)
   Compiling proc-macro2 v1.0.94
   Compiling unicode-ident v1.0.18
   Compiling heck v0.5.0
   Compiling regex-lite v0.1.6
   Compiling thiserror v2.0.12
   Compiling inventory v0.3.20
   Compiling cfg-if v1.0.0
   Compiling equivalent v1.0.2
   Compiling hashbrown v0.15.2
   Compiling either v1.15.0
   Compiling libloading v0.8.6
   Compiling rustc-hash v2.1.1
   Compiling itertools v0.14.0
   Compiling htmlparser v0.2.1
   Compiling indexmap v2.8.0
   Compiling rustler v0.36.2
   Compiling quote v1.0.40
   Compiling syn v2.0.100
   Compiling thiserror-impl v2.0.12
   Compiling rustler_codegen v0.36.2
   Compiling enum-as-inner v0.6.1
   Compiling mrml v6.0.0 (https://github.com/jdrouet/mrml?branch=release-plz-2025-05-20T12-58-25Z#35276756)
   Compiling mjml_nif v6.0.0-pre (/Users/rNoz/projects/using_mjml_nif/deps/mjml/native/mjml_nif)
Compiling lib/mjml/native.ex (it's taking more than 10s)
    Finished `release` profile [optimized] target(s) in 17.39s
Generated mjml app
==> using_mjml_nif
Compiling 2 files (.ex)
Generated using_mjml_nif app
Running ExUnit with seed: 314126, max_cases: 20

....
Finished in 0.02 seconds (0.00s async, 0.02s sync)
4 tests, 0 failures

❯ mix deps.tree | rg '(mjml|mrml|rustler)' 
├── mjml (https://github.com/rNoz/mjml_nif.git - rnoz/release-v6)
│   ├── rustler >= 0.0.0 (Hex package)
│   └── rustler_precompiled ~> 0.8.1 (Hex package)
├── rustler >= 0.0.0 (Hex package)
└── rustler_precompiled ~> 0.8.0 (Hex package)
    └── rustler ~> 0.23 (Hex package)

@rNoz
Copy link
Contributor Author

rNoz commented Jun 29, 2025

Btw, when #180 is merged, if I am still around, I can update this PR to reflect how not using protocol file:/// will also work (updating tests).

@rNoz rNoz force-pushed the rnoz/release-v6 branch from af6362d to 1fcdd46 Compare July 1, 2025 10:39
@rNoz rNoz force-pushed the rnoz/release-v6 branch from c93a7ed to 8144fb7 Compare July 1, 2025 10:45
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.

1 participant