Collection records

[Gozala]

I've being using Records for modelling application state and soon enough run into a case where I'd like to specify a record field that must be a collection of other Records. Something along these lines:

const Suggestion = Record({ url: '', title: '' })

const Completer = Record({
  selected: -1
  entries: List()
})

Here I'm unable to specify that Completer entries should be a Suggestion records. I would love if there was a way to declare that somewhat along these lines:

const Suggestion = Record({ url: '', title: '' })

const Completer = Record({
  selected: -1
  entries: Suggestion.List()
})

[leebyron]

I think you may be mis-interpretting the initial value for the Record class. It is intentional that you're unable to specify what type each field is because type checking will quickly expand into a very large (and possibly expensive) extension to this codebase in a really unrelated matter.

What the object provided to Record describes is the list of valid keys on that Record and the default values for each key, not their type. For example: to articulate the difference between entries, a list of Suggestion records, having a default value of empty list List() or null (e.g. should entries be nullable?) you could write either:

const Completer = Record({
  selected: -1 // Number
  entries: List() // List<Suggestion>
})

or

const Completer = Record({
  selected: -1 // Number
  entries: null // ?List<Suggestion>
})

Does this make sense?

[leebyron]

I'm very interested in layering type information atop Record, I just don't think this library is the right place for this to live. I see a few ways forward:

1. Integration with static type checking tools like Flow and Typescript. This would allow the additional type information to be removed at runtime, offering zero overhead and allow for full program analysis ahead of time.
2. A separate library which supplies runtime type checking for Record (and presumably the rest of Immutable.js). This will come at a serious runtime cost, but will enable the strongest sense of safety. That cost (both runtime cost and library size) is not one that every user of Immutable.js should pay, which is why I think this needs to be it's own library if desired.

[leebyron]

I now understand from your pull-requests that you're interested not just in type validation but parsing an arbitrary JS body into a tree of Immutable collections. That's also super interesting and something I would love to see, but probably should not be specific to Record, but apply to all Immutable collections.

Something like that would probably have a different sort of API from the Immutable collection constructors. For example, as you discuss here, you can't simply write: List(MyRecord) and expect it to operate as a type checker, for the same reason you can't write Record({number:Number}) and expect that to operate as a type checker. Both cases expect runtime values. We will need something else for schema-based object inflation.

[Gozala]

I now understand from your pull-requests that you're interested not just in type validation but parsing an arbitrary JS body into a tree of Immutable collections. That's also super interesting and something I would love to see, but probably should not be specific to Record, but apply to all Immutable collections.

Indeed! I'm not actually interested in adding a type checking to the records, what I'm interested in is to allow polymorphic record transformation functions that don't need to be a record type specific.

Something like that would probably have a different sort of API from the Immutable collection constructors. For example, as you discuss here, you can't simply write: List(MyRecord) and expect it to operate as a type checker, for the same reason you can't write Record({number:Number}) and expect that to operate as a type checker. Both cases expect runtime values. We will need something else for schema-based object inflation.

I'm not sure I am following what you're trying to say here. Could you maybe explain it little differently ? Also would you mind proposing what the API for that could look like ?

[Gozala]

To put it from a different perspective I want to get following:

1. To be able to serialise records into JSON and then construct them back from the JSON producing equal values:

   let Point = Immutable.Record({ x: 0, y: 0 })
   
   let point = Point({x: 10})
   point.equals(Point(point.toJSON())) // => true
   
   let Line = Immutable.Record({start: Point(), end: Point() })
   let line = Line({start: Point({x: 10})})
   
   line.equals(Line(line.toJSON())) // => false

   As you can see from the example right now it only works if you have flat records.

2. Ability to define polymorphic functions that can set / push entries into record without needing to instantiate concrete record instances as that would imply function per record type or method definition per record type with a possible name collisions.

[Gozala]

As of runtime costs I agree this is not free lunch, but I don't think in practice it is going to be different from a normal records use as is. Let me elaborate on this:

1. If you do not use sub-records only added runtime cost is instanceof check for set operations.

2. If user does use sub-records most likely user already does the same sub-Record construction by hand:

   line.set('start', Point({x: 1})

   Or something slower when executed from a polymorphic function:

   line.remove('start').setIn(['start', 'x'], 1)

   P.S: polymorphic function that do .set is actually not that bad, but it's a lot more difficult with an .update that we tend to use a lot more.

3. If user does set / update with in the record like line.setIn(['start', 'x', 10) or line.updateIn(['start', 'x'], inc) there is no added runtime cost.

So unless I'm missing something only added runtime cost is instanceof check per modification operation (which I suspect js engines are well optimised for) and if it is still too much it could probably be optimised further by generating specialised setters per key during RecordType construction.

[Gozala]

I will understand if you'll say this is out of scope for this library and maybe defining whole new library would be a better way to go about it. Although I would still appreciate feedback from you on some of the points I tried to make above, as I could be missing some details.

[Gozala]

For example, as you discuss here, you can't simply write: List(MyRecord) and expect it to operate as a type checker, for the same reason you can't write Record({number:Number}) and expect that to operate as a type checker. Both cases expect runtime values. We will need something else for schema-based object inflation.

Putting a typecheker part of this aside, with this change ListRecord(RecordType)() kind of does exactly with RecordType.List as a syntax sugar. In fact I did also wanted to propose another addition in a following iteration:

const Point = Record({x: Record.Number(0),
                      y: Record.Number(0)})

That would add optional schema validation when desired. In fact I wanted to propose React like props validation as well:

const Email = Record.Field(value =>
  emailRegExp.test(value) ? value : TypeError('Invalid email address')

const Login = Record({
  user: Email('user@gmail.com'),
  ...
})

[Gozala]

BTW implementing this as third party library is not quite visible right now because:

1. List implementation is not sub-classing friendly as part of implementation presumes that instances produced can be created using Object.create(List.prototype) and that there can be only single empty list. I had to make following changes to List.js to be able to subclass it:
   https://github.com/facebook/immutable-js/pull/413/files#diff-be56562c9370c7fbf67b5bfca397aac5
2. As you noticed in the pull request the way I construct list subclasses is far from ideal due to the fact that List constructor does a lot and isn't really extendable. It would be great if that could also be changed to make subclassing a List easier.

If you'll be more in favour of having a separate library (which I'll understand) maybe we could at least make Immutable Types more subclassing friendly so one would not need to maintain a fork of List.js and possibly other types to achieve this.

[tgriesser]

@Gozala check out #368 for a proposed approach of subclassing Immutable types across the board - I've been using it quite a bit it's been working great so far.

[Gozala]

@tgriesser thanks for pointing that out, indeed that would greatly simplify this pull request or allow defining this as a separate library. Any ideas when is it going to land ?

[tgriesser]

@leebyron mentioned there's a better benchmarking system in the works internally so once that's available we can have more certainty there aren't any regressions in terms of performance across the board, though with the current npm run perf these are the results (pretty impressive performance gains):

There's one (minor) caveat, if you try to add something like a metadata API to an object, or any other case where empty state is signaled by more than just value equality of the Map, List, etc., calling something like clear on the type will cause it to go back to a full empty state for now.

I believe I have a good fix for this but it modifies even more of the internals and didn't want to go any further in the PR until there's more discussion.

[leebyron]

Thanks for all the detailed input @Gozala.

To be able to serialise records into JSON and then construct them back from the JSON producing equal values

This is not possible in all cases because converting to JSON is lossy. In some simplified common cases it's possible though. I understand your overall point of wanting to convert from JSON in a deep way. This points to a need for a serialization proposal which I'm interested in, but is maybe out of scope for this task.

List implementation is not sub-classing friendly

This is true, and @tgriesser has good ideas on how to improve this. However, I'm not sure that I'm convinced a subclass of List is necessary in order to properly parse JSON to Records and Lists. I understand you want this to be able to override the set/merge/etc methods, but I'm not sure this is strictly necessary.

In fact I did also wanted to propose another addition in a following iteration:

const Point = Record({x: Record.Number(0),
                      y: Record.Number(0)})

Something along these lines is more interesting to me. I think we can probably get closer to the syntax of your original proposal by differentiating when providing a Record constructor instead of a Record value in the default argument position. For example:

const Point = Record({ x: 0, y: 0 });
const Line = Record({ start: Point, end: Point });

In this case, the Point refers to the Point constructor. We can then infer that the fromJS should parse these fields using this constructor, and the default value will be the default values of the Point, e.g. Point().

This would preclude the ability to define a record constructor as the default value for a Record field, but that use case is probably very rare. I cannot think of a case where that's what you would expect.

My one real concern with an API like this is that it may be confusing to see an intermixing of default values and types. I foresee misreading examples like this and mistakenly writing:

const Point = Record({ x: number, y: number }); 

which is a reference error, or:

const Point = Record({ x: Number, y: Number }); 

Which simply wouldn't do what you expect (Number function as a default value). We can chip away at these cases, but it still makes me nervous.

The remaining case is for Lists of a particular type, but by extension also Map, Set, OrderedMap, etc. As well as all the future possible data structures we may add to the library, or extenders of the library may define (especially if we follow @tgriesser's extension proposal). I don't think requiring subclassing is the answer, but I'm not sure what the answer is.

Rough and dirty sketches of an API might look like:

points: Record.Type(List, Point)
points: Record.Type(js => List(Seq(js).map(Point))
points: Record.Type(List, item => Point(item))

Not sure, exactly. This needs more exploration.

[leebyron]

For the first part of this: nested Records, I have a diff pushed to the 4.0 branch here: 5001072. This is based on your pull request, I actually borrowed the tests directly.

I've made some generalizations to hopefully make this even more useful - not being limited to only Records as the type factories. This made using Number, Boolean, and String possible to use directly which provides some of the runtime type safety we were interested in without being a wart. It also let me add a Nullable factory function which is important when assembling a runtime type system like this. By leaving these factories as just functions, I'm also leaving the future open. I imagine at some point, someone will want something like Union, but not today.

The 4.0 branch was set up to start tracking changes like these that are going to be breaking. There's no set target date for launching 4.0, but having the space will allow for some exploration. Consider it an alpha branch.

[Gozala]

This is not possible in all cases because converting to JSON is lossy. In some simplified common cases it's possible though.

Could you please elaborate, example of where it's not going to work ? I understand that conversion to JSON is lossy, but (with proposed changes) RecordType does hold enough information to be able to reconstruct it back by parsing generated JSON. But it could be that I'm missing something.

However, I'm not sure that I'm convinced a subclass of List is necessary in order to properly parse JSON to Records and Lists.

Parsing from serialised JSON is just one part of it. The other that I think you missed is ability to have a polymorphic functions that operate on the different RecordTypes.

To give you more insight here is how I got to where I am now with this.

1. We are working on an application that uses react form via omniscient.js and all our state is implemented with immutable.js data structures (basically maps & list). All of the application state is in central immutable map. And we used cursors to allow separation of concerns between components (although we're moving away from them).
2. As application got large enough it became little cumbersome to deal with a huge mostly unstructured data. In addition use of .get instead of destructuring was little annoying. So we considered using Records instead.
3. Once we started using records we've quickly run into several issues:
   1. Restoring from persisting app state in DB became more difficult (it still restores but we essentially loose all the properties that made us consider records instead.
   2. Many functions that were used on different subsections of the state were no longer applicable Record types the dealt there were different (even though sub-shapes that functions operate are same).
4. Te resolve first issue we end up writing additional functions that would basically destruct inputs and map parts of it before passing to the TypeRecord that it would return. This end up generating huge amounts of code and any structure change would require this guardian function change as well.
5. Later we created a RecordType decorator function that was basically doing more or less what you see in the pull. Figuring out field types extracting their constructors so that it could map over data before it would pass it to the RecordType itself & restoring from app state as well.
6. We still were running into issues though that sometimes some of the lists would contain data that was not of he Record type that List was supposed to contain. Mostly it was going down to the issue of transformation functions that would update record with some logic but return would be a JS object not the record type it took.
7. Some cases were resolved by making function per Record type in other cases functions would actually sniff for the other records and create a new one with it's constructor. This allowed for some kind of polymorphic functions but had another issue that RecordType again if had nested structures creating data with it's constructor was not enough it needed to use our decorator instead etc...

At the end of the day yes you can use Records as is but it requires a lot extra code and decorators and discipline in writing functions that operate on them. With a changes in this pull request we were able to fix most issues and get rid of all the glue code that made whole experience a lot nicer.

I understand you want this to be able to override the set/merge/etc methods, but I'm not sure this is strictly necessary.

I hope lengthy story above explained why I feel this is necessary.

[Gozala]

In this case, the Point refers to the Point constructor. We can then infer that the fromJS should parse these fields using this constructor, and the default value will be the default values of the Point, e.g. Point().

Inference in the pull was pretty much doing that, it was looking at the ._defaultValues and if they were instances of Records it would infer via their constructors.

[Gozala]

For the first part of this: nested Records, I have a diff pushed to the 4.0 branch here: 5001072. This is based on your pull request, I actually borrowed the tests directly.

Great thanks a lot!

[leebyron]

Your rationale is sound here. I totally agree that having a clean way to parse JSON into Immutable collections is really valuable. The trick is getting the semantics and API right so it doesn't preclude other valuable use cases and the API leads you to what you might expect. Hopefully the diff I pushed to 4.0 branch is on the right point of that scale, though it doesn't tackle non-Record types yet. I'd like to brainstorm a bit more on the appropriate API and architecture to enable this kind of factory function on input data across the suite of collections.

Could you please elaborate, example of where it's not going to work ? I understand that conversion to JSON is lossy, but (with proposed changes) RecordType does hold enough information to be able to reconstruct it back by parsing generated JSON. But it could be that I'm missing something.

There are a ton of small edge cases where information is lost, but there are also larger representation issues. Certainly having a detailed tree of type information that mirrors a JSON body helps enormously, but the toJS() form on Maps produces an Object, but a serialization API would probably want to produce and consume a list of tuples as JS Object keys can only represent strings and their ordering semantics are awkward at best and ill-defined at worst.

[Gozala]

@leebyron I have few more questions:

1. Did I managed to make a compelling case for RecordLists that are the key to having polymorphic functions without workarounds described earlier ? Asking because all your responses seem to be related to JSON parsing.

2. How do I help to move this forward ? I do understand you do not want to rush this and rightfully so. I just don't know if you'd like to have more discussion or some more code or maybe just time to think through this.

3. I do need to move the project I'm working on forward & given the issues I described earlier I do believe I have only following options:

   1. Use a patched up version & hope that it would at some point in one form or other make it into upstream.
   2. Make a separate library built on top of immutable.js (I think this has a benefit of providing some prior art that you could consider in making a call on this), but this seems difficult without Custom Immutable Classes, Round 2 #368 because some of the assumptions in the List implementation. In best case scenario I might get away but copying most of List.prototype methods onto custom one and overriding some of them, but then I'd be touching too many intimate parts of immutable.js to be comfortable with.

   Which option would you recommend ? Maybe there is yet another option that I even fail to see.

Thanks!

[leebyron]

Did I managed to make a compelling case for RecordLists that are the key to having polymorphic functions without workarounds described earlier

The need for a List that contains Records is pretty clear. The mechanism for doing so, less clear. I'd like to avoid subclassing if at all possible. I have some early thoughts, but nothing implementable yet.

How do I help to move this forward ? I do understand you do not want to rush this and rightfully so. I just don't know if you'd like to have more discussion or some more code or maybe just time to think through this.

I think I need more time to stew on possible ways to implement this that have the properties:

• Very small overhead on existing library
• Highly performant
• Works for all Immutable collections, current, future, and 3rd party.

If you have thoughts on how to accomplish this, I'm happy to keep spit-balling.

I do need to move the project I'm working on forward & given the issues I described earlier I do believe I have only following options:

I think fork and patch is probably going to work for you. You can unlock a working but maybe not final API, and by the time we agree on something that works best, it will probably be quite easy for you to code-mod your codebase to fit the new API. Building up a separate library sounds like a lot of work given that I really like the idea and want to see it happen here eventually.

[Gozala]

I think I need more time to stew on possible ways to implement this that have the properties:

Very small overhead on existing library

As long as you keep typed fields optional you can have 0 overhead in case of untyped records by using different .set implementation. And nearly 0 overhead for partially typed records where overhead can be just a property lookup in the js object to see if the factory for the field exist. And very small overhead of instanceof check for typed fields when field assigned is already typed. Of course you'll have an overhead of constructing fields when untyped data is passed, but that's not really an overhead as user would need to construct that anyhow.

Highly performant

Assuming current implementation is considered highly performant, I'd say this requirement is same as one above it, isn't it ?

Works for all Immutable collections, current, future, and 3rd party.

I'm not sure this a valid requirement. Each data structure comes with it's own pros and cons and if some data structures are not meant to be used with typed records I don't think there is anything wrong with that.

If you have thoughts on how to accomplish this, I'm happy to keep spit-balling.

I have being meaning to take a stab at prototyping extensible records after elm's implementation that is based of this paper http://research.microsoft.com/pubs/65409/scopedlabels.pdf I wonder it could provide relevant input in this context, I'll need to re-read it.

Without digging to much into library my guess would be that if you want to allow optional typing for all structures likely you'd need a parse function for the type construction that will be involved in leaf node creations for that structure. I'd also presume that you can have typed & untyped version of leaf node creations so that untyped versions won't get any performance overhead.

[Gozala]

Lately I have being thinking maybe it's best not to try changing a Record API but instead create a whole new data structure like TypedRecord and do the exploratory work there. That way other data types can gradually be added like TypedList etc... That way there will be neither breaking changes nor performance overhead for current data structures. Later Typed and untyped version could be merged if that would make more sense.

[Gozala]

@leebyron I want to bring up a problem I've run into while making / using typed list that I can't see a solution for. The use case example is something along these lines:

const Point = Record({ x: Number(0), y: Number(0) }, 'Point')
const PointList = List(Point, 'Points')

const graph = PointList(data)
const xs = graph.map(({x}) => x)

This code is doomed to have one of the following problems:

1. mapping a typed list returns untyped list. That sounds ok at first but then if you just wish to maybe move points in a graph by some factor you'll end up with an untyped list and you can also wind up with invalid items in the list. Also if you end up doing it like state.update('graph', graph => graph.map(moveBy(2, 5))) (assuming state is a record) you'll wind up creating one untyped list and then parsing it to a typed list.
2. mapping a typed list returns typed list. That would be great, but as we don't know what type of records resulting list should be of we can only go with a same type or any type. Later is basically a case described above while former is makes it impossible to map points to numbers in first place. Only other options I see is to provide optional type argument to map to say what type of list it should construct & assume it's a same / any type if omitted.
3. mapping a typed list returns a typed list where type of the resulting list is a union type of types that it winded up with. I find this most reasonable as in common case where mapping function is a -> b it will be able to do [a] -> [b] although in cases where mapping function returns arbitrary types it may lead to surprising result and suboptimal performance with when used within the records.

I've being thinking that it may in fact be better to not do the typed list or records as I originally was proposing. Maybe indeed it would be the best to only do a type parsing during construction and leave it up to user to push / unshift / set right types during updates. Although I still think it would be nice to provide some feedback if a wrong data structures are inserted during update.

[leebyron]

Yeah, I see what you mean here. I'm glad you dug into this and figured out the boundaries. I think I agree that this is pretty dooming for typed lists. However typed records still seems valuable, perhaps there's a way to leverage "list" being part of a record type. Typed records won't have the same issue of the mapping functions desiring to produce different shaped records, so I'm not sure the same pitfall exists there.

For example maybe very roughly like:

var MyRecord = Record({
  listOfFoo: values => List(values).map(Foo)
});

[Gozala]

However typed records still seems valuable, perhaps there's a way to leverage "list" being part of a record type.

That is how I got to typed lists actually as you do need to have this special lists that are part of the record but can be operated just as same if passed to you without owner record.

That being said I think I end up with pretty reasonable solution see next post.

[Gozala]

More updates on this front:

1. Maintaining a fork turned out to be a pain so I end up pulling related code out into separate project / repo which currently lives here: https://github.com/Gozala/typed-immutable
2. Current implementation avoids any changes to the stable version of immutable.js and does more or less what Record implementation in immutable.js does, meaning TypedList wraps an actual Immutable.List and most methods basically delegate to internal list and then wrap it with new TypedList. Not ideal but avoids forking library.
3. Earlier @leebyron mentioned that Record({ x: Record.Number, y: Record.Number }) was not ideal as users may confuse and do Record({x:Number, y:Number}) which I end up using to my advantage and basically both produce equivalent record types. As a matter of fact all JS built-in constructors can be used now and even Record({x:Number(0), y:Number(0)}) will do right thing and define record with x and y fields of number type with a default value 0.
4. I have added Maybe(Type) to allow nullable types.
5. I have added union types like Union(Username, Email).
6. Typed lists can be mapped to lists of different type and the way it works is that type inference is used during mapping to detect type of mapped items. If resulting item is of the same type then resulting list will be of the same class as an original if say you mapped list of points to a list of .x then result will be typed list of numbers. Basically type is open during mapping but then it's locked. Now obviously mapping function can return values of different types, in that case resulting list is of union type of all types involved. Although in some cases if mapping function just returned js object list will be of Any type which is unwrapped Immutable.List. I think in practice this matches map behavior in typed languages where resulting list is of type of return value although there type is inferred at compile type :)

I do intend to add few more data structures Tuple and Map. I also will be more than happy to merge this work back to immutable.js if there will be an interest in doing so.

[ghost]

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.

[leebyron]

Closing this aging issue - We're very often relying on Flow and Typescript for this kind of typing now, rather than runtime coercion. Though these ideas are still interesting for this and other future companion libraries.

[joffrey-bion]

Hi @leebyron, you seem to say the problem is solved when relying on Flow, but I'm using Flow and Immutable JS v4.0.0-rc.5 and I still have the same problem of not being able to inflate a Record with a Map inside it from a json I get from a server.
(Not able to inflate = it does not crash but I don't get a map with the get() method and size property)

Could you point me to any example where nested immutable records/maps/lists can be instantiated in one operation using Flow to describe the schema?

[leebyron]

There is no such method at this time

[iShawnWang]

I just found a solution like this for immutable v4 , https://stackoverflow.com/a/41234524

export class Geo extends Record({
    lat:"",
    lng:"",
}){}

export class Address extends Record({
    street:"",
    suite:"",
    city:"",
    zipCode:"",
    geo: new Geo(),
    phone:"",
    website:"",
}){
    constructor({geo=new Geo(), street="", suite="",city="",zipCode="",phone="",website=""}={}){
        super({street,suite,city,zipCode,phone,website,geo:new Geo(geo)})
    }
}

It works but writes annoying, any better solution or example ?

:D