Contribute newer flow definitions for v3.x to flow-typed

[jedwards1211]

As discussed in facebook/flow#4917, .js.flow files provide no way to simultaneously support different versions of Flow. However, flow-typed allows you to store defs for multiple versions of flow for a single version of immutable.

This is the first snag I've hit trying to upgrade the dozens of packages in my company to Flow 0.56; to get compatible defs, I would have to install a release candidate of immutable, which I don't want to do for production code, so I have no choice but to wait or ignore any flow checking on immutable altogether.

So, until if and when flow supports defs for multiple flow versions within a package itself, I'd propose storing the typedefs in flow-typed as soon as possible instead of in this package.

Thoughts?

[bassjacob]

I've written a vendored immutable binding (for the 3.x.x branch) that seems to work in my projects: https://gist.github.com/bassjacob/b23e7bde65291ed927d46d12ccc1512f

I've placed that file in flow-typed and ignored the immutable lib from node_modules.

I had to change the format from the file in the repo because it declares some classes and types globally which conflict with React. @leebyron are the declare class Iterable and declare class _Iterable global on purpose? And should they be brought inline with the definitions in the react bindings?

[bassjacob]

(Also, please let me know if that gist is unsound or broken in some way. I'm fairly new to writing flow bindings)

[leebyron]

I'm more than happy to support community led efforts for newer flow types for prior versions of this library, unfortunately that's not something I have time to do myself. I encourage anyone interested in this to contribute pull requests to the flow-typed repo.

I've tagged this issue as help wanted in case anyone would like to contribute

This repo will continue to host (and distribute on npm) a flow definition file for the latest shipping version. To get the best type definitions, we may rely on newer flow features that require a newer version of flow.

[bassjacob]

I'd be happy to PR this to flow-typed and maintain it. I'd be a little concerned at deviating too far from the internal types for this lib, but I don't think it should be too large a job.

Do you have any information around the Iterable problem I raised above? That's the biggest difficulty I've faced. The second would be removing the use of any in some of the return types for the bindings so they can be flow-checked.

[leebyron]

Do you have any information around the Iterable problem I raised above?

v3 exports a type with this name, so a flow definition file should type it. It was renamed in v4 to avoid this exact ambiguity. To account for this we defined an ESIterable type so both could be used in the definition file.

declare class is only available to the containing scope. For the file contained within this project, that's the exported module, so there is no overwriting of any true globals. However I believe flow-typed definitions operate on the true global scope, so you'll need to wrap it in a module {} definition.

The second would be removing the use of any in some of the return types for the bindings so they can be flow-checked.

You can take a look at the improvements in the v4 definitions which have dramatically reduced the reliance of any - though some uses may still be required for cases where proper types cannot be defined.

[bassjacob]

That makes total sense. I didn't realise flow would scope .js.flow files inside node_modules to only that module. I've wrapped the file for flow-typed in a module so it doesn't conflict and I'll check out the changes in the v4 branch to see what else can be done.

Thanks for giving me some pointers 😄 I'll report back here once things are moving.