Custom Immutable Classes, Round 2

[tgriesser]

In case anyone wants to kick the tires on this a bit, here's another pass at custom classes, actually with really great performance... this is the syntax:

// Note this is Immutable.Classes.Map not Immutable.Map
// This class is not meant to be called directly, but via the factory created below:
class UserClass extends Immutable.Classes.Map {
  fullName() {
    return this.get('first_name') + ' ' + this.get('last_name')
  }
}

// Make the Immutable factory
var User = Immutable.createFactory(UserClass)

// For better debugging, you can also create it with a named function:
// var User = Immutable.createFactory(function Immutable_User() {
//    UserClass.apply(this, arguments)
// }, UserClass)

Now you've got a nice new Immutable "User" type:

var u = User({first_name: 'Joe', last_name: 'User'})
var u2 = u.set('first_name', 'Jane')
u.fullName() // Joe User
u2.fullName() // Jane User
var u3 = u.set('first_name', 'Joe')
u3 === u // true

With the following guarantees:

u instanceof User // true
Immutable.Map.isMap(u) // true
u instanceof Immutable.Classes.Map // true

// false... though Immutable.Map() instanceof Immutable.Map is still true
u instanceof Immutable.Map 

Still a WIP, any feedback is appreciated!

@leebyron sent more details via email

/cc @janmarek, @vkurchatkin, @dandelany

[dandelany]

Cool @tgriesser thanks for the update! I will play with this branch when I have time.

[kuraga]

@tgriesser I has started to use this patch and it's awesome!
How do I change constructor here? I want:

var u = User({first_name: 'Joe', last_name: 'Green'})
// constructor changes arguments, and
// {first_name: 'cool Joe', last_name: 'Mr. Green'}
// will being received by Map

Can I do it? Thanks!

[tgriesser]

I suppose you could provide a custom __factory implementation to handle that, but I don't see that as necessarily a good idea.

[dashed]

This looks neat. Hopefully this lands.

[chicoxyzzy]

I like this too 👍

[tgriesser]

Updated this against 4.0, even though apparently GH doesn't let you change the target branch in a pull request.

As fun as it's been to use in my own projects, I'm a bit wary this will open pandora's box a bit, people might start trying to do things with this that they really shouldn't be doing in general.

Wouldn't mind just keeping this as an upstream fork for awhile to see if anyone has other opinions or ideas.

[leebyron]

Yeah - the more I'm thinking about it, the more I'm concerned about the pandora's box that you brought up just now (and before) @tgriesser.

I agree with your conclusion that we should keep floating this as a fork to investigate the compelling use cases.

In previous conversations you compelled me to include a metadata API which I think handles a lot of the cases that subclassing was desired for - and that's still on the docket.

[tgriesser]

In previous conversations you compelled me to include a metadata API which I think handles a lot of the cases that subclassing was desired for - and that's still on the docket.

Awesome to hear

[vtambourine]

In my project on React.js I want to wrap certain data in type classes to perform strict datatype checking using propTypes. For example, to check if property cargo if array of Cargo I want to use:

propTypes: {
    cargos: PropTypes.arrayOf(PropTypes.instanceOf(Cargo))
}

But since all props came from store they must be considered as immutables. So, to typify raw data I intend to use this fork of ImmutableJS:

class Cargo extends Immutable.Classes.Map { }

Can you, please, comment on this pattern. Is it appropriate use of proposed feature or there exist simpler solution?

[leebyron]

@vtambourine I'm a little confused about your prop-types example. Are you saying that you have arrays of Immutable Maps? Or do you have Immutable Lists of Immutable Maps? Or Records?

Also note that React propTypes are just functions which do or do not return an Error, you can always write your own propType function that will do the custom validation you want.

[vtambourine]

Yes, you're right about propType custom validation functions. I can use it in my project, but how to handle situations with passing certain props deep into children components? For example, consider following structure:

class CargoList extends React.Component {
    render() {
        return <div>{this.props.cargos.map(cargo => <CargoItem cargo={cargo} />}</div>
    }
}

class CargoItem extends React.Component {
    render() {
        return <div>{this.props.cargo.getName()}</div>
    }
}

Note, that in CargoList component I have property holding array of Cargo-type objects, and in CargoItem component I expect the single Cargo-type property. As far as I understand React approach, first array should be treated as immutable array of immutable object. How to handle props validation on each components?

First method is to have a custom validation function. I can have some kind of AppPropTypes.cargo validator and use it in a way:

CargoList.propTypes = { cargos: React.PropTypes.arrayOf(AppPropTypes.cargo) }
CargoItem.propTypes = { cargo: AppPropTypes.cargo }

But this may be quite verbose. I need to maintain type structure both in creator and validator.

Much plainer might be to validate props by their types using React.PropTypes.instanceOf:

CargoList.propTypes = { cargos: React.PropTypes.arrayOf(React.PropTypes.instanceOf(Cargo)) }
CargoItem.propTypes = { cargo: React.PropTypes.instanceOf(Cargo) }

// somewhere in store
_cargos = update(_cargos, { $push: new Cargo(data) })

Where Cargo extends Immutable Record.

Am I correct in my conclusions or I'm going totally misconcept?

[leebyron]

You're correct that if you're using Record, you can use PropTypes.instanceOf(MyRecord) without any issue.

As far as I understand React approach, first array should be treated as immutable array of immutable object. How to handle props validation on each components?

There's no such thing as an immutable array in JavaScript, but you could certainly use an Immutable.js List collection here if you wish. You could use PropTypes.instanceOf(List) as your propTypes for this property, there's no need to test for the contents of the list since your CargoItem propTypes will already be doing this and there's no need to do so twice.