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.

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

Problem processing non-ERB (Haml, Slim) layouts #34

Open
aleksandrs-ledovskis opened this issue Jul 10, 2018 · 4 comments
Open

Problem processing non-ERB (Haml, Slim) layouts #34

aleksandrs-ledovskis opened this issue Jul 10, 2018 · 4 comments

Comments

@aleksandrs-ledovskis
Copy link

aleksandrs-ledovskis commented Jul 10, 2018

In #33 / v4.2.0 a change in logic was made around assumption that layout template will always contain verbatim <mjml>.

In ERb world this (mostly) holds, but when layout is composed using advanced templating languages like Haml and Slim, both allowing tag definition without <> wrapping, it fails.

Also, it doesn't handle edge case where somebody is referencing/writing code comment in partial that is e.g. <%# Let's use MJML in this partial. Note that <mjml> must reside in layout %> - currently, mjml-rails will take that comment's <mjml> as flag to process template with MJML library. Or when root tag is generated using content_tag. Or when it's contained in a partial.

Current workaround: Put verbatim <mjml> in Haml/Slim layouts. Avoid dynamically generating MJML root tag. Don't reference <mjml> proper anywhere else except MJML layout files.


Simple change of

if template.source =~ /<mjml>/

to

if compiled_source =~ /<mjml>/

would solve no <> and comment-triggering-MJML situations, but still leave other aforementioned edge cases broken.


Possible solutions:

  1. Doing first-pass render of template to process nested partials/helpers, capture this output and use same match (=~ /<mjml>/) as done currently.

  2. Require some kind of magic comment in file (e.g. # mjml_processing: true) that would serve as signal to do MJML processing of rendered file.

  3. Require users to whitelist paths to MJML-processed layout/legacy-template files in initializer, akin to Asset Pipeline compilation does. Then just match against template.identifer


/cc @sighmon

@sighmon
Copy link
Owner

sighmon commented Jul 11, 2018

@aleksandrs-ledovskis I haven't used Haml or Slim, so will take your recommendation for the best way forward.

  1. sounds like a pretty decent way forward to me if it's similar to how Asset Pipeline compilation handles things.

But again, please use your best judgement. :-)

sbiastoch added a commit to sbiastoch/mjml-rails that referenced this issue Jul 20, 2018
@jbwl
Copy link

jbwl commented Jul 31, 2018

For the time being the documentation should contain a hint that other template languages than ERB are currently broken.

@sighmon
Copy link
Owner

sighmon commented Aug 3, 2018

@biastoch2 @jbwl would your favoured solutions be to change if template.source =~ /<mjml>/ to if compiled_source =~ /<mjml>/ as you've done in your fork @biatoch2 ?

sighmon added a commit that referenced this issue Aug 3, 2018
@jbwl
Copy link

jbwl commented Aug 3, 2018

I tried the fix from @biatoch2 and it works for me. I'd vote for it.

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

No branches or pull requests

3 participants