Mjml: keepComments config option doesn't seem to do anything

Created on 12 Mar 2021  ยท  8Comments  ยท  Source: mjmlio/mjml

Describe the bug

I know I'm new to this, so maybe I'm just missing something basic?

I've tried passing --config.keepComments false on the command line, and it doesn't seem to have any effect on the output. I thought maybe it only worked if minification was turned on, but that doesn't make any difference. I also tried --config.minifyOptions='{"minifyCSS": true, "keepComments": false}' but the effect was the same.

To Reproduce
Steps to reproduce the behavior:

  1. echo '<mjml><mj-body><!-- comment --></mj-body></mjml>' > index.mjml
  2. mjml -r index.mjml --config.keepComments false -o index.html
  3. grep 'comment' index.html
  4. grep returns <!-- comment -->

Expected behavior

<!-- comment --> should not occur in the file; grep should return nothing.

MJML environment (please complete the following information):

  • OS: Linux (Mint 20.1, Ubuntu 20)
  • MJML Version: 4.9.0
  • MJML tool: npm mjml

Most helpful comment

Fixed in v4.9.3

All 8 comments

@kmcb777 It looks like this option is broken on CLI

const html = `
<mjml>
  <mj-body>
    <mj-section>
<!-- Debug comment -->
    </mj-section>
  </mj-body>
</mjml>
`

const mjml2html = require('mjml')

console.log(mjml2html(html, { keepComments: false }))

Removes comment properly

@rootwork This should also be fixed, there was an issue in the cli concerning the whole options object passed to mjml2html

Hmm, I've upgraded:

โฏ mjml --version
mjml-core: 4.9.1
mjml-cli: 4.9.1

But when I run the steps to reproduce it's still occurring.

my bad, this option was actually not handled by the cli
this will be fixed in the next release

@rootwork it should be good now in 4.9.2

Hi @kmcb777, this seems to have broken something for us. We run mjml in a CI pipeline, and in the last 15 minutes (so since 4.9.2 release) the pipeline has been failing.

Here's where we run it, as you can see we don't do anything complex:

RUN npm i -g mjml
[...]
RUN ["mjml", "/tmp/messaging-service.mjml", "-o", "/tmp/messaging-service.html"]

This is failing with the following error:

Step 5/17 : RUN ["mjml", "/tmp/messaging-service.mjml", "-o", "/tmp/messaging-service.html"]
 ---> Running in 58cd7b711d46
/usr/local/lib/node_modules/mjml/node_modules/mjml-cli/lib/client.js:176
  }, argv.c.keepComments === 'false' && {
            ^
TypeError: Cannot read property 'keepComments' of undefined
    at _default (/usr/local/lib/node_modules/mjml/node_modules/mjml-cli/lib/client.js:176:13)
    at Object.<anonymous> (/usr/local/lib/node_modules/mjml/node_modules/mjml-cli/bin/mjml:3:28)
    at Module._compile (node:internal/modules/cjs/loader:1108:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1137:10)
    at Module.load (node:internal/modules/cjs/loader:988:32)
    at Function.Module._load (node:internal/modules/cjs/loader:828:14)
    at Module.require (node:internal/modules/cjs/loader:1012:19)
    at require (node:internal/modules/cjs/helpers:93:18)
    at Object.<anonymous> (/usr/local/lib/node_modules/mjml/bin/mjml:4:1)
    at Module._compile (node:internal/modules/cjs/loader:1108:14)

I have locked us to 4.9.1 for now, but thought you might want a heads up.

Hi @kieranajp thanks for reporting this i'll fix this right now

Fixed in v4.9.3

Was this page helpful?
0 / 5 - 0 ratings

Related issues

karanmartian picture karanmartian  ยท  3Comments

kytosai picture kytosai  ยท  4Comments

liminspace picture liminspace  ยท  3Comments

iwanaga-sakura picture iwanaga-sakura  ยท  4Comments

ghost picture ghost  ยท  4Comments