mjmlio / mjml

MJML: the only framework that makes responsive-email easy
https://mjml.io
MIT License
17.08k stars 960 forks source link

MJML weigth #724

Closed nfroidure closed 7 years ago

nfroidure commented 7 years ago

Hi Guys!

MJML is a great idea but it weights too much.

I chosen to use it in the context of lambda functions and as a result, all my builds that sends e-mails now are 44M (26M zipped) when the average weight of other lambdas are 1M max.

It is not sustainable and slows down deployments exponentially. I unfortunately will have to use something else :(.

Just leaving an issue for you to be aware of the fact MJML needs some diet ;).

tornad commented 7 years ago

I agree with that :(

nfroidure commented 7 years ago

FYI I tried to browserify my lamdas as a temporary workaround but with no luck:

/home/nfroidure/sencrop/infrastructure-sencrop/test.js:81897
  var $ = cheerio.load('', { decodeEntities: false });
                  ^

TypeError: cheerio.load is not a function

Not sure why it fails but i guess there are some globals assumed somewhere.

Edit: The problem sits there: https://github.com/mjmlio/mjml/blob/ebe92f6f658ef96db06d3939ea4e18982fe21a7c/packages/mjml-core/src/helpers/dom.js

I assume that putting the require statement on top of the file would fix that issue but not sure the original issue will be solved.

nfroidure commented 7 years ago

I finally managed to make a browserify build that do not fail:

env NODE_ENV=cli "browserify" "builds/development/functions/putPassword/index.js" "-r" "cheerio" "--exclude" "crypto" "-o" "build.js"

The reason why i exclude crypto is explained here: https://stackoverflow.com/questions/38986957/how-to-bundle-express-node-js-application-with-webpack/44604498#44604498

Leaving the issue since it is clearly a ugly hack but hope it will help people having the same issue.

meriadec commented 7 years ago

Did you tried MJML API ? https://mjml.io/api So you can avoid bundle MJML in your sources.

nfroidure commented 7 years ago

@meriadec it could be an option if it was not a beta service with no SLA, also I prefer avoid relying on an external API for such a simple need.

Thanks for the suggestion.

iRyusa commented 7 years ago

Hi @nfroidure

As we already discussed here #274 MJML isn't really meant to be build in a browser context. I'm curious why you would package MJML with each build ?

In MJML4 email's build time is way faster and we're removing React as a dependency so it should help with package size but we can't do more here.

nfroidure commented 7 years ago

I'm curious why you would package MJML with each build ? That's the way AWS Lambda works https://aws.amazon.com/fr/lambda/

I finally did a workaround consisting of building the HTML with templating values inteact and replacing it at execution time so that i do not have to put mjml in the dependencies but in the devDependencies instead.

That said, it is just a workaround (it do not allows repeatable sections for example) and sadly MJML will probably be useless for me until it changes.

The issue here is really weight, the browserify issues were just workaround attempts for that issue.

nfroidure commented 7 years ago

Here are some suggestions to make the repo lighter:

There are certainly lots of other optimizations to chase but it would be a good start.

iRyusa commented 7 years ago

mjml-core doesn't require any component as you can see here : https://github.com/mjmlio/mjml/blob/master/packages/mjml-core/package.json

mjml package is a "community" package that require every standard components to provide an easy to install experience without any configuration.

As we said in #274 any pr on browser build are welcome, but we're not providing any support on this. With MJML4 around the corner, it would take us too much time to ensure browser build behave as server build ( it works today with an ugly hack of wrapping cheerio/jquery API in https://github.com/mjmlio/mjml/blob/master/packages/mjml-core/src/helpers/dom.js but it's not used everywhere and have some non ISO behaviour )

About lodash I don't think this is a good idea. lodash is a tiny project, really helpful, and has a nice API and we don't want to reinvent the wheel when something as lodash is widely adopted among the javascript community

I'm not familiar with Amazon's Lambda but you couldn't call an external lambda that only take mjml and spit out the result ? So you don't have to bundle MJML in each build but just call the Lambda that will take care of that for you ?

bradgreens commented 7 years ago

I'm not familiar with Amazon's Lambda but you couldn't call an external lambda that only take mjml and spit out the result ?

I'm new to the issue report... our company is building the lambda service pattern by default (that is, the endpoint to spit out the results). There's no reason for a browser to transpile the markup if you already have a resource like Lambda. At the same time we can't possibly count on a public API for a prod app (hence, a node lambda service we own).

I love the platform. If we implement it well I'd be happy to post back in a couple months.

nfroidure commented 7 years ago

As stated earlier the connection timeouts while uploading the zip file. Even in a separate Lambda the problem remains the same.

@bradgreens I also could have not fill the issue or give advice on how to resolve it. I guess smart people do not mind when answers take some time to pop. A bit of thinking may at least avoid giving negative output like the one I just read.

Edit: Thanks for the reminder though.

bradgreens commented 7 years ago

I understand, I'll own my words.

@nfroidure what did I miss?

As we said in #274 any pr on browser build are welcome, but we're not providing any support on this.

nfroidure commented 7 years ago

Won't feed the troll anymore.

ngarnier commented 7 years ago

Guys, let's stay courteous. I don't want to lock the feed but I will do if we can't have a polite debate of opinions.

We don't think the size of MJML is crazy big but we understand it could be better so we take good note of your comments @nfroidure. However, with MJML 4 coming, optimizing the weight won't be our short-term priority (we also won't be able to divide the weight by two). We also think there are decent workarounds:

nfroidure commented 7 years ago

Thank you for the help and I indeed found a workaround.

I understand it is not a priority.

Keep up the good work ;).