Use classes to improve tree-shaking

[ggrossetie]

I've noticed that, currently, it makes little to no difference to use import {deflate} from 'pako' rather than import pako from 'pako'.

import pako from 'pako'

kroki.js → kroki.bundled.js...
Build summary for kroki.bundled.js - es
┌──────────────────┬───────────┐
│ File name        │ Size      │
│ ---------------- │ --------- │
│ kroki.bundled.js │ 247.11 kB │
│ ---------------- │ --------- │
│ Totals           │ 247.11 kB │
└──────────────────┴───────────┘
created kroki.bundled.js in 851ms

import {deflate} from 'pako'

kroki.js → kroki.bundled.js...
Build summary for kroki.bundled.js - es
┌──────────────────┬───────────┐
│ File name        │ Size      │
│ ---------------- │ --------- │
│ kroki.bundled.js │ 244.26 kB │
│ ---------------- │ --------- │
│ Totals           │ 244.26 kB │
└──────────────────┴───────────┘
created kroki.bundled.js in 854ms

While investigating, I saw that Inflate was not removed from the bundle but if I convert function Inflate(options) to a class class Inflate then it's working as expected:

kroki.js → kroki.bundled.js...
Build summary for kroki.bundled.js - es
┌──────────────────┬───────────┐
│ File name        │ Size      │
│ ---------------- │ --------- │
│ kroki.bundled.js │ 156.18 kB │
│ ---------------- │ --------- │
│ Totals           │ 156.18 kB │
└──────────────────┴───────────┘
created kroki.bundled.js in 662ms

Is there a particular reason for not using classes? Will you consider a pull request with that change?

[puzrin]

This package was done to be compatible with ancient browsers, probably not actual now.

But you can import pre-build files from dist https://github.com/nodeca/pako/tree/master/dist. Will it solve your problem right now, without changes?

[ggrossetie]

Not really because I'm creating an ESM-first web component so I cannot import deflate from pako/dist/pako_deflate since it's a CommonJS file.

If we don't want to update the code, an alternative solution would be to produce ESM files for pako_deflate.js and pako_inflate.js but it feels counter productive since ESM was designed to produce compact bundle with a static structure. In other words, import deflate from 'pako/dist/pako_deflate' (instead of import {deflate} from 'pako') would be contrary to the ESM philosophy (in my opinion).

[ggrossetie]

Since we are already using Babel, I believe that class will be transpiled/transformed when the target is ES5. So this change will only affect the ESM file.

[HansBrende]

@puzrin I too only need to use deflate and it would be very nice to have inflate tree-shooken out.

[M-a-c-Carter]

@ggrossetie @rlidwka is this a change that you would be interested in supporting? This library has like 10m+ users tree shaking would be a really good add especially since that is far more modern. @ggrossetie @puzrin what changes would be needed to add that support is it really just a simple as adding class?

[ggrossetie]

what changes would be needed to add that support is it really just a simple as adding class?

Yes.

[clshortfuse]

From my understanding, classes don't tree-shake as well as ESM Modules, even static. That's because you can always reference a method by name:

class MyClass {
  myMethod() { }

  myMethodINeverUse() {}
}

const instance = new MyClass();

instance.myMethod();
let methodName = 'myMethodINeverUse'
instance[methodName]();

ESM Modules don't really exhibit this issue and are better for tree-shaking:

export function myMethod() {}
export function myMethodINeverUse() {};

Babel, Webpack, and Rollup will all tree-shake when you do:

import {deflate, inflate} from 'pako'

And even if you were to import inflate and never use it, those bundlers will remove them anyway during minification.

You can even do import * as pako from 'pako' and bundlers will remove what you don't use. But then you have to be conscious of the sideEffects: true if you try to do something like:

import * as pako from 'pako';

const pakoFn = 'import';
pako[pakoFn]()

That's a side-effect where you don't actually support clean tree-shaking of imports.

See: https://webpack.js.org/guides/tree-shaking/

TL;DR: Drop require and use native ESM modules, not classes, for better tree-shaking.

[clshortfuse]

Also, I'm pretty sure you can do

import { deflate } from 'pako/lib/deflate.js';
import { inflate } from 'pako/lib/inflate.js';

And you're basically tree-shaking yourself. Though because the files are using require, they both might import utils/string.js completely, instead of tree-shaking only what it needs.