Support tree shaking

[wmertens]

I avoid using immutable because it is a huge chunk of code relative to my apps, and for initial page load on mobile, every KB matters. I do get it via third party components, which I then load on-demand only.

However, I noticed that the size is always the same, 57KB minified (using the excellent webpack-bundle-analyzer), which is also the size that it ships with in the npm package.

I doubt that all third party modules use all functionality, so that means that the minification cannot identify unused pieces of code. I build with webpack tree shaking, which works using ES2016 export syntax, first identifying unused exports, not exporting them, and then letting the minification remove any unused code.

So, I think that the immutable library could be smaller if it was also available in an es flavor (in the same package, via a package.json key) that exports everything separately.

However, that will only help if the code is "chunky", so that if you only use e.g. a Map, it will only touch part of the code.

Furthermore, the code should not have side effects. I noticed a lot of createClass calls, the minifier won't be able to optimize those away because it cannot assume it to have no side effects.

Another caveat is that lodash faces the same problem, and the lodash-es build is not helping, probably due to the fact that the default export exports everything as an object. Lodash has to resort to a Babel plugin that rewrites import {foo} from 'lodash' to import foo from 'lodash/foo', and as a consequence, both lodash and lodash-es get the same build size without tree shaking.

TL;DR: If Immutable is "chunky", it can lead to smaller build sizes by tree shaking iif it has an es build and it doesn't export everything on default and things like createClass are done at build time. I think/hope.

[andrewmclagan]

Even having an option to import through paths would suffice in my opinion:

import List from 'immutable/List';

[crodriguez1a]

I'd be willing to put in the grunt work to chunk it up. Is there an RFC?

[a-x-]

is any progress here?

tree shaking is not working for me still

immutable@4.0.0-rc.2
webpack@2.6.1

I use only one thing:
{ List } = require 'immutable'
but my bundle contains everything of immutable-js

webpack & babel config

{
  "entry": [
    "/Users/mxtnr/rocket/felix/frontend/projects/admin/src/index"
  ],
  "output": {
    "path": "/Users/mxtnr/rocket/felix/public/webpack/admin",
    "filename": "[chunkhash].js",
    "publicPath": "/webpack/admin/"
  },
  "plugins": [
    new webpack.optimize.UglifyJsPlugin({ sourceMap: true }),

    // https://webpack.js.org/guides/migrating/#uglifyjsplugin-minimize-loaders
    new webpack.LoaderOptionsPlugin({ minimize: true, sourceMap: true }), // @deprecated
  ],
  "module": {
    "rules": [
      {
        test: /\.js$/,
        exclude: /node_modules/,
        "use": [
          {
            "loader": "babel-loader",
            "options": {
              "presets": [
                [
                  "env",
                  {
                    "targets": {
                      "browsers": [
                        "last 2 versions"
                      ]
                    },
                    "modules": false,
                    "useBuiltIns": true
                  }
                ],
                "react",
                "stage-2"
              ],
              "cacheDirectory": true
            }
          }
        ]
      },
      // ...
    ]
  },
  "target": "web",
  "resolve": {
    "modules": [
      "node_modules",
      "/Users/mxtnr/xp/spacebuilder/node_modules"
    ],
    "extensions": [
      ".js",
      ".coffee",
      ".cjsx"
    ]
  },
  "resolveLoader": {
    "modules": [
      "/Users/mxtnr/xp/spacebuilder/node_modules",
      "node_modules"
    ]
  },
  "devtool": "source-map",
  "externals": {
    "jquery": "jQuery"
  }
}

[alzalabany]

any intention to work on this ?

[wmertens]

@alzalabany it seems that there is an es build now. Next thing to try would be to set sideEffects:false in package.json, which helps webpack with tree shaking.

[ulrichb]

@wmertens I'm using Create React App (2.1.1) and tree-shaking with immutable js (4.0 RC12) still doesn't seem to work.

Why is this issue closed?

[a-x-]

sideEffects:false must be added to:

https://github.com/facebook/immutable-js/blob/master/package.json

[a-x-]

#1661

[ulrichb]

sideEffects:false must be added

I've tested this (sideEffects:false) locally and it still didn't work.

---

Note that I have also created an webpack issue for this: webpack/webpack#8483.

There @sokra replied:

Immutable probably uses a wrapper with a lot of methods on the prototype, this basically references all of the methods.
=> These libraries are not optimized for tree-shaking. File issues there. That's probably not a webpack issue.

[geeky-biz]

Has tree shaking with immutable js worked for anyone?
Wondering why is the issue closed.

[nenya1840]

why is this issue closed?

[Danetag]

+1, could we re-open it? Thanks!

[dawsbot]

From all I can tell, tree shaking was merged into master with this but it requires an upgrade to immutable v4.

Check if your version of immutable is supposed to support tree shaking with

cat node_modules/immutable/package.json | grep module\"

An empty response means that you do not have a tree-shaking version ("module" in the package.json)

---

In my application, immutable 4 introduces many errors, which other folks have noticed. I'm not sure the best path towards adopting tree-shaking, but unless someone can show me otherwise, I'll be stuck on v3 😢

[Serginho]

@dawsbot I'm not seeing any improvements in 4.0.0-rc.12.

• I'm using tree-shaking with angular-cli
• In my project I'm only use List.
• source-map-explorer is not detecting submodules.

[alexgleason]

Same here on rc14.

Since this project has new maintainers this would be a really nice thing to have.

[jdeniau]

Hi,

It's not on our timeline for now. Our focus is really to close all the issues related to 4.0.0 only, and release that.

But you are right, tree-shaking is a really required features.

I re-open this issue and tag it for later.

[jdeniau]

In fact, sideEffect HAS been added on the package.json released in the v4.0.0-rc.15 release.

Can you check and tell us if the issue is fixed ?

[alexgleason]

I gave rc15 a shot but it doesn't seem affected. 🤔

I'm using the library heavily, so it's possible I really am using 100% of the code.

But it's suspicious that the bundle analyzer shows it as one large block. Some packages like lodash display the individual modules broken out:

You can see my bundle analyzer report here: https://fe.soapbox.pub/report.html

[Methuselah96]

Yeah, my guess is that we haven't separated things out enough to get it to tree shake properly, but I haven't looked into it at all yet.

[iamrenejr]

Running cat node_modules/immutable/package.json | grep module\" for me produces an output. So, I can see that I do have tree-shaking.

But having only imported fromJS, the nextjs analyzer tells me it has brought in not just Map and List, but also Seq, Stack, Hash, Range, plus utilities I haven't used, such as isImmutable, isRecord, merge, etc. The total size is 59.9kb parsed, 16.42kb gzipped.

I raised in #1961 some clarification about how this could be addressed. It seems, based on the discussion, that there's a lot of cross-referencing instead of a collection of loosely defined objects and utilities. Untangling these cross-references is probably going to be key to making the package tree-shakeable.

[bdurrer]

But having only imported fromJS, the nextjs analyzer tells me it has brought in not just Map and List, but also Seq, Stack, Hash, Range, plus utilities I haven't used, such as isImmutable, isRecord, merge, etc. The total size is 59.9kb parsed, 16.42kb gzipped.

You might not have called isImmutable or isRecord when importing fromJS. But fromJS needs these to parse arbitrary, unforseeable data. Neither we nor your toolchain can possibly know [at compile time] whether that input data needs to result in a Map, List, OrderedMap. Seq, Range, Hash are widely used common objects, can't exclude these either.

It seems, based on the discussion, that there's a lot of cross-referencing instead of a collection of loosely defined objects and utilities. Untangling these cross-references is probably going to be key to making the package tree-shakeable.

Immutable isn't a loosely collection of objects, it is a collection api/framework. It's not a collection of helper utilities like e.g. lodash.
Defining fully usable collections classes requires defining all the exposed methods and you can't simply tree-shake them away - at least current tree-shaking tools can't do that.

I don't think rewriting immutable into an unfriendly, unusable utility-based framework is a real solution.

The only option I currently see is that the compilation of the base collection (CollectionImpl) maybe results in that code being copied to each collection type. If that is the case, we should and probably can rewrite the internals.

[iamrenejr]

I see! I was under the impression that fromJS only imports Map and List, and that doing so does not also import OrderedMap, Seq, Range, Hash, etc. What I'm gathering now and in #1961 is, due to method chaining and the ability to convert one data type into another, importing Map and List only is enough to import nearly all of immutable.js. I wasn't using the chaining operations directly, and so I didn't consider those use cases. For me, I was only using .get and .set and had no plans on converting to Seq or Hash from Map or List. But now I know better! Thanks!

When I was part of a team that maintained an internal library, we had a mechanism for allowing the user of that library to specify which "use cases" and "services" were going to be exposed. This is because the library itself was very, very huge, and it was wasteful to import all of it into any service. So, if the User class had the method "acceptUserFriendRequest", but if the user of that library didn't explicitly include the method, that method was not getting imported. How this worked is there was a way to mark the individual methods for which "use case" they belonged to, and its use case had to be specified at initialization time for that method to be included.

Of course, that scenario is different, and so maybe that solution isn't applicable - those were backend libraries, they were also internal, it's not really tree-shaking, and the patterns were very strictly enforced to conform to the way the library wanted to be used, etc.

Drawing ideas from that though - and this is just a stab in the dark! - I'm wondering if an optional configuration file or package.json setting can be made to whitelist or blacklist specific data structures or class methods, such that if the user specifies a "deny all except whitelist" configuration, then we essentially have "manual tree-shaking", and it's a "do this at your own risk" sort of deal on the part of the user of immutable.js. The default behavior is that immutable.js works as it does now - including all classes and methods that might be needed in the bundle. But there is a compile-time option of choosing to include or exclude specific features of the library. And maybe, through incremental rollouts, the option to exclude only Range (random example) is released first, and then the exclusion options are expanded over time as new features.

I think what that would mean is immutable.js still has to work if the user only uses the exposed methods, making use of internal methods as needed to make it work even if it isn't directly exposed in the configuration. But, if due to the exclusion settings, if some part of the library becomes unreachable, then that unreachable part can be tree-shaken.

For example, if I did not want to use OrderedSet, then as the user of the library, I can make a design decision to exclude the use of OrderedSet and all methods that produce an OrderedSet in my code. Then the bundle would not need to include OrderedSet at all, and it would be my fault as the user of the library if I tried to use OrderedSet after I specifically excluded it in the configuration.

I don't know how simple, easy, or even feasible this idea is, so again, grain of salt! I imagine if immutable.js was not designed from the ground-up to be loosely coupled, then this would need significant internal changes. On the other hand, my intuition is that something like this could introduce tree-shaking without breaking changes, because excluding modules under this scheme is opt-in and not automatic. Either way, at least maybe I can learn something new again by throwing this idea out there! Sorry if it misses the mark though!

EDIT: Digesting what you said some more, I see you've already addressed my point before I even made it!

Defining fully usable collections classes requires defining all the exposed methods and you can't simply tree-shake them away - at least current tree-shaking tools can't do that.

Yep, this makes sense. Now I see your point, I don't think it's possible to remove existing methods from a class with tree-shaking. We'd need the logic to remove methods to execute at runtime, at which point tree-shaking is not applicable anymore.

Though I can imagine an additive approach where only the base methods of Map were implemented (like .get, .set, .map, etc) and methods that reference, say, OrderedMap, are implemented as classes of their own with, say, one-level inheritance, which can be tree-shaken away if only the base class needs to be used. Following the throughline of my suggestion, then some kind of configuration would select which one is imported - but I don't know myself how that could be done. Maybe it's not possible.

Alternatively, if a "base map" (having only .set, .get, .map, etc) and a "complete map" (having .sort, .toList, etc) were implemented, then they could be imported separately. When the "complete map" is imported, "base map" always comes along. But if only "base map" is ever imported, then the "complete map" is tree-shakeable.

[jdeniau]

I do not know if we want to go to this path, but following your idea, a possible solution might be to have a "more" tree-shakeable version of immutable that remove all converters

import { Map, fromJS } from 'immutable/base'

// the following code won't work with `base` immutable
Map().toSet()

It would require for us to build twice, once without toXXX.
It would require the user to use this extremely carefully as if you use both imports, you will have twice the weight of immutable (we may reduce it to immutable 100% weight depending of how we build it).

It will require a lot of work, and a lot of nurturing though… I don't not if this is worth it (with this solution, tree-shaking, if possible, is worth it).

[bdurrer]

I had the same thought and I think the chance to mix up that import a single time is super high. Maybe having extra NPM packages would be more useful/save.

I can see three potentially useful targets:

• full
• without ordered types (no sort, toOrdered etc)
• base version with only the basic methods used for freezing state: Map,Set,List with set,get,merge

I am not even sure it would be so difficult to split these up, as CollectionImpl already works like an injection tool. Some methods would need moving around, but it would allow leaving away some stuff.

In theory, the library could come with empty implementations for unsupported methods that log access and would warn you about usages that would either break code or break tree-shaking.

But I am still not sure this is worth it. For one thing a lot of internal classes are still needed (Seq, Hash, ...).
IMO people use immutable because they want to have a full blown collection api. If you want to just have a simple freezer utility, you probably take another library anyway?

[jdeniau]

We are in the same mood here : I know that tens of bytes can be a lot, but having several packages or a complex system will lost user. I would prefer not make any step in this direction if we do not have a solution that is a no-brainer for the end user.

[iamrenejr]

That makes sense! For now, I'll go a different route, but will circle back to this if I notice myself needing the optimizations. Beyond a freezer library for objects, having persistent data structures in my corner during my preliminary testing helped smooth out the memory profile and decrease GC cycles (I've gone to using POJOs, and I'm seeing lots of GC now).

Problem is, going for something like seamless-immutable, even though it's lighter, it hasn't been updated in years, and Immer is friendlier to an object mutation pattern than immutable.js is. But these could be called premature optimizations, admittedly, though in my particular case adding, swapping, or removing immutable.js is very easy due to using lenses, so in practice it costs very little to change which library provides the PDS (or if PDS is used or not).

I totally appreciate that supporting only Map or Set - so basically, exposing only the PDS part - may not necessarily be the most value-adding thing to immutable.js right now. Just wanted to share some of the value that I see personally in separating the base data structures from the additional utilities in a tree-shakeable way. :)