Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Root HTML comment fails to parse #409

Closed
moomerman opened this issue Apr 16, 2024 · 6 comments
Closed

Root HTML comment fails to parse #409

moomerman opened this issue Apr 16, 2024 · 6 comments
Labels
bug Something isn't working

Comments

@moomerman
Copy link

moomerman commented Apr 16, 2024 •
edited
Loading

I'm happily using this library via Elixir and have hit an issue rendering the following template:

<!-- comment -->
<mjml>
  <mj-body>
    <mj-section>
      <mj-column>
        <mj-text>Hello World</mj-text>
      </mj-column>
    </mj-section>
  </mj-body>
</mjml>

The root comment seems to cause the parser to fail:

{:error, "unexpected token at position 0..12"}

Comments elsewhere in the template parse fine, eg:

<mjml>
  <mj-body>
    <mj-section>
      <!-- comment -->
      <mj-column>
        <mj-text>Hello World</mj-text>
      </mj-column>
    </mj-section>
  </mj-body>
</mjml>
<!-- comment -->
{:ok, "<!doctype html><html>...</html>"}
@jdrouet
Copy link
Owner

jdrouet commented Apr 16, 2024

👋 Hey @moomerman, thanks for this issue.
So the problem here is that we parse against a component in particular.
The entry point being Mjml.parse(...). If we accept anything, I think it would require a breaking change.
Is it really something you can't live without? Why can't you put it in the <mjml> component itself? 🤔

@jdrouet jdrouet added the bug Something isn't working label Apr 16, 2024
@moomerman
Copy link
Author

👋🏼 @jdrouet thanks for getting back. It isn't something under my control unfortunately, an update to the framework I'm using to generate the MJML is adding these comments in. I can work around it on my side, I just thought perhaps it was a bug in the parser since it handles comments fine otherwise, including outside the </mjml><!-- comment -->. The online MJML live editor also correctly ignores the first comment.

@jdrouet
Copy link
Owner

jdrouet commented Apr 17, 2024

I'm curious about that tool you use to generate the mjml. Could you share it with me?

@moomerman
Copy link
Author

Yep sure. I'm using the Phoenix web framework for Elixir. Phoenix comes with a way of writing components as functions which gives you formatting, syntax highlighting and compile-time checks for things like mismatched tags and required attributes. It also enables you to compose your components nicely as custom HTML tags.

Here's a real example from our codebase:

Reset Password Template 
defmodule App.Emails.ResetPassword do
  @moduledoc false
  use MyApp, :email

  attr :url, :string, required: true

  def mjml(assigns) do
    ~H"""
    <.simple_layout>
      <:preview>Reset your password</:preview>
      <:title>Reset your password</:title>

      <mj-text align="center" color="#231F20" font-size="16px" line-height="1.5" padding="10px">
        <span style="word-spacing: normal;">
          You can reset your password by clicking the button below
        </span>
      </mj-text>
      <mj-spacer height="30px"></mj-spacer>
      <.button href={@url}>
        Reset your password <span aria-hidden="true">→</span>
      </.button>
    </.simple_layout>
    """
  end

  def text(user, url) do
    """
    ==============================

    Hi #{user.email},

    You can reset your password by visiting the URL below:

    #{url}

    If you didn't request this change, please ignore this.

    ==============================
    """
  end
end

In the example you can see I'm reusing <.simple_layout> and <.button> components and have "slots" for things like preview and title.

The issue I've come across recently is that Phoenix added debug annotations to track where your components were rendered from in development when you look at the source.

When I render my MJML templates I'm getting these annotations which look like this:

<!-- <App.Emails.ResetPassword.mjml> lib/app/emails/reset_password.ex:8 (app) -->
<!-- @caller lib/app/emails/reset_password.ex:9 (app) -->
<!-- <App.Emails.Components.simple_layout> lib/app/emails/components.ex:11 (app) -->
<mjml>...</mjml>

which currently cause it to fail to parse. It is only happening in dev though and I can add a workaround for it since it is predictable.

@jdrouet
Copy link
Owner

jdrouet commented Apr 18, 2024

After doing some tests with mjml itself, the comments that are out of mj-body are ignored. I might do it the same way.

@jdrouet jdrouet mentioned this issue Apr 20, 2024
Merged
@jdrouet
Copy link
Owner

jdrouet commented Jun 13, 2024

FYI, the last release embeds the changes 😉

@jdrouet jdrouet closed this as completed Jun 13, 2024
moomerman added a commit to moomerman/mjml_nif that referenced this issue Jun 18, 2024
@moomerman
Bump mrml from 3.1.5 to 4.0.0 in /native/mjml_nif
090cec6 
Version 4.0.0 fixes a bug if you use phoenix function components
to generate MJML and have heex annotations enabled by default.

Essentially the heex annotations add comments outside the <mjml> tag
and this was breaking the pareser.  The bug has been fixed upstream.

See jdrouet/mrml#409 for more info.
@moomerman moomerman mentioned this issue Jun 18, 2024
Closed
moomerman added a commit to moomerman/mjml_nif that referenced this issue Jun 18, 2024
@moomerman
Bump mrml from 3.1.5 to 4.0.0 in /native/mjml_nif
4610467 
Version 4.0.0 fixes a bug if you use phoenix function components
to generate MJML and have heex annotations enabled by default.

Essentially the heex annotations add comments outside the <mjml> tag
and this was breaking the pareser.  The bug has been fixed upstream.

See jdrouet/mrml#409 for more info.
@moomerman moomerman mentioned this issue Jun 18, 2024
Merged
moomerman added a commit to moomerman/mjml_nif that referenced this issue Jun 18, 2024
@moomerman
Bump mrml from 3.1.5 to 4.0.0 in /native/mjml_nif
87f41af 
Version 4.0.0 fixes a bug if you use phoenix function components
to generate MJML and have heex annotations enabled by default.

Essentially the heex annotations add comments outside the <mjml> tag
and this was breaking the pareser.  The bug has been fixed upstream.

See jdrouet/mrml#409 for more info.
moomerman added a commit to moomerman/mjml_nif that referenced this issue Jun 18, 2024
@moomerman
Bump mrml from 3.1.5 to 4.0.0 in /native/mjml_nif
be2dc97 
Version 4.0.0 fixes a bug if you use phoenix function components
to generate MJML and have heex annotations enabled by default.

Essentially the heex annotations add comments outside the <mjml> tag
and this was breaking the pareser.  The bug has been fixed upstream.

See jdrouet/mrml#409 for more info.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@moomerman @jdrouet