NightlyCommit / twing

First-class Twig engine for Node.js
BSD 2-Clause "Simplified" License
199 stars 23 forks source link

Reduce bundle size #464

Closed MickL closed 11 months ago

MickL commented 4 years ago

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
});
ericmorand commented 4 years ago

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.

MickL commented 4 years ago

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.

ericmorand commented 4 years ago

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 commented 4 years ago

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.

kctang commented 4 years ago

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. ;-)

ericmorand commented 4 years ago

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.

dorian-marchal commented 4 years ago

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?

ericmorand commented 4 years ago

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.

MickL commented 4 years ago

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.

ericmorand commented 4 years ago

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.

MickL commented 4 years ago

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 commented 4 years ago

@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?