New maintainer is needed

[Gozala]

I am afraid I do not have time to work on this project, so if someone want's to step up and move it forward review / land pull requests respond to issues etc.

[udnisap]

I can help on this. we are using this heavily in our code and have done few more improvements on this as well.

[davecoates]

We are using this as well so happy to help out

[stutrek]

Also happy to do my part, should we put together a github org for it?

[Gozala]

I can help on this. we are using this heavily in our code and have done few more improvements on this as well.

Thanks @udnisap! Do you mind describing what the improvements are ?

Thanks @davecoates and @stutrek for jumping onboard.

If you all guys could collaborate that would be fantastic. If you want to setup an org I can just migrate repo to it. Otherwise I could give you access rights.

If you could define what's the plan moving forward that would be great. That would also help us figure out if you all can collaborate.

Thanks

[stutrek]

@kirach and I are going to set up an org for it, I'll add you guys. Doing the hardest part now: figuring out a name

[stutrek]

https://github.com/typed-immutable

[davecoates]

Sounds good!

In terms of a plan moving forward there's a couple of pull requests I'd like to land (mainly Map support) but that's from me so would like someone else to review it. The library also has a hard dependency on specific version of immutable - I think this should be changed to a minimum version as a peer dep but I'll open a PR for that later.

We've been using this in production for a while and it's been good but there's a few issues, one of which is performance related to Union's so I'd like to look at that next.

Thanks for the library @Gozala, we have found it very useful. Are you wanting any involvement with this project going forward or looking to hand it over entirely? If the latter someone will need to be added as collaborator on the npm package to cut new releases.

[stutrek]

I think all PRs and issues come with the repo if gozala transfers ownership. You and udnisap have good PRs, I'll need to make the Date one work very soon.

[lukesneeringer]

I know I am late to the party, but I would be happy to help out too.

I have actually been using a fork in production, which includes two fixes:

• Support for maps (which @davecoates mentioned)
• Support for Date objects, which currently throw a TypeError.

Fixing Date objects is as simple as adding the following to typed.js:

Typed.Date = Typed("Date", value => {
  var d = new Date(value)
  if (isNaN(d.valueOf())) {
    return new TypeError(`"${value}" is not a valid date.`)
  }
  return d
})

[stutrek]

@lukesneeringer I added you to the org. Do you have a PR open for your additions?

[lukesneeringer]

@stutrek One of them is the Map PR by someone else (#19). I will open one for the other.

[lukesneeringer]

@stutrek #22

Which means you are off the hook for making Date work, as I have done it. :-)

[stutrek]

Thanks!

@Gozala let me know how you want to proceed. I made a group and invited you as an owner, thoughtbot has a great guide to contributing so I forked that to the group. If you want to keep the repo here that's fine, but if you transfer ownership all the issues, PRs, and webhooks will stay intact. https://help.github.com/articles/about-repository-transfers/

[udnisap]

Hi guys sorry i was bit busy these days.
I have sent couple of PR to the project
#18
#17

and somethings we did on our own was to have data cleaning methods on top of Record when intial object is passed to Record. For example in our use cases default values are not always undefined.

const Person = Record({
  name: String(),
  age:  Number(0)
})

but from our data scheme, we might sometimes get Person objects as
{ name: 'Tom', age: '5'} . it is a cleaning step of data before going to Records but we have a custom function that can be passed to Record which will do that cleaning/transforming step before pushing to Record.

const Person = Record({
  name: String(),
  age:  Number(0)
}, (rawData) => cleanedData)

We are also using few more types such as Fractions, moment and has support for that as well

Further we have few methods to work with Records.

1. Remove default values(including nested records) when converting to JS.
2. Utitlity methods to check a certain record/nested record has changed from its default values. (person.hasChangedFromDefault())

[stutrek]

@udnisap I'm very interested to see many of these changes! The Fractions part sounds amazing. I also like the idea of a .toJSONWithoutDefaults().

Does hasChangedFromDefault just do a person.equals(new Person())?

[nmn]

I'm willing to help as well. I was about to start my own project that was similar when I found this. It's everything I wanted it to be and more.

The first order of business for me would be to add flow types to the project.

[udnisap]

@stutrek yeah we are using something like person.equals(defaultPerson) where we have the default in memory.

We do have Fractions to number and number to fraction convention. for example

const Item = Record({
  name: String(),
  price:  Fraction(0)
});

and when using it (autocasting will result in a type error if the string is a not a valid number)

const itm = new Item({price: 20})
const itm2 = new Item({price: '20'})

itm.price //is a fraction
itm.toJS() //will convert to number

[Gozala]

Sorry for the delay folks, had really busy last week. Looks like you're all eager to collaborate, which is great. @stutrek would you mind adding people who want to help with maintenance to the typed-immutable organization ?

It also looks like I need admin rights for that organization to be able to complete transfer.

[stutrek]

Great! @Gozala I think you need to accept the invite to the group before I can give you admin rights. I can invite you to a team, but that doesn't make you an owner.

[lukesneeringer]

@udnisap

it is a cleaning step of data before going to Records but we have a custom function that can be passed to Record which will do that cleaning/transforming step before pushing to Record.

This makes a ton of sense to me, although it is worth noting that we would need to decide on what the expected signature for Record is. Right now, Record accepts a string with a type name (which is somewhat useful in places like console.log. If we implemented your change, probably we just push the string back one (so the signature becomes (structure, transformFunc, className)), and we would have to test the second argument to see if it is a string or a function (basically, if typeof transformFunc...).

Another thing that might be useful, either instead of or in addition to the changes you propose, is having the method for each type receive the key and full initial data as subsequent arguments. Right now we have:

Typed('CustomTypeName', value => {
  // Return the appropriate thing.
}

If we instead passed (value, key, allData), then the custom types could do that kind of transformation.
This might be a little more portable than putting this sort of thing on the Record classes themselves. (Honestly, though, I think it would be reasonable to do both of these things.)

I could make this change in a matter of a few minutes; I will do so once the repo is transferred and submit a PR. (Unless there is a preference that I do it now?)

[stutrek]

@Gozala thanks for transferring it! I'll look at everything tomorrow, we should merge many of these.

I have to be honest, I'm not a big fan of the data transformer being part of the API. I don't like the idea of us adding to the API. Immutable.js may change in a way that conflicts with any changes we make, then we have the choice of either making a release that breaks to conform with the Immutable API or to add a confusing hurdle to others that are trying to adopt this library.

I think this functionality belongs either as a utility function or as a part of a larger data loader of which typed-immutable is only one part.

[lukesneeringer]

Putting it as part of the type reader would solve that problem, and be pretty immune to changes that Immutable.js might make.

[lukesneeringer]

@stutrek Another thing we need to do is make sure the test suite is complete and set up CI and such. I can do that soon.

[stutrek]

Unless anyone has objections, I'm going to start merging PRs that have been open for at least a week and there seems to be a 👍 consensus on.

@Gozala how do you want to handle npm publishing? You can add me, my handle is sakabako.

[Gozala]

@Gozala how do you want to handle npm publishing? You can add me, my handle is sakabako.

Done!

[Gozala]

I have to be honest, I'm not a big fan of the data transformer being part of the API. I don't like the idea of us adding to the API. Immutable.js may change in a way that conflicts with any changes we make, then we have the choice of either making a release that breaks to conform with the Immutable API or to add a confusing hurdle to others that are trying to adopt this library.

I am not entirely sure what you mean by data transform here. If it's parsing arbitrary structures as expected types. I think I agree I have to the same conclusion but for different reasons see #2 for details.

[Gozala]

I think we can close this issue now! Thanks everyone for stepping up and keep this project going!

[stutrek]

@Gozala I think I was referring to something that there's not a PR for... Some functionality @udnisap implemented that works well for his case.

[Gozala]

@Gozala I think I was referring to something that there's not a PR for... Some functionality @udnisap implemented that works well for his case.

I linked to the issue not the PR though.