[Bug]: Node: Argument list too long

[stevebauman]

What happened?

I have some really large MJML templates (several megabytes) that I'm rendering in production, but they're failing to render with this library due to the MJML being too long to send as a command line input.

I think the current approach of sending the base-encoded MJML to render as a string to a custom JS script to then decode it and process it through the MJML library could be improved. Instead, we could:

1. Create a temporary MJML file to store the given MJML
2. Pass the tmp file path to the native mjml command, along with a new tmp file path to output in
3. file_get_contents of the output to return
4. Delete tmp files

I have this working in production right now and it removes the need for a middleman script, along with base64 encoding/decoding completely.

I was going to build my own MJML PHP library with this approach, but wanted to see if this may be something you're interested in integrating before doing so. Let me know!

Here's my working code:

namespace App\Support;

use App\Support\Node;
use App\Filesystem\Facades\TemporaryFile;

class Mjml
{
    /**
     * Convert the MJML to HTML.
     */
    public static function toHtml(string $mjml): string
    {
        $templatePath = TemporaryFile::generate(
            TemporaryFile::makeUniqueFilePath('.mjml'),
            $mjml
        );

        $outputPath = TemporaryFile::generate(
            TemporaryFile::makeUniqueFilePath('.mjml'),
            '',
        );

        try {
            Node::execute(base_path('node_modules/mjml/bin/mjml'), [
                $templatePath,
                '-o',
                $outputPath,
            ]);

            return file_get_contents($outputPath);
        } finally {
            unlink($templatePath);
            unlink($outputPath);
        }
    }
}

namespace App\Support;

use Exception;

use Symfony\Component\Process\Process;
use Symfony\Component\Process\ExecutableFinder;
use Symfony\Component\Process\Exception\ProcessFailedException;

class Node
{
    /**
     * Execute the script.
     */
    public static function execute(string $scriptPath, array $args = [], array $input = []): string
    {
        $process = new Process([...static::getCommand($scriptPath), ...$args]);

        $process->setInput(json_encode($input));

        $process->run();

        if (! $process->isSuccessful()) {
            throw new ProcessFailedException($process);
        }

        if ($process->getErrorOutput()) {
            throw new Exception($process->getErrorOutput());
        }

        return $process->getOutput();
    }

    /**
     * Get the command to execute.
     */
    protected static function getCommand(string $scriptPath): array
    {
        $extraDirectories = config('node.directories', []);

        if ($nodePathFromEnv = config('node.path')) {
            array_unshift($extraDirectories, $nodePathFromEnv);
        }

        return [
            (new ExecutableFinder)->find('node', 'node', $extraDirectories),
            $scriptPath,
        ];
    }
}

How to reproduce the bug

Attempt to generate a few megabytes in size of MJML.

Package Version

1.2.1

PHP Version

8.2

Which operating systems does with happen with?

No response

Notes

No response

[riasvdv]

Using temporary files is a good idea I think, I'd definitely accept a PR that implements this

[stevebauman]

Ok cool, will work on this today, thanks for the quick response @riasvdv! 👍

[stevebauman]

Ah, unfortunately discovered this may be easier said than done after looking through this:

1. This will require a new major version, as various config options are not supported in the CLI version, such as keepComments and ignoreIncludes
2. This library may utilize MJML Sidecar, which uses a Lambda function... I'm not experienced in this regard so I'd be hesitant to make any breaking changes to it and have you guys responsible
3. We'd have to update both these packages in tandem due to the breaking changes required here

Closing this out. Will have to create my own package instead! 👍

[riasvdv]

I think you’re able to pass all the config options necessary https://github.com/mjmlio/mjml/blob/master/packages/mjml-cli/README.md#available-options

The sidecar version would need to have the current behaviour though, as there’s no way to send a file. Maybe we could have it configurable?

If you’re more comfortable with a separate package, then that’s fine for us as well of course 😄

[riasvdv]

@stevebauman I opened a draft PR with an implementation #26 would you be willing to test it and see if you have any remarks?

[stevebauman]

Sorry for the late reply @riasvdv, signed off on Thursday and have been sick as a dog since. Almost back now though.

We're currently using the originally posted implementation in prod now which resolved the issue so I don't want to flip it back to test in case -- but I'll give it a review 👍