sighmon/mjml-rails

MJML 4.8.x broke my validator again :)

Closed this issue · 4 comments

denny commented

I don't have time to dig into this one as thoroughly as last time, sorry 🙁 I have a nasty feeling it may be filed under 'feature not a bug' at the MJML end so I'll have to adapt somehow, but I thought I'd run it past you as it is a change in behaviour of/via your gem, without your code having changed, which I would imagine is quite frustrating!

Layout: plugins/ShinyNewsletters/app/views/layouts/newsletter_mailer.html.mjml
Partial: spec/fixtures/TEST/views/shiny_newsletters/newsletter_mailer/an_example.html.mjml
Validator: app/validators/mjml_syntax_validator.rb
Test: plugins/ShinyNewsletters/spec/models/shiny_newsletters/template_spec.rb#L44

Up to MJML 4.7.1 the validator passed and now it doesn't. If I move the layout code into the partial, then the validator passes, so it seems like either it was wrapping the partial in the layout before and isn't now, or it was less strict about templates being a whole document?

If this is just a case of my needing to tweak a config setting somewhere to allow partials to render (and you could point me at it!) that would be great 😄 Otherwise, I've pinned MJML to 4.7.1 for now and hopefully I'll have more time to dig into this again late February/early March.

denny commented

mjmlio/mjml@1ee581f looks possibly relevant... :-\

denny commented

Definitely a change in how the mjml command responds to a partial input file 😒

denny@rocinante:~/ShinyCMS$ cat partial.mjml 
<mj-section>
  <mj-column>
    <mj-text>
      Hello world.
    </mj-text>
  </mj-column>
</mj-section>
denny@rocinante:~/ShinyCMS$ ./node_modules/.bin/mjml --version
mjml-core: 4.7.1
mjml-cli: 4.7.1
denny@rocinante:~/ShinyCMS$ ./node_modules/.bin/mjml -v partial.mjml 
denny@rocinante:~/ShinyCMS$ ./node_modules/.bin/mjml partial.mjml > /dev/null
denny@rocinante:~/ShinyCMS$ yarn install
yarn install v1.22.5
[1/4] Resolving packages...
[2/4] Fetching packages...
[3/4] Linking dependencies...
[4/4] Building fresh packages...
Done in 2.85s.
denny@rocinante:~/ShinyCMS$ ./node_modules/.bin/mjml --version
mjml-core: 4.8.0
mjml-cli: 4.8.0
denny@rocinante:~/ShinyCMS$ ./node_modules/.bin/mjml -v partial.mjml 
denny@rocinante:~/ShinyCMS$ ./node_modules/.bin/mjml partial.mjml > /dev/null
File: partial.mjml
TypeError: Cannot read property 'replace' of undefined

Command line error:
Input file(s) failed to render
denny@rocinante:~/ShinyCMS$
denny commented

Possibly interesting...

denny@rocinante:~/ShinyCMS$ cat invalid_partial.mjml
<mj-section>
  <mj-column>

     <mj-title>
      Titles don't belong in the body block
    </mj-title>

  </mj-column>
</mj-section>
denny@rocinante:~/ShinyCMS$ ./node_modules/.bin/mjml -v invalid_partial.mjml
Line 4 of /home/denny/code/denny/ShinyCMS-ruby/invalid_partial.mjml (mj-title) — mj-title cannot be used inside mj-column, only inside: mj-attributes, mj-head

Command line error:
Validation failed
denny@rocinante:~/ShinyCMS$ ./node_modules/.bin/mjml  invalid_partial.mjml > /dev/null
File: invalid_partial.mjml
TypeError: Cannot read property 'replace' of undefined

Command line error:
Input file(s) failed to render

... so the explicit validation option -v fails (with a useful error) if the MJML in a partial input file is actually invalid, but not if it's valid-but-incomplete. Is there any way for me to get at that behaviour through mjml-rails currently?

denny commented

I rewrote my validator to call out to mjml --validate directly, which solves the problem for me.

I'm going to close this issue as I feel like the validation/validator stuff is probably an edge case 🙂 If anybody else runs into a related problem then hopefully they'll find this and they can link/re-open.

I was wondering whether to try to add a .validate method alongside .render in Mjml::Parser (again, using mjml --validate to do the actual work). Let me know if that's a feature you'd like to add and I'll take a swing at it if I get time.

  • 👍1