New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Flow type defenition #203

Closed
ghost opened this Issue Nov 19, 2014 · 88 comments

Comments

Projects
None yet
@ghost
Copy link

ghost commented Nov 19, 2014

No description provided.

@leebyron leebyron self-assigned this Nov 19, 2014

@leebyron

This comment has been minimized.

Copy link
Collaborator

leebyron commented Nov 19, 2014

Hell yes

@jasonkuhrt

This comment has been minimized.

Copy link

jasonkuhrt commented Nov 23, 2014

@leebyron Curious, I can only assume you knew about flow before us haha, why did you go with TS anyways? Flow just wasn't/isn't as mature?

@leebyron

This comment has been minimized.

Copy link
Collaborator

leebyron commented Nov 24, 2014

TypeScript predates Flow by a while, so when I started this project Flow didn't yet exist. Also, the type definitions are not here for the benefit of the library, but for the benefit of people using the library with these type checking tools. Since a lot of people use TypeScript (myself included) it felt like the right thing to do to include type definition files for it.

Once I get the Flow type definition files, the TypeScript ones will remain in the repository and up to date as well!

@jasonkuhrt

This comment has been minimized.

Copy link

jasonkuhrt commented Nov 24, 2014

@leebyron sweet

@lambdahands

This comment has been minimized.

Copy link

lambdahands commented Nov 25, 2014

@leebyron We had a discusson on #flowtype – wanted to add the definitions file I was working on, which can be found in my fork here.

Would love some feedback and tips on how to treat something like this. I'm really excited to begin implementing Flow into javascript projects that utilize immutable data.

Cheers!

@leebyron

This comment has been minimized.

Copy link
Collaborator

leebyron commented Dec 9, 2014

Sorry for the delay. I'm excited to see progress on this. Nice work.

How have your flow definitions been working? Are there any holes?

@lambdahands

This comment has been minimized.

Copy link

lambdahands commented Dec 9, 2014

Hey, no problem! There are some snags I've run into. I'm not fully acquainted with optional function arguments and how to operate on them in Flow.

Immutable's map and reduce functions use optionals that produce Flow errors; this is the type of code I would typically write as well: https://gist.github.com/lambdahands/3a1caaec0a9eb967b60e

@tmcw

This comment has been minimized.

Copy link
Contributor

tmcw commented Jan 6, 2015

Once I get the Flow type definition files, the TypeScript ones will remain in the repository and up to date as well!

Are you planning on updating the two manually, or is there some Flow/TypeScript syncing/conversion magic that you can employ?

@leebyron

This comment has been minimized.

Copy link
Collaborator

leebyron commented Jan 6, 2015

The flow team is working on the magic auto-conversion, but it's possible that it will be lossy, so the plan will be to update both manually.

@jareware

This comment has been minimized.

Copy link

jareware commented Feb 21, 2015

I'm also very excited about being able to combine Flow and Immutable, but I'm in a similar place with this guy.

Do you guys have opinions on how - if at all - this could work out?

Thanks a lot!

@samwgoldman

This comment has been minimized.

Copy link
Member

samwgoldman commented Feb 21, 2015

I find I use a very small subset of the functionality in this library, so it's not too onerous to maintain my own declaration library, all gratuitously copied from @lambdahands.

@almost

This comment has been minimized.

Copy link

almost commented Apr 14, 2015

Any progress on this? I've started with @lambdahands's file (thanks!) but would be great if it was an officially supported thing

@karel-3d

This comment has been minimized.

Copy link

karel-3d commented Jun 15, 2015

+1

@arypurnomoz

This comment has been minimized.

Copy link

arypurnomoz commented Jun 22, 2015

+2

@pluma

This comment has been minimized.

Copy link
Contributor

pluma commented Jun 22, 2015

+n

@gorillatron

This comment has been minimized.

Copy link

gorillatron commented Jun 26, 2015

+11

@kastermester

This comment has been minimized.

Copy link

kastermester commented Jul 16, 2015

I have taken a stab at trying to improve the work by @lambdahands. Mainly I added support for the static methods to construct immutables. I also addressed an issue with the static of method on Seq, for some reason it seems to conflict with flow that the return type of the method changes in the classes: Seq, IndexedSeq and KeyedSeq.

I have not tested the definition too much so far, but would love some feedback for fixes. I know I have omitted the usual constructor definitions, but it should be easy to add them back in if wanted.

https://gist.github.com/kastermester/ad79da3e48064effd82e

@ghost

This comment has been minimized.

Copy link

ghost commented Aug 4, 2015

Thank you for reporting this issue and appreciate your patience. We've notified the core team for an update on this issue. We're looking for a response within the next 30 days or the issue may be closed.

@gasi

This comment has been minimized.

Copy link
Contributor

gasi commented Aug 21, 2015

👍 for using Flow with Immutable.js 😄

@cpojer

This comment has been minimized.

Copy link

cpojer commented Sep 1, 2015

It's dangerous to go alone! This is the version we have internally at Facebook right now but it is experimental and we don't support it officially yet: https://gist.github.com/cpojer/2dd5e3f4ce41b91a6bd8

The flow team is currently working on improving support for projects like immutable JS so we should have an officially support version of the definition file "soon".

Feel free to fork and contribute to the gist above and let me know how it works out for you. If it is valuable to a lot of people, even if it is not perfect, we might consider making it more official. Personally, the way I see it, having a basic definition file is better than not having one at all. In a project of mine I've successfully adopted the file from above and it is good enough. I had to make some modifications to it and I copy-pasted some type definitions: For example, the return value of a List's filter call is Iterable because that is the type it is inherited from based on the library file above. This means that doing list.filter(fn) will return an Iterable<K, V> instead of a List<T>. To fix this I copied the definitions from Iterable to List (and Map etc.) and turned the return values into List instead of Iterable. I haven't included these local modifications in the gist above but I can add them if it is useful.

Put the file in a lib folder and add it to your .flowconfig like this to make sure Flow doesn't visit your node_modules version of immutable and accidentally types it as any:

[libs] 
lib/

[ignore]
.*/node_modules/immutable/.*

To sanity check if it works, try:

var a: List<number> = List.of('string');

and run Flow – it should yell at you :)

@kastermester

This comment has been minimized.

Copy link

kastermester commented Sep 4, 2015

@cpojer very nice, I'll try it out when I get the time :)

@splodingsocks

This comment has been minimized.

Copy link

splodingsocks commented Sep 16, 2015

WOOOOOO @cpojer Thanks for sharing!!

@splodingsocks

This comment has been minimized.

Copy link

splodingsocks commented Sep 18, 2015

@cpojer I'm attempting to start something like Typescript's Definitely Typed over here: https://github.com/splodingsocks/FlowTyped

I was on my way to add the file you linked, but I noticed that the copyright notice at the top says Facebook, all rights reserved. Does that mean I shouldn't put it in that repo?

@bouk

This comment has been minimized.

Copy link

bouk commented Sep 22, 2015

I've updated @cpojer 's gist to use flow interfaces (flow complained about multiple class inheritance for me otherwise): https://gist.github.com/bouk/d29a6cafab0b9b3b1ec6

The lack of dynamic classes in flow such as those generated by Immutable.Record is unfortunate, but would probably require a big overhaul to support

@nikgraf

This comment has been minimized.

Copy link

nikgraf commented Sep 29, 2015

Am I'm doing something wrong or is it fine that flow is not detecting an error with this code:

var b: Map<string, number> = Map({'key': 'value'});

/cc @bouk

@jergason

This comment has been minimized.

Copy link

jergason commented Sep 29, 2015

Your value isn't a number. That should be an error, so if flow is yelling at you that seems right.

@msolli

This comment has been minimized.

Copy link

msolli commented Oct 6, 2015

I'm using @bouk's interface file and the following config:

[ignore]
.*/node_modules/immutable/.*

[libs]
.*/src/main/javascript/lib/interfaces/.*

Given this JS file:

/* @flow */
import {List} from "immutable";
var a: List<number> = List.of(666);

I get this Flow error:

/Users/<...>/foo.js:2:9,12: List
This type is incompatible with
/Users/<...>/foo.js:2:9,12: List

/Users/<...>/foo.js:4:23,34: call of method of
Error:
/Users/<...>/foo.js:2:9,12: List
This type is incompatible with
/Users/<...>/foo.js:2:9,12: List

Any idea what's going on?

@nikgraf

This comment has been minimized.

Copy link

nikgraf commented Oct 7, 2015

@jergason sorry, I got the text wrong and corrected it above. In the case above Flow didn't point out an error.

@samwgoldman

This comment has been minimized.

Copy link
Member

samwgoldman commented Feb 4, 2016

Well, there are two related, but separable problems here.

  1. Can we declare types for immutable? Probably, most of it, barring some overly dynamic patterns. It's also likely that some of this stuff is theoretically possible, but Flow is missing some key intelligence to make it work in practice. In particular, static, literal information can easily be "lost" in all but the simplest cases. Also, complex types tend to report complex errors, which limits the utility of the tool.

  2. Can we machine verify an implementation against a type? This part can be a lot harder than (1). Need to nail down the first bit first, but I'm not super optimistic. Thankfully, that doesn't really apply in this case, because we're declaring types anyway—unless Immutable.js starts using Flow internally, but the value prop is unclear there since it's already mature and battle tested.

Maybe I misunderstood your question, though. Regardless of the specific mechanism, we'll need some custom, specific (read: hardcoded) support for getIn et al., but hopefully we can decompose that into reusable primitives, because that kind of dynamic property access is a common JS idiom. You also see it in Falcor, for example.

OK, probably doing more harm than good pontificating wildly in a public forum. I need to pop a few things off my stack before I can even look into this stuff.

@nelix

This comment has been minimized.

Copy link

nelix commented Feb 17, 2016

Is this a case that perhaps making some changes to immutable might be easier to work with? Maybe even a type safe subset (import TypedImmutable from 'immutable').
Or maybe its just a case of when we consume immutable, we abstract it and type our abstractions

@hallettj

This comment has been minimized.

Copy link

hallettj commented Feb 22, 2016

Hello folks; I am another person interested in type definitions for Immutable. I have put some work into this previously. After reading the discussion here, I was able to put together a file that I have high hopes for:

https://github.com/hallettj/immutable-js/blob/flow-types/type-definitions/immutable.js.flow

I want to do a bit more testing before opening a pull request.

I have a lot of notes here, so I am going to break them into sections.

.js.flow file

I chose to add a .js.flow file to the Immutable package instead of creating an interface file. This way, anyone who pulls in Immutable as a dependency will pick up the type definitions automatically (if they use Flow).

dynamic behavior

I tried to make the definitions flexible, to reflect possible real-world, dynamic use. For example, when pushing a value onto a List, the type of the new value might not match types of values already in the list. I wrote a signature that permits the type of the resulting list to "broaden":

push<U>(...values: U[]): List<T|U>;

Where T is the type of values already in the list. In practice, if you have a list of numbers and push a string, then Flow will treat values in the resulting list as being either numbers or strings.

I was very pleased to learn from @samwgoldman about overload syntax for methods. That let me write signatures like this, which I find to be very convenient:

declare class Iterable<K,V> {
  // ...
  get<V_>(key: K, notSetValue: V_): V|V_;
  get(key: K): ?V;
  // ...
}

If you use the get method, this signature says that the result might be undefined if you do not provide a notSetValue. But if you do provide a notSetValue, then the result will not be undefined (unless the iterable actually contains undefined values).

interaction with ES2015 Iterable

I noticed that a lot of Immutable methods can operate an any Iterable value - that being the ECMAScript 2015 Iterable type, as opposed to the Immutable Iterable type. So I made heavy use of this type alias:

/*
 * Alias for ECMAScript `Iterable` type, declared in
 * https://github.com/facebook/flow/blob/master/lib/core.js
 *
 * Note that Immutable values implement the `ESIterable` interface.
 */
type ESIterable<T> = $Iterable<T,void,void>;

This allows many of the constructor signatures to be simplified. The constructor signature that I wrote for Map, for example, is:

declare class Map<K,V> extends KeyedCollection<K,V> {
  static <V>(obj?: {[key: string]: V}): Map<string, V>;
  static <K, V>(iterable?: ESIterable<[K,V]>): Map<K, V>;

  // ...
}

This builds on the feature that all Immutable types implement the ECMAScript 2015 Iterable interface. When iterating over Keyed iterable types you get map entries (key-value pairs) - which happens to be compatible with the Map constructor overload for creating a map based on an existing Keyed iterable.

the problem with subclasses as static properties

The biggest problem that I have not been able to solve is expressing that Iterable has the static properties Keyed, Indexed, and Set; and that Collection and Seq have the same static properties, but with different types. Collection and Seq are subclasses of Iterable, so Flow thinks that static properties with the same name as the superclass should have the same type. (That does seem like a reasonable constraint to me, actually).

It also seems that there might be a recursion issue with a class holding a reference to its subclasses.

For the moment I put in this cop-out:

declare class Iterable<K,V> {
  static Keyed:   Class<Object>;
  static Indexed: Class<Object>;
  static Set:     Class<Object>;
  // ...
}

Collection and Seq declare the same properties.

I export KeyedIterable, IndexedIterable, SetIterable, and the related Collection and Seq types, even though those names do not exist in the top-level namespace anymore. That way users can import those names as types, since using Iterable.Keyed and friends in type expressions will not produce satisfactory results with the above cop-out.

Records

I made some headway on nice type-checking for Records. The most useful approach that I have found so far is to declare Record types in this fashion:

type ConversationSpec = {
  id:            string,
  activities:    List<Object>,
  allActivities: List<Object>,
  subject:       ?string,
}

type Conversation = ConversationSpec & Record<$Keys<ConversationSpec>>

const ConversationRecord = Record({
  id:            '',
  activities:    List(),
  allActivities: List(),
  subject:       null,
})

That gives you a type, Conversation that has these features:

  • Flow infers that Conversation has the Record methods, get, set, etc.
  • When creating a record, Flow verifies that keys and types given to the constructor match with ConstructorSpec.
  • Flow is also able to check types of Record fields when they are accessed using dot notation.
  • Flow will check that keys are valid for the record when using get and set, but will not check the types of values.

interfaces vs mixins

@bouk: One important change that I made from your declaration file is to use classes for everything instead of interfaces. My examination of Immutable's implementation indicates that everything is a class under the hood. But in the actual type hierarchy, some relations are subclasses and some are mixins. For example, IndexedCollection is properly a subclass of Collection and Iterable - but it is not a subclass of IndexedIterable. Instead, it appears that the IndexedIterable methods are copied onto the IndexedCollection prototype.

I used the Flow mixin keyword which, if I understand correctly, captures that relationship:

declare class IndexedCollection<T> extends Collection<number,T> mixins IndexedIterable<T> {
  toSeq(): IndexedSeq<T>;
}

It seems that Flow requires mixins to be values (such as classes). Interfaces do not work.

dealing with ambiguity in overloaded functions

@bouk, @samwgoldman: I also ran into issues with Flow choosing the wrong signature in an overloaded function. After some experimentation, I saw nice results with null-terminated argument lists. Here is a compose function that has been working well for me:

declare var compose: (<A,B,C,D,E>( fn3: (d: D) => E
                                 , fn2: (c: C) => D
                                 , fn1: (b: B) => C
                                 , fn0: (a: A) => B
                                 , $?: null
                                 ) => (a: A) => E)
                   & (<A,B,C,D>( fn2: (c: C) => D
                               , fn1: (b: B) => C
                               , fn0: (a: A) => B
                               , $?: null
                               ) => (a: A) => D)
                   & (<A,B,C>( fn1: (b: B) => C
                             , fn0: (a: A) => B
                             , $?: null
                             ) => (a: A) => C)
                   & (<A,B>( fn0: (a: A) => B
                           , $?: null
                           ) => (a: A) => B)

function compose(...functions) {
  return functions.reduceRight((accum, fn) => x => fn(accum(x)))
}

Oddly, when I try to extend this type with one more signature that accepts five function arguments, I stop getting type errors for incorrect uses. At least that was the case with Flow 0.21.

I used a similar type for IndexedIterable#zip.

@hallettj

This comment has been minimized.

Copy link

hallettj commented Feb 22, 2016

@jedwards1211, @samwgoldman, @leebyron: I am not inclined to put effort into making getIn, updateIn, etc. type-check correctly because I think that would be hard, and lenses are an existing solution that provides the same functionality, but with type safety.

In particular, I want to plug my own library: safety-lens. If you consider this code using Immutable:

import { List, Map } from 'immutable'

let calendar = List.of(
  Map({ date: new Date, title: 'get coffee' }),
  Map({ date: new Date, title: 'plan day' })
)

assert( calendar.getIn([0, 'title']) === 'get coffee' )

The equivalent code using lenses looks this:

import { compose, lookup } from 'safety-lens'
import { index, key } from 'safety-lens/immutable'

const firstTitleLens = compose(index(0), key('title'))

assert( lookup(firstTitleLens, calendar) === 'get coffee' )

But lenses also work on non-Immutable values. In my testing, this works with lenses, but does not work with getIn:

import { prop } from 'safety-lens/es2015'

calendar = List.of(
  { date: new Date, title: 'get coffee' },  // vanilla objects
  { date: new Date, title: 'plan day' }
)

firstTitleLens = compose(index(0), prop('title'))

assert( lookup(firstTitleLens, calendar) === 'get coffee' )

And lenses keep track of types through multiple layers of data structure nesting:

const badLens = compose(index(0), prop('title_mispelled'))

lookup(badLens, calendar)

// flow error:
// 
//  42: badLens = compose(index(0), prop('title_misspelled'))
//                                       ^^^^^^^^^^^^^^^^^^ string literal `title_misspelled`. Property not found in
//  39:   { date: new Date, title: 'plan day' }
//        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ object literal
@corbt

This comment has been minimized.

Copy link

corbt commented Feb 22, 2016

@hallettj the type definition looks awesome to me. Also thanks for plugging lenses, I haven't used them before but it looks like an interesting concept.

@karel-3d

This comment has been minimized.

Copy link

karel-3d commented Apr 28, 2016

@hallettj hey, this looks awesome. I will definitely start experimenting with immutable + your lenses + flow. Will tell you if there are some bugs!

@karel-3d

This comment has been minimized.

Copy link

karel-3d commented Apr 28, 2016

@hallettj you don't have issues on your repo, so I am writing here

How is the Record working now? Your code from this thread doesn't work now, but type Conversation = ConversationSpec & Record<ConversationSpec> works.

Is the ConversationSpec & Record<ConversationSpec> still needed?

@chrisui

This comment has been minimized.

Copy link

chrisui commented Apr 28, 2016

@Runn1ng Check out this comment from the linked PR #805 (comment)

@karel-3d

This comment has been minimized.

Copy link

karel-3d commented Apr 28, 2016

Hm, no, it's not working, it doesn't seem to check anything and lets anything through.

What would happen, if I rewrite this

static <T: Object>(spec: T, name?: string): /*T & Record<T>*/any;

to this

static <T: Object>(spec: T, name?: string): T & Record<T>;

? It seems to check correctly; however it's probably commented out for some reason.

@karel-3d

This comment has been minimized.

Copy link

karel-3d commented Apr 28, 2016

@chrisui ha! didn't see that PR! awesome

@glenjamin

This comment has been minimized.

Copy link
Contributor

glenjamin commented Jun 22, 2016

In case anyone is interested, I'm using this approach for records, which is working pretty well: https://gist.github.com/glenjamin/75a96b45f4bb5c6ac221815d28c548dd

Although the errors it generates when something is wrong are not ideal.

@iansinnott

This comment has been minimized.

Copy link

iansinnott commented Jun 22, 2016

I've run into some issues when using OrderedMap. It seems that since the type definition for OrderedMap simply extends that of Map calling methods on an OrderedMap instance will return the wrong type. Namely Map instead of OrderedMap.

An example:

/* @flow */
import * as Immutable from 'immutable';
import type { OrderedMap } from 'immutable';

const oMap: OrderedMap = Immutable.OrderedMap({ a: 'b' });
const oMap2: OrderedMap = oMap.set('c', 'd');

The above code has two errors when I run it through Flow:

src/app/modules/alerts/stores.js:5
  5: const oMap: OrderedMap = Immutable.OrderedMap({ a: 'b' });
                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Map. This type is incompatible with
  5: const oMap: OrderedMap = Immutable.OrderedMap({ a: 'b' });
                 ^^^^^^^^^^ OrderedMap

src/app/modules/alerts/stores.js:6
  6: const oMap2: OrderedMap = oMap.set('c', 'd');
                               ^^^^^^^^^^^^^^^^^^ Map. This type is incompatible with
  6: const oMap2: OrderedMap = oMap.set('c', 'd');
                  ^^^^^^^^^^ OrderedMap

It seems that both the constructor and the set method are returning a Map in Flow's eyes, and not an OrderedMap as would be expected.

Looking at the type definitions file (immutable.js.flow) it looks like this is happening because every method of Map returns a Map type. I'm not sure if there is an elegant way to write "return the same type as this object". Something like typeof this.

@MakotoUchimoto

This comment has been minimized.

Copy link

MakotoUchimoto commented Jun 22, 2016

immutable-js@noreply.github.com

🍀

2016/06/23 5:22、Ian Sinnott notifications@github.com のメッセージ:

I've run into some issues when using OrderedMap. It seems that since the type definition for OrderedMap simply extends that of Map calling methods on an OrderedMap instance will return the wrong type. Namely Map instead of OrderedMap.

An example:

/* @flow */
import * as Immutable from 'immutable';
import type { OrderedMap } from 'immutable';

const oMap: OrderedMap = Immutable.OrderedMap({ a: 'b' });
const oMap2: OrderedMap = oMap.set('c', 'd');
The above code has two errors when I run it through Flow:

src/app/modules/alerts/stores.js:5
5: const oMap: OrderedMap = Immutable.OrderedMap({ a: 'b' });
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Map. This type is incompatible with
5: const oMap: OrderedMap = Immutable.OrderedMap({ a: 'b' });
^^^^^^^^^^ OrderedMap

src/app/modules/alerts/stores.js:6
6: const oMap2: OrderedMap = oMap.set('c', 'd');
^^^^^^^^^^^^^^^^^^ Map. This type is incompatible with
6: const oMap2: OrderedMap = oMap.set('c', 'd');
^^^^^^^^^^ OrderedMap
It seems that both the constructor and the set method are returning a Map in Flow's eyes, and not an OrderedMap as would be expected.

Looking at the type definitions file (immutable.js.flow) it looks like this is happening because every method of Map returns a Map type. I'm not sure if there is an elegant way to write "return the same type as this object". Something like typeof this.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

@mhagmajer

This comment has been minimized.

Copy link

mhagmajer commented Jun 22, 2016

@leebyron, I wish that Immutable.js ships with official flowtype definitions one day given how popular the library is... :)

@leebyron

This comment has been minimized.

Copy link
Collaborator

leebyron commented Jun 22, 2016

Sorry this issue should be closed. Flow types were added in the last major version.

See: https://github.com/facebook/immutable-js/blob/master/type-definitions/immutable.js.flow

@leebyron leebyron closed this Jun 22, 2016

@ccorcos

This comment has been minimized.

Copy link

ccorcos commented Aug 25, 2016

How do I actually write a type definition using this?

const State : ??? = Immutable.fromJS({ name: 'chet', age: 25, tags: ['something']})

That is, how do I say something is an Immutable Map with a certain schema?

@kayneb

This comment has been minimized.

Copy link

kayneb commented Aug 29, 2016

I'm still unable to consume the definition added with Flow v0.31.0. I have added node_modules/immutable/dist/immutable.js.flow to my [libs], but when I import with import * as Immutable from 'immutable';, I get the error:

3: import * as Immutable from 'immutable';
                                ^^^^^^^^^^^ immutable. Required module not found

Any idea what I'm missing? For reference, here is my .flowconfig:

[ignore]
<PROJECT_ROOT>/node_modules/.+\.json$
<PROJECT_ROOT>/node_modules/.+/node_modules/.+$
<PROJECT_ROOT>/node_modules/fbjs/.+$

[libs]
src/main/public/interfaces
src/main/javascript/js/bootstrap.js.flow
node_modules/iflow-react-router-redux/index.js.flow
node_modules/immutable/dist/immutable.js.flow

[options]
module.system=haste

module.file_ext=.js
module.file_ext=.less

module.name_mapper.extension='less' -> 'LessModule'

@stephanos

This comment has been minimized.

Copy link

stephanos commented Aug 29, 2016

@kayneb I had the same issue. I wrapped the whole file content with declare module 'immutable' { ... } and then it seemed to work. Don't know why that is necessary, though :-?

@volkanunsal

This comment has been minimized.

Copy link

volkanunsal commented Sep 3, 2016

@kayneb I used your configuration file and through some trial and error discovered that this line is causing Flow to ignore immutable library for whatever reason. If anyone knows why, I'd be interested to find out.

<PROJECT_ROOT>/node_modules/.+\.json$
@volkanunsal

This comment has been minimized.

Copy link

volkanunsal commented Sep 3, 2016

I'll answer my own question. This is because the line above also excludes package.json and without it Flow doesn't know where to find the entrypoint of the module.

@hmeerlo

This comment has been minimized.

Copy link

hmeerlo commented Nov 5, 2016

@leebyron Any chance the recent 0.34 of flowtype can improve on the support for Record annotations?

@mvolkmann

This comment has been minimized.

Copy link

mvolkmann commented Nov 24, 2016

Why is it that even though this issue is closed because definitions for Immutable exist now, I don't see definitions for Immutable at https://github.com/flowtype/flow-typed/tree/master/definitions/npm?

@glenjamin

This comment has been minimized.

Copy link
Contributor

glenjamin commented Nov 24, 2016

@iansinnott

This comment has been minimized.

Copy link

iansinnott commented Nov 24, 2016

There's also a work in progress PR out to add the definitions to flow typed: flow-typed/flow-typed#401

@karel-3d

This comment has been minimized.

Copy link

karel-3d commented Dec 2, 2016

Just for interest. I have made for myself a Record class wrapper, that checks Records for me (works better - for me - than the definitions in here). It's not complete, but it checks the types of the parameters, which is what I wanted; I don't use getIn/ get/set/setIn.

The small wrapper looks like this

/* @flow */

import {Record as ImmutableRecord} from 'immutable';

export type Record<Def: Object> = {
  merge: (o?: $Shape<Def>) => Record<Def>;
} & Def;

type $RecordCreator<Def: Object> = (def?: $Shape<Def>) => Record<Def>;

export function RecordC<Def: Object>(definition: Def): $RecordCreator<Def> {
  const RecordClass = ImmutableRecord(definition);
  return (newDef) => {
      return new RecordClass(newDef);
  };
}

You can only use the "default" getters, but it checks them correctly; you cannot use set, you can only use merge, but it checks the types correctly (unlike using set with this definition). I use it like this

/* @flow */

import type {Record} from './record'
import {RecordC} from './record'

const ABRecordC = RecordC({a: 3, b:3});
const recW = ABRecordC({b: 2});
recW.merge(); //works
recW.c; // throws error
recW.a; 

// or explicitly
type CDRecord = Record<{c: number, d: number}> 
const CDRecordC = RecordC({c: 3, d:3});
const rec: CDRecord = CDRecordC();
rec.merge();
rec.merge({c: 3});
rec.merge({c: 'foo'}); // throws error
rec.c;
rec.a; // throws error

I have to do the dance with new because of this - facebook/flow#2930

It works as I want to. You can use it if you want

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment