Requesting clarification on the "correct" export(s) of immutable.js

[zertosh]

I understand that since Immutable is transpiled, much of this is a fiction. I'm only looking for "correctness" because flow isn't as forgiving.

The module entry exports only a default, implying that the correct usage of Immutable is:

import Immutable from 'immutable';

However, the flow definition exports each data structure and helper function as a named export. Implying that the correct usage of Immutable is:

import {Map, fromJS} from 'immutable';

This seems a bit odd since many of the exports correspond to common globals. So to avoid shadowing globals (and creating confusion when seeing Map buried deep in code), one would have to use Immutable like this:

import {Map as ImmutableMap, fromJS as ImmutableFromJS} from 'immutable';
// or
import * as Immutable from 'immutable';

So which is correct? The module entry or the flow definition? Or can we have both- Can Immutable export both a default and named exports?

Other discussions: #854 and #855.

[pospi]

In the current versions I'm running- flow 0.29.0 and Immutable 3.8.1 on node 6.3.0, checking

import Immutable from 'immutable';

results in a "This module has no default export" error, whilst

import * as Immutable from 'immutable';

runs fine. I guess that decides the issue, then? I have never had any issues using import { Map } from 'immutable' in this context, either.

[ccorcos]

I believe this should fix it: #961

[zertosh]

@ccorcos: The issue is a little more nuanced than just missing Flow definitions for a default export.

There are huge semantic differences between named and default exports. However, transpilers blur the line because (depending on the implementation), among other reasons, they allow you to treat a module of named exports as a single default export.

Right now there are multiple inconsistencies:

• The actual implementation only exports a default (src/Immutable.js).
• The flow and typescript definitions only have named exports.
• The documentation is written to imply only a default export.

Correcting this is trivial, however, a decision must be made about what is the "correct" export of Immutable. The options are:

(A) Only a default export of an object with everything on it.
(B) Only named exports.
(C) Both a default export with everything and named exports.

Once one is picked, all of the inconsistencies can be fixed. Your PR (#961) is making the decision that (C) is the way to go. Fine. But then the PR is incomplete because the implementation (src/Immutable.js) then needs to have named exports, and the documentation needs to be updated too.

[DavidSnider]

I worry that this also hasn't quite solved the issue raised above:

"This seems a bit odd since many of the exports correspond to common globals. So to avoid shadowing globals (and creating confusion when seeing Map buried deep in code), one would have to use Immutable like this:

import {Map as ImmutableMap, fromJS as ImmutableFromJS} from 'immutable';
// or
import * as Immutable from 'immutable';

"

I feel like this current solution makes it very easy to abuse this to write code that obfuscates code through shadowing globals. Perhaps the better solution might be (A) simply because it makes it more difficult to write bad code. That being said, being forced to import everything isn't necessarily best case either.

[ccorcos]

Interesting... I didn't realize theres an ambiguity around import { X } from './x' working for both export X and export default { X }.

At the very least, I think the flow type definition should represent the codebase (otherwise the type checker doesnt work!). I can change that in the PR.

But as a general discussion, I think destructuring the default export when you import is probably not a good idea. But the stage-0 babel plugin apparently does that just fine. I believe the point of non-default exports is for tree shaking and stuff like that. So I think we should do that. import * as Immutable from 'immutable' is kind of annoying though. But I suppose it would encourage you to just import what you need: import { fromJS, Map, List } from 'immutable'. If there's an objective argument for why thats better, I think thats the way to go. The namespace issue with window.Map can be resolved as @DavidSnider mentioned -- I think code aesthetics should be a deciding factor here.

[leebyron]

IMHO the standard way to use with ES6 imports should be:

import {Map, fromJS} from 'immutable';
// or 
import * as Immutable from 'immutable';

[leebyron]

however it would be nice for convenience to also export an ES6 default such that

import Immutable from 'immutable';

Also continues to work correctly. That might mean a slightly more awkward flow definition file, but that would enable those who use it this way currently to keep doing so without breakage.

[lacker]

I agree with @leebyron - it should be OK to do either of:

import Immutable from 'immutable';
import {Map, fromJS} from 'immutable';

It seems like this is how most things export stuff, React and React Native work this way for example. This is option (C) from @zertosh above.

[leebyron]

Both are now implemented in master and will be released soon

[chriszrc]

This is more of a general question than anything specific to immutable, but is there a way with es6 or typescript to do an import of just the exports we need, but still have them namespaced? This doesn't actually work, but something like this:

import {Map as Immutable.Map, fromJS as Immutable.fromJS} from 'immutable';
//or
import {Map, fromJS} as Immutable from 'immutable';
//so that we could do
Immutable.Map() //etc