Mj Include crash on latest Beta

[nagman]

Describe the bug
When I put a mj-include in mj-head, node throws this:

TypeError: setBackgroundColor is not a function
    at MjBody.render (/api/node_modules/mjml-body/lib/index.js:67:7)
    at /api/node_modules/mjml-core/lib/createComponent.js:336:28
    at Array.map (<anonymous>)
    at MjHead.handlerChildren (/api/node_modules/mjml-core/lib/createComponent.js:317:24)
    at MjHead.handler (/api/node_modules/mjml-head/lib/index.js:43:19)
    at _processing (/api/node_modules/mjml-core/lib/index.js:235:26)
    at Object.mjml2html [as default] (/api/node_modules/mjml-core/lib/index.js:329:25)
    at compileTemplate (/api/src/emails/templates/index.ts:16:30)
    at Object.<anonymous> (/api/src/emails/templates/index.ts:35:5)
    at Module._compile (internal/modules/cjs/loader.js:701:30)

To Reproduce
email.mjml

<mjml>
	<mj-head>
		<mj-include path="./partials/style.mjml" />
	</mj-head>

	<mj-body>
		<mj-section>
			<mj-column>
				<mj-text>Blabla, whatever</mj-text>
			</mj-column>
		</mj-section>
	</mj-body>
</mjml>

partials/style.mjml

<mj-attributes>
	<mj-text font-family="Arial" font-size="14px" line-height="170%" align="center" />
</mj-attributes>

Expected behavior
It should compile nicely.

MJML environment (please complete the following information):

• OS: Ubuntu 18.04
• MJML Version: 4.4.0-beta.2
• MJML tool used: mjml for node

Additional context
When I remove the mj-include, it compiles without error.

Notice that another bug occurs whith mj-include when I put one just after the first mj-body tag:
email.mjml

<mjml>
	<mj-body>
		<mj-include path="./partials/header.mjml" />
		<mj-section>
			<mj-column>
				<mj-text>Blabla, whatever</mj-text>
			</mj-column>
		</mj-section>
	</mj-body>
</mjml>

throws mj-section cannot be used inside mj-raw, only inside: mj-attributes, mj-body, mj-wrapper.

But if I put it before the last mj-body tag AND after a comment, it works.
I mean... it doesn't work at the end (partial is not included) but it doesn't throw anything.

[iRyusa]

Maybe related to #1585 ? cc @kmcb777 we should see this one before release the 4.4.0

[kmcb777]

Hi @nagman thanks for reporting this. Concerning the first point, head components are not handled the same way as body components.
One consequence is that when you use head components in a mj-include, you must wrap them in <mjml><mj-head> tags, otherwise they will be considered as body components by default. I'll add this to the doc.
I'll check this anyway because there might be other issues concerning the other thing you reported, in the last release i made a few changes in the way mj-heads are handled in mj-includes because there were already problems with that

[nagman]

@kmcb777 In fact, I already tested with <mjml> and <mj-head> in partials/style.mjml:

<mjml>
	<mj-head>
		<mj-attributes>
			<mj-text font-family="Arial" font-size="14px" line-height="170%" align="center" />
		</mj-attributes>
	</mj-head>
</mjml>

And I still get the same error:

TypeError: setBackgroundColor is not a function
    at MjBody.render (/api/node_modules/mjml-body/lib/index.js:67:7)
    at /api/node_modules/mjml-core/lib/createComponent.js:336:28
    at Array.map (<anonymous>)
    at MjHead.handlerChildren (/api/node_modules/mjml-core/lib/createComponent.js:317:24)
    at MjHead.handler (/api/node_modules/mjml-head/lib/index.js:43:19)
    at _processing (/api/node_modules/mjml-core/lib/index.js:235:26)
    at Object.mjml2html [as default] (/api/node_modules/mjml-core/lib/index.js:329:25)
    at compileTemplate (/api/src/emails/templates/index.ts:16:30)
    at Object.<anonymous> (/api/src/emails/templates/index.ts:33:5)
    at Module._compile (internal/modules/cjs/loader.js:701:30)

[vdsabev]

Same error here. Something worth noting is that when I use mj-include for attributes the VS Code preview works fine while trying to render the HTML via Node.js returns this error.

[iRyusa]

VSCode plugin isn't updated to 4.4.X beta yet, so it confirms that it's a bug introduced in 4.4.X

[kmcb777]

I made several tests, with the cli and as an import in a node app, and everything worked fine. It turns out that even without the mjml and mj-head tags in the included file, it works.

@nagman How do you install mjml exactly ? Your version could still be linked to other version's package, it happens sometimes.
Do you install locally or globally ? If local could you remove your node_modules, and npm (or yarn) install again ? If global (not recommended, higher chances of having this versions conflicts problem), you should make sure that no other version is still installed.
You can use npm list --depth=0 | grep mjml to check the versions of sub-packages
Also, do you provide your mjml as a string, or as a json ?

@vdsabev Do you use mjml with the cli, or as a package in a node app ?
Also, do you install it locally or globally ?

[vdsabev]

@kmcb777
Version: [email protected]
Installed: locally (using npm install mjml)
Usage: const mjml = require('mjml')
OS: Windows 10

[kmcb777]

@nagman @vdsabev
Sorry actually you can ignore my previous comment, i could reproduce the issue with an other template.
But it turns out this happens only when the path of the included file is incorrect.
When mjml can't find the included file, it adds a comment in the html saying it couldn't include the file. That's where the bug actually is, so we can keep this issue opened.

[nagman]

@kmcb777
Glad you could reproduce the issue, but my path to the included file is correct.

Or maybe mjml does something strange with the path. But how can I check it?

[kmcb777]

@nagman to investigate the path problem you can just move your mj-include to a position where it doesn't crash the process, if the included file is not found then in the html output it will replace the mj-include with a comment, saying
mj-include fails to read file :, followed by the relative and absolute paths. By checking the absolute path you should be able to guess which directory is taken as reference (it should be either the mjml file directory or process.cwd()).

[nagman]

@kmcb777 er... So I made tests and the behavior is VERY strange.

I have a header and a footer partials.

If I include the header before the content, it crashes.
If I include the header after the content, it works.
If I include the footer before the content, it crashes.
If I include the footer after the content, it works.
If I include the header and the footer after the content, it crashes.
If I include the header and the footer after the content and I comment the footer, it crashes.
And so on... I don't know how many strange tests can be made but the two errors I randomly get are:

mj-raw cannot be used inside mj-raw, only inside: mjml, mj-accordion, mj-accordion-element, mj-attributes, mj-body, mj-column, mj-group, mj-head, mj-hero, mj-navbar, mj-section, mj-wrapper

mj-section cannot be used inside mj-raw, only inside: mj-attributes, mj-body, mj-wrapper

And I don't have any mj-raw tag in my partials and my main template.

Also, if I change the mj-include path of the footer to a totally wrong path (like 'fghsdlkfjgsdhlfkvjsdbflkvb'), I don't have any compilation errors.

[iRyusa]

@nagman Can you create a small repo to reproduce the issue you're encountering ?

It would help a lot to check what's going on, if you can provide all of those cases, it would help use to create a proper test suite on the parser to prevent those weird issues

[kmcb777]

@nagman when you say 'it works', it doesn't crash, but is your mj-include correctly included ?

During my tests, what i saw is that when a mj-include fails, the position in the mjml tree is not correctly reset by the parser, and the next element is incorrectly placed. (This is what i fix in the commit referenced above).
The failed include is replaced with a comment saying that the include failed (this is normal behavior). This comment itself is inserted as a mj-raw.
Thus the next element can be incorrectly placed as a child of this mj-raw, instead of a sibling, resulting in validation errors like mj-section cannot be used inside mj-raw

I never got any of these errors when the mj-include path is correct, so if you can reproduce with a valid include, please provide the mjml files and folder structure you're using, i couldn't reproduce with the examples provided above

[iRyusa]

@nagman any news on that ?

[nagman]

@iRyusa Here's the test repo:
https://github.com/nagman/mjml-test

I've noticed that it doesn't crash if the mjml file is at the root of the project.

But according to my project complexity, I surely cannot put my mjml templates at the root.

[iRyusa]

Ok then I understand the issue, you're not providing the filePath option to the mjml2html function, so mj-include is relative from where the process is launch.

[nagman]

I've just updated the repo with two more tests.

You will see that the filePath option doesn't change anything.

[nagman]

Ok, I've managed it.

I actually thought that the filePath was the path to the folder where the files belong, but it is the path to the file you're compiling.

Example:

import { readFileSync } from 'fs';
import { join } from 'path';
import mjml2html from 'mjml';

const path = './templates/test.mjml';
const mjmlString = readFileSync(join(__dirname, path), 'utf8');
mjml2html(mjmlString, { filePath: join(__dirname, path) })

So you need to readFileSync the file, then compile it with the same path as filePath.

Couldn't it be set by default?

[iRyusa]

You're passing a string to mjml2html so it has no idea of what is the current file/context, it will default to current process.cwd

[nagman]

@iRyusa You're right. Is there any way not to use fs.readFile and to pass directly the template path to mjml2html?

[iRyusa]

Nope, it only take a string as an input. But it's not really that hard to do something like that :

const transpileMjmlFromFile = (filePath) => mjml2html(readFileSync(join(__dirname, filePath), 'utf8'), {filePath})

[kmcb777]

@nagman @vdsabev The new beta has been released, you can check if this behaviour is fixed. You can install it with the tag mjml@next.

[nagman]

@kmcb777 Thanks but as I said here, I've solved my issue.

I hadn't fully understood the docs.