ping @bdurrer as you did work on that subject too.

do we have a test that verifies tree-shaken code still works? I remember that our first (failed) attempt looked good until webpack removed the magic.

You did "move" some functions, like isList → probeIsList. Why did you do that ?

Grouping things into a 'probe' module with prefixed 'probe'-naming enables one to intuit where probe-related behaviours are defined and to grep where they are imported. These things helped me to understand and change the sources.

It seems that you did sort imports too. [...] (or for the sorting, open a PR that sort all imports and add an eslint rule that enforce sorted imports).

Okay. Things were moved around without any awareness of order of imports. A separate PR could be made for it.

Feel free to leave this PR idle and use as inspiration or reference to solve the issue in some other way.

Changes continued here to export a functional interface similar to erlang's "map" https://www.erlang.org/docs/23/man/maps and the main benefit is smaller bundle size.

Using esbuild to bundle separate files that create and manipulate map-like data with immer, mutative, immutable.js and my own immutable-derived 'im' this is the result,

• bench.mini.immer.min.js 9.1kb
• bench.mini.mutative.min.js 18.8kb
• bench.mini.immutable.min.js 65.9kb
• bench.mini.im.min.js 6.7kb

My modified version of immutable is smallest. It could be smaller if time were spent further reducing size. Additionally, no functionality is lost; a file still exists which exports a chainable-methods-variant of Map and provides a choice to import that variant when needed.

Considerations for immutablejs based on my experience.

mergeDeep, updateIn, hasIn, removeIn etc could maybe be moved to a separately-imported package or module and removed as methods. If immutablejs' core interface can be used externally to support deep-manipulating behaviour (it can, imo) then these complex operations can be moved to "userspace", simplifying immutablejs' core. map4 = opMapsMerge(map1, map2, map3) Similarly, some niche-methods like flip() could maybe be moved to userland as well.

List and Map are probably the main types needed by users and all other types more niche (imo). If types like Record and Stack are moved to their own separated packages with some duplicated code, the overall impact is likely positive.

Using function calls rather than method calls internally would make refactoring and moving things around easier. For example the patch below would decouple Map from KeyedCollection

diff --git a/src/Map.js b/src/Map.js
index dd9faf5e4..2c1b411a9 100644
--- a/src/Map.js
+++ b/src/Map.js
@@ -37,11 +37,9 @@ export const Map = (value) =>
     ? emptyMap()
     : isMap(value) && !isOrdered(value)
       ? value
-      : emptyMap().withMutations((map) => {
-          const iter = KeyedCollection(value);
-          assertNotInfinite(iter.size);
-          iter.forEach((v, k) => map.set(k, v));
-        });
+      : opWithMutations(emptyMap(), m => {
+        Object.keys(value).forEach(k => opSet(m, k, value[k]))
+      })
 
 export class MapImpl extends KeyedCollectionImpl {
   create(value) {

Related to the above diff; consider exporting separate versions of constructors for different value types. eg, MapFromList, MapFromNativeMap, MapFromPlainObject and users can import and use specific (and comparatively simpler) variants they need.

Also consider removing or moving-out "atattransducer"-related things like this,

MapPrototype['@@transducer/init'] = MapPrototype.asMutable = asMutable;
MapPrototype['@@transducer/step'] = function (result, arr) {
  return result.set(arr[0], arr[1]);
};
MapPrototype['@@transducer/result'] = function (obj) {
  return obj.asImmutable();
};

Remove duplicated methods like map.merge and map.concat and support each behaviour through one method name only.

Basically these are the useful things to do now imo that would make immutablejs smaller and easier to refactor. Other changes would be useful as well, but those are the immediate things.

That's quite impressive dividing the size by 10! 🤯

About your suggestions:

Reducing the API

It might be interesting to reduce the size, but I don't want to reduce the API for now: I have no statistics about what methods are used or not.
Creating modules or separate packages might work for reducing the size, but it will split immutable in two: methods in immutable and methods outside. I think if you want to do that, you can choose a toolbox-approach library like lodash, no?

List and Map as first class

I think that you are right, List and Map are the two most useful classes here, but if tree-shaking works, it will automatically remove unused classes, don't you think ?

decouple Map from KeyedCollection

You use Object.keys in your example, but using KeyedCollection here is useful if you use the "array-of-tuple" constructor here: https://immutable-js.com/play/#TWFwKFtbJ2EnLCAnQUEnXSwgWydiJywgJ0JCJ11dKQ==

moving-out "atattransducer"

https://github.com/cognitect-labs/transducers-js has been archived in 2023, I think that we can remove that in v6 👍

About your PR

I have your review in mind, but I need to take time to review that.

If you can guide us about the global logic and how to apprehend this PR, it will help a lot as a lot of changes are relocating and renaming functions, which make the review quite harder to read.

Thanks for the thoughtful response. Seems agreeable. I will reply to some of those points only to clarify thoughts and not from disagreement.

[reducing the api] might be interesting to reduce the size, but [...] no statistics about what methods are used or not [...] it will split immutable in two: methods in immutable and methods outside

Consider removing the deep methods in a separate future release, after they have been supported with separately importable modules like this. These would be kept with core immutablejs sources and unit-tested with them,

import mergeMapsDeep from 'immutable/mergeMapsDeep'

The 'deep' manipulation functions are an area of challenge when refactoring immutablejs. Mainly this is because they create KeyedSequences, IndexedSequences and Maps and so lend themslves to circular conditions. If manipulations like 'mergeDeep' are imported separately from core immutablejs, the circular coupling issues go away and things easier to change.

if tree-shaking works, it will automatically remove unused classes, don't you think ?

Yes true. Each collection-type must be defined through its own mostly-separate import-tree, regardless of where it is defined.

You use Object.keys in your example, but using KeyedCollection here is useful if you use the "array-of-tuple"

Yes. If there were a MapFromLists like erlang's maps:from_list/1, a small function could be made for this specific condition. Small, tree-shakeable and also does not rely on KeyedCollection

const mapFromList = list => {
  utilAssertTupleShape(list)

  return opWithMutations(mapCreateEmpty(), map => {
    list.reduce((acc, kv) => opSet(acc, kv[0], kv[1]), map)
  })
}

I have your review in mind, but I need to take time to review that.

It may be impractical to review it. Large PRs are not great, but truly many layers of difficult changes are needed to remove circular dependencies here.

If you can guide us about the global logic and how to apprehend this PR

Removing the circular dependencies in controlled steps is not easy. The current immutablejs does not cooperate well with changes for various and surprising reasons. It is good to pace oneself.