Faire / mjml-react

React component library to generate the HTML emails on the fly
https://www.npmjs.com/package/@faire/mjml-react
MIT License
387 stars 17 forks source link

Current usage of html-minifier does not work well in runtimes without node_modules #36

Closed eddeee888 closed 1 year ago

eddeee888 commented 2 years ago

Hi Faire friends 👋

First of all, thanks for taking over the maintenance this awesome package!

Problem

Currently, we are trying to create an AWS Lambda to render templates. This works locally but since we use ESbuild to compile for Lambda environment in production, we do not expect a node_modules or any other files, apart of one index.js file.

This is a problem when integrating with mjml-react because it always imports html-minifier which brings in uglify-js which uses require.resolve which expects certain files on the filesystem.

Suggestion

One workaround for this is to dynamically import html-minifier if options.minify === true? e.g.

// Remove from the top: `import { minify as htmlMinify } from 'html-minifier';`

function render(email, options = {}) {
  const defaults = {
    keepComments: false,
    beautify: false,
    validationLevel: 'strict',
  };

  const html = mjml2html(renderToMjml(email), {
    ...defaults,
    ...options,
    minify: undefined,
  });

  if (options.minify) {
    // Added this line to resolve html-minifier if needed.
    // Or maybe we could use ES6 dynamic import but that'll make this async
    const htmlMinify = require("html-minifier");

    return {
      html: htmlMinify(html.html, {
        caseSensitive: true,
        collapseWhitespace: true,
        minifyCSS: true,
        removeComments: true,
        removeEmptyAttributes: true,
      }),
    };
  }

  return html;
}
IanEdington commented 2 years ago

Hi @eddeee888, Thanks for the detailed bug report! In the v3 alpha release we've moved the render function out of the main index.ts file in order to sidestep this issue. Instead of spending time fixing this in v2 we'll be focusing on getting v3 stable asap, but are happy to accept a pull request if you need this functionality in v2.

We call v3 an alpha because it will probably change a lot before release but everything defined in index.tsx is very stable, we've used them internally for 1 1/2 years.

npm i @faire/mjml-react@3.0.0-a1
or
yarn add @faire/mjml-react@3.0.0-a1

Moving to v3 will mean implementing your own version of the render function. Also happy to get PRs with the suggested improvement to the render function (low priority for us in v3).

v3 development is happening on the main-v3 branch

eddeee888 commented 2 years ago

Thanks for the context @IanEdington ! I'm able to use the solution outlined here to get around my issue for now so it's not urgent to make this fix in v2 either. I'm curious about what's happening in v3 though. Do you have a roadmap? :)

emmclaughlin commented 2 years ago

Hi @eddeee888 ! We have a discussion of what we want V3 to look like going on here https://github.com/Faire/mjml-react/discussions/12. Some of the main takeaways are migrating to typescript and cleaning up the dependencies, as those seem to be some of the most common requests.

We would be happy to hear any suggestions you have and have you get involved if there is something you want to see in V3 or beyond 🙂

IanEdington commented 1 year ago

This is resolved in v3.0.0 which was just released today

tranhl commented 10 months ago

Providing some additional info for people experiencing the same problem.

Although renderToMjml doesn't import html-minifier, mjml-core still does, so to get MJML React to work in Lambda environments, it's not enough to directly import renderToMjml. You still have to add uglify-js to esbuild's externals to stop it from bundling the package.

See this comment: https://github.com/mjmlio/mjml/issues/2132#issuecomment-1167116042