[BREAKING] ES6 tree-shaking by only using named exports and preserving modules

[bdurrer]

The main issue with tree-shaking seems to have come from exporting a default module.
The UMD build still exposes an global object Immutable, so it's not affected.

Furthermore, sideEffects: true works by excluding whole modules (files), so the ES6 export must preserve modules in order to enable the end-user's bundler to detect unused parts.

I included an test module, which can be used to see the effects of tree-shaking when using different parts of the library.
The project comes with two analyzers.

The analyzers tell us that only using List results in 34kB (instead of 64kB). However, include a single Map and we are already at 54kB.
One thing I saw was that there are a few cross-references, some of them quite a suprise to the average user. E.g. Map includes OrderedMap because of the sort function. Same with Set and OrderedSet.

I don't think Immutable can ever go super-low, as each collection type comes with an extensive api. A bundler will likely never be able to detect which methods on a collection type are actually used and always has to include the whole class.

[jdeniau]

A possible issue with that might be that the following might not work on strict compiler (possibly with nextjs for example)

import Immutable from 'immutable'

Immutable.Map()

Even if it's not the required way to import immutable, don't you think that this should be breaking and included in a major release?

[bdurrer]

Even if it's not the required way to import immutable, don't you think that this should be breaking and included in a major release?

err yes, I would say that would cause a major release (if we actually follow semantic versioniong to the letter). Do we have a pattern for that or just collect changes and then decide whether it is a minor or major release? 🤔

[bdurrer]

import Immutable from 'immutable'

Immutable.Map()

BTW the worarkound would be import * as Immutable from 'Immutable';

[jdeniau]

For me it's a 👍 , but I think that we should release this in a 5.x version, as the gain is quite small and the API is breaking.

You can add the 5.0 milestone, if you agree.

[bdurrer]

I thought a bit about the cyclic dependencies. They seem to only exist because of the sort function on Set and Map and all they do is call OrderedSet(this) / OrderedMap(this). We probably should remove them or find some way to offer the functionality in a disconnected way. It would make tree shaking way more useful

[TaylorClay]

Wanted to chime in here:
Not to get up on a high horse, but I feel strongly this change should go in (to v5.0 - agreed that it should be a major version bump as it is a breaking change).

Immutable is used by so many devs across many different sizes of projects (NPM has the weekly downloads at 8.75m!).
Many of these devs don't know about / care about bundling & tree shaking.
This is why I think we need to make this change; it would "force" devs to optimize their Immutable imports (unless they're stubborn and do import * as Immutable) after upgrading.

The product I work on professionally is getting an additional 30+ kB on the Critical Path due to transitive dependencies bulk importing Immutable - just to use one or two utilities.

[bdurrer]

Nice, I had the same idea last week but didn't act yet 😁

I think I will propose another PR to 5.x soonish, which would remove the sort function from Set and Map. We can offer a sort function on top level that returns a sorted collection instead.

IMO the migration path is easy and it resolves the whole cyclic dependency thing.

[jdeniau]

Merging this in 5.x. The TypeScript and Flow types are probably wrong.

Opening #1955 for follow up.