Repository: jdrouet/mrml (language: python)

Issue:
Root HTML comment fails to parse
I'm happily using this library via [Elixir](https://hexdocs.pm/mjml/readme.html) and have hit an issue rendering the following template:

```html
<!-- 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:

```elixir
{:error, "unexpected token at position 0..12"}
```

Comments elsewhere in the template parse fine, eg:

```html
<mjml>
  <mj-body>
    <mj-section>
      <!-- comment -->
      <mj-column>
        <mj-text>Hello World</mj-text>
      </mj-column>
    </mj-section>
  </mj-body>
</mjml>
<!-- comment -->
```

```elixir
{:ok, "<!doctype html><html>...</html>"}
```


Discussion / hints:
👋 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 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.
I'm curious about that tool you use to generate the mjml. Could you share it with me?
Yep sure.  I'm using the Phoenix web framework for Elixir.  Phoenix comes with a way of writing [components as functions](https://hexdocs.pm/phoenix_live_view/Phoenix.Component.html) 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:

<details>

<summary>Reset Password Template</summary>

```elixir
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
```
</details>

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](https://hexdocs.pm/phoenix_live_view/Phoenix.Component.html#module-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:

```html
<!-- <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.

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

Fix patch:
diff --git a/packages/mrml-core/src/lib.rs b/packages/mrml-core/src/lib.rs
index 14a0299e..90e42e18 100644
--- a/packages/mrml-core/src/lib.rs
+++ b/packages/mrml-core/src/lib.rs
@@ -162,6 +162,9 @@ pub mod node;
 pub mod prelude;
 pub mod text;
 
+// Only used to ignore the comments at the root level
+mod root;
+
 mod helper;
 mod macros;
 
@@ -191,9 +194,8 @@ pub fn parse_with_options<T: AsRef<str>>(
     input: T,
     opts: &crate::prelude::parser::ParserOptions,
 ) -> Result<mjml::Mjml, prelude::parser::Error> {
-    let parser = crate::prelude::parser::MrmlParser::new(opts);
-    let mut cursor = crate::prelude::parser::MrmlCursor::new(input.as_ref());
-    parser.parse_root(&mut cursor)
+    let root = crate::root::Root::parse_with_options(input, opts)?;
+    root.into_mjml().ok_or(prelude::parser::Error::NoRootNode)
 }
 
 #[cfg(all(feature = "parse", feature = "async"))]
@@ -224,9 +226,8 @@ pub async fn async_parse_with_options<T: AsRef<str>>(
     input: T,
     opts: std::sync::Arc<crate::prelude::parser::AsyncParserOptions>,
 ) -> Result<mjml::Mjml, prelude::parser::Error> {
-    let parser = crate::prelude::parser::AsyncMrmlParser::new(opts);
-    let mut cursor = crate::prelude::parser::MrmlCursor::new(input.as_ref());
-    parser.parse_root(&mut cursor).await
+    let root = crate::root::Root::async_parse_with_options(input, opts).await?;
+    root.into_mjml().ok_or(prelude::parser::Error::NoRootNode)
 }
 
 #[cfg(feature = "parse")]
@@ -241,9 +242,7 @@ pub async fn async_parse_with_options<T: AsRef<str>>(
 /// ```
 pub fn parse<T: AsRef<str>>(input: T) -> Result<mjml::Mjml, prelude::parser::Error> {
     let opts = crate::prelude::parser::ParserOptions::default();
-    let parser = crate::prelude::parser::MrmlParser::new(&opts);
-    let mut cursor = crate::prelude::parser::MrmlCursor::new(input.as_ref());
-    parser.parse_root(&mut cursor)
+    parse_with_options(input, &opts)
 }
 
 #[cfg(all(feature = "parse", feature = "async"))]
@@ -259,9 +258,8 @@ pub fn parse<T: AsRef<str>>(input: T) -> Result<mjml::Mjml, prelude::parser::Err
 /// # })
 /// ```
 pub async fn async_parse<T: AsRef<str>>(input: T) -> Result<mjml::Mjml, prelude::parser::Error> {
-    let parser = crate::prelude::parser::AsyncMrmlParser::default();
-    let mut cursor = crate::prelude::parser::MrmlCursor::new(input.as_ref());
-    parser.parse_root(&mut cursor).await
+    let opts = std::sync::Arc::new(crate::prelude::parser::AsyncParserOptions::default());
+    async_parse_with_options(input, opts).await
 }
 
 #[cfg(all(test, feature = "parse"))]
diff --git a/packages/mrml-core/src/root/mod.rs b/packages/mrml-core/src/root/mod.rs
new file mode 100644
index 00000000..c2b60797
--- /dev/null
+++ b/packages/mrml-core/src/root/mod.rs
@@ -0,0 +1,31 @@
+use crate::comment::Comment;
+use crate::mjml::Mjml;
+
+#[cfg(feature = "parse")]
+mod parse;
+#[cfg(feature = "render")]
+mod render;
+
+#[derive(Debug)]
+enum RootChild {
+    Mjml(Mjml),
+    Comment(Comment),
+}
+
+#[derive(Debug)]
+pub(crate) struct Root(Vec<RootChild>);
+
+impl AsRef<[RootChild]> for Root {
+    fn as_ref(&self) -> &[RootChild] {
+        self.0.as_ref()
+    }
+}
+
+impl Root {
+    pub(crate) fn into_mjml(self) -> Option<Mjml> {
+        self.0.into_iter().find_map(|item| match item {
+            RootChild::Mjml(inner) => Some(inner),
+            _ => None,
+        })
+    }
+}
diff --git a/packages/mrml-core/src/root/parse.rs b/packages/mrml-core/src/root/parse.rs
new file mode 100644
index 00000000..7a401180
--- /dev/null
+++ b/packages/mrml-core/src/root/parse.rs
@@ -0,0 +1,105 @@
+use crate::{
+    comment::Comment,
+    prelude::parser::{Error, MrmlCursor, MrmlParser, MrmlToken, ParseChildren, ParserOptions},
+};
+
+use super::RootChild;
+
+impl<'opts> crate::prelude::parser::ParseChildren<Vec<RootChild>> for MrmlParser<'opts> {
+    fn parse_children(&self, cursor: &mut MrmlCursor<'_>) -> Result<Vec<RootChild>, Error> {
+        use crate::prelude::parser::ParseElement;
+
+        let mut result = Vec::new();
+        while let Some(token) = cursor.next_token() {
+            match token? {
+                MrmlToken::Comment(inner) => {
+                    result.push(RootChild::Comment(Comment::from(inner.text.as_str())));
+                }
+                MrmlToken::ElementStart(inner) if inner.local.eq("mjml") => {
+                    result.push(RootChild::Mjml(self.parse(cursor, inner.local)?));
+                }
+                other => {
+                    return Err(Error::UnexpectedToken(other.span()));
+                }
+            }
+        }
+        Ok(result)
+    }
+}
+
+#[cfg(feature = "async")]
+#[cfg_attr(target_arch = "wasm32", async_trait::async_trait(?Send))]
+#[cfg_attr(not(target_arch = "wasm32"), async_trait::async_trait)]
+impl crate::prelude::parser::AsyncParseChildren<Vec<RootChild>>
+    for crate::prelude::parser::AsyncMrmlParser
+{
+    async fn async_parse_children<'a>(
+        &self,
+        cursor: &mut MrmlCursor<'a>,
+    ) -> Result<Vec<RootChild>, Error> {
+        use crate::prelude::parser::AsyncParseElement;
+
+        let mut result = Vec::new();
+        while let Some(token) = cursor.next_token() {
+            match token? {
+                MrmlToken::Comment(inner) => {
+                    result.push(RootChild::Comment(Comment::from(inner.text.as_str())));
+                }
+                MrmlToken::ElementStart(inner) if inner.local.eq("mjml") => {
+                    let element = self.async_parse(cursor, inner.local).await?;
+                    result.push(RootChild::Mjml(element));
+                }
+                other => {
+                    return Err(Error::UnexpectedToken(other.span()));
+                }
+            }
+        }
+        Ok(result)
+    }
+}
+
+impl super::Root {
+    /// Function to parse a raw mjml template with some parsing
+    /// [options](crate::prelude::parser::ParserOptions).
+    ///
+    /// You can specify the kind of loader mrml needs to use for loading the
+    /// content of [`mj-include`](crate::mj_include) elements.
+    ///
+    /// You can take a look at the available loaders
+    /// [here](crate::prelude::parser).
+    ///
+    /// ```rust
+    /// use mrml::root::Root;
+    /// use mrml::prelude::parser::ParserOptions;
+    /// use mrml::prelude::parser::memory_loader::MemoryIncludeLoader;
+    ///
+    /// let options = ParserOptions {
+    ///     include_loader: Box::new(MemoryIncludeLoader::default()),
+    /// };
+    /// match Root::parse_with_options("<mjml><mj-head /><mj-body /></mjml>", &options) {
+    ///     Ok(_) => println!("Success!"),
+    ///     Err(err) => eprintln!("Something went wrong: {err:?}"),
+    /// }
+    /// ```
+    pub(crate) fn parse_with_options<T: AsRef<str>>(
+        value: T,
+        opts: &ParserOptions,
+    ) -> Result<Self, Error> {
+        let parser = MrmlParser::new(opts);
+        let mut cursor = MrmlCursor::new(value.as_ref());
+        Ok(Self(parser.parse_children(&mut cursor)?))
+    }
+
+    #[cfg(feature = "async")]
+    pub(crate) async fn async_parse_with_options<T: AsRef<str>>(
+        value: T,
+        opts: std::sync::Arc<crate::prelude::parser::AsyncParserOptions>,
+    ) -> Result<Self, Error> {
+        use crate::prelude::parser::AsyncMrmlParser;
+        use crate::prelude::parser::AsyncParseChildren;
+
+        let parser = AsyncMrmlParser::new(opts);
+        let mut cursor = MrmlCursor::new(value.as_ref());
+        Ok(Self(parser.async_parse_children(&mut cursor).await?))
+    }
+}
diff --git a/packages/mrml-core/src/root/render.rs b/packages/mrml-core/src/root/render.rs
new file mode 100644
index 00000000..e2baeb57
--- /dev/null
+++ b/packages/mrml-core/src/root/render.rs
@@ -0,0 +1,40 @@
+use std::cell::{Ref, RefCell};
+use std::rc::Rc;
+
+use crate::prelude::render::{Error, Header, Render, RenderOptions, Renderable};
+
+pub(crate) struct RootRender<'e, 'h> {
+    header: Rc<RefCell<Header<'h>>>,
+    element: &'e super::Root,
+}
+
+impl<'e, 'h> Render<'h> for RootRender<'e, 'h> {
+    fn header(&self) -> Ref<Header<'h>> {
+        self.header.borrow()
+    }
+
+    fn render(&self, opts: &RenderOptions) -> Result<String, Error> {
+        let mut result = String::new();
+        for element in self.element.as_ref().iter() {
+            let content = match element {
+                super::RootChild::Comment(inner) => {
+                    inner.renderer(self.header.clone()).render(opts)?
+                }
+                super::RootChild::Mjml(inner) => {
+                    inner.renderer(self.header.clone()).render(opts)?
+                }
+            };
+            result.push_str(&content);
+        }
+        Ok(result)
+    }
+}
+
+impl<'r, 'e: 'r, 'h: 'r> Renderable<'r, 'e, 'h> for super::Root {
+    fn renderer(&'e self, header: Rc<RefCell<Header<'h>>>) -> Box<dyn Render<'h> + 'r> {
+        Box::new(RootRender::<'e, 'h> {
+            element: self,
+            header,
+        })
+    }
+}


Test patch:
diff --git a/packages/mrml-core/tests/issue-409.rs b/packages/mrml-core/tests/issue-409.rs
new file mode 100644
index 00000000..aa1426c2
--- /dev/null
+++ b/packages/mrml-core/tests/issue-409.rs
@@ -0,0 +1,7 @@
+#![cfg(feature = "parse")]
+
+#[test]
+fn should_parse_and_ignore_root_comment() {
+    let template = r#"<!-- ignore me --><mjml><mj-body></mj-body></mjml>"#;
+    let _ = mrml::parse(template).unwrap();
+}