twing icon indicating copy to clipboard operation
twing copied to clipboard

Reduce bundle size

Open MickL opened this issue 6 years ago • 12 comments

Would it be possible to reduce the total file size of this module? When I compile with webpack it results in 1.1mb minified.

I know bundle-file-size is normally not a requirement in Node.js, but for example an serverless environment like Cloudflare Worker do have a limit of 1mb.

Test-code I compiled:

const {TwingEnvironment, TwingLoaderArray} = require('twing');

let loader = new TwingLoaderArray({
    'index.twig': 'Hello {{ name }}!'
});
let twing = new TwingEnvironment(loader);

twing.render('index.twig', {name: 'Fabien'}).then((output) => {
    // do something with the output
});

MickL avatar Dec 06 '19 12:12 MickL

The main culprit here is the encoding conversion support that is part of Twig specification (see the encode filter for example).

But it should be possible to have the encoder module removed from the bundle if you don't need encoding support in your application.

Twing is using iconv-lite to support encoding.

ericmorand avatar Dec 06 '19 16:12 ericmorand

I dont understand where this encode is used exactly and what feature would be missing then.

Could you create an module we can import that does not use encoding?

Btw. I didnt understand why I need TwingLoaderArray and TwingEnvironment. I would have liked to just use twing.render(). If thats possible that would save bundle size for me, too.

MickL avatar Dec 06 '19 17:12 MickL

The encoding support is needed for the convert_encoding filter:

https://twig.symfony.com/doc/3.x/filters/convert_encoding.html

You should be able to use Webpack to replace iconv_lite module by a fake module that returns the input as output to dramatically decrease the bundle size.

This is not Twing responsibility to provide custom set of specifications. Twing implements the specification strictly,.without compromise. Bundlers provide ways to remove module and are responsible of removing the features you don't need.

ericmorand avatar Dec 07 '19 18:12 ericmorand

As an alternative, grab the sources, modify src/lib/extension/core/filters/convert_encoding.ts to remove the importation of iconv_lite and just return string in the function.

Than install the dependencies and run npm run bundle and you'll get a nice bundle without encoding conversion support.

ericmorand avatar Dec 07 '19 18:12 ericmorand

Any chance this can be done as an optional dependency where twing detects if iconv_lite exists?

Making this optional will be useful for scenarios where twing is used in browser, where downloaded payload size matters.

Thanks for this nice package and hope you consider this. ;-)

kctang avatar Dec 13 '19 06:12 kctang

Well iconv_lite exists since it is bundled with the rest of the app.

Maybe someone can maintain a lite version of Twing without encoding conversion support?

I keep on thinking that having iconv_lite replaced by a stub by your module bundler is the best way to go. I can provide help with that. Even write a Wiki about it.

ericmorand avatar Dec 16 '19 14:12 ericmorand

I can provide help with that. Even write a Wiki about it.

Thanks, it would be nice to have an "official" and supported way of stripping iconv_lite from bundles. I'm not familiar with webpack (and bundlers in general), but it sounds like something that could break easily due to a Twing update. Is this even possible to support something like that officially in a "bundler agnostic" way?

dorian-marchal avatar Dec 16 '19 14:12 dorian-marchal

I'm not familiar with webpack (and bundlers in general), but it sounds like something that could break easily due to a Twing update.

It could if for example we change the encoding library to something else. But not easily because it's not gonna change anytime soon.

But what I can do is provide two bundles with the package. One with encoding conversion support and one without. And update the doc accordingly. That seems reasonable to me.

ericmorand avatar Dec 16 '19 14:12 ericmorand

From my perspective it would be great to have a different import which inits twing without inconv_lite instead of hacking it away when bundling.

MickL avatar Dec 16 '19 14:12 MickL

My testing shows than removing iconv-lite decreases the bundle from 1.3MB to 1.0MB with Browserify. Not bad. I guess results would be even better with Webpack.

ericmorand avatar Dec 16 '19 15:12 ericmorand

Still this is (too) huge :/ Is it possible to refactor twing so each developer can import only the things we actually need? Keyword: tree shaking

MickL avatar Dec 20 '19 10:12 MickL

@ericmorand

My testing shows than removing iconv-lite decreases the bundle from 1.3MB to 1.0MB with Browserify.

Currently it is more than 4MB which is an rediculous amount of code and Webpack throws a warning:

WARNING in ./node_modules/twing/dist/cjs/lib/cache/filesystem.js 35:28-47
Critical dependency: the request of a dependency is an expression
 @ ./node_modules/twing/dist/cjs/main.js
 @ ./src/services/twig-template-engine.service.ts
 @ ./src/services/response.service.ts
 @ ./src/main.ts

Could you provide this an TwingEnvironment export that does not contain iconv_lite? Are there more options to reduze the bundle size?

MickL avatar Jan 17 '20 16:01 MickL