ES6 Classes and Higher Order Components

[leemeichin]

Currently WIP

• Fixes Deprecation of React.createClass #437, Deprecation of React.PropTypes #438 and Rewrite readme mixin example to use Higher Order Component #395 by updating React internally to 15.5, and replacing createClass with ES6 classes.

• Converts the Mixin to a HOC, because of the reasoning in Rewrite readme mixin example to use Higher Order Component #395 and because it won't work without createClass anyway. This means that Mixin needs a new name.

• Passing tests

Optionally (depending on feedback from other collaborators because it increases the PR size):

• Transform main source to consistent ES6, since it's 50/50 right now (e.g. require for importing, var over const, etc.)

[aesopwolf]

First glance this looks fine. Ideally we will rewrite all the docs (and source) to use ES6 classes and High Order Components though. #395

I'll wait a week (maybe 2?) before merging this. Hopefully myself, or someone else can create a PR to fully deprecate createClass/createReactClass

[leemeichin]

If that's the plan it should be no problem for me to kill 2 birds with one stone and do that here :) the question is what to do with the mixin related code though: should that be removed along with createClass, so only the higher order component remains?

I'm happy to change this PR to do that (which fixes the warnings anyway).

[leemeichin]

Ok, here's the beginning of it, all but 10 or 11 tests are passing. Once they're fixed then we can see if anything in there should be cleaned up. :)

[track0x1]

Oh this would be fantastic!

[leemeichin]

It's been taking a teensy bit longer than I planned, sorry :(

[track0x1]

@leemachin Can I help expedite this somehow?

[leemeichin]

@track0x1 there are just the last ~10 failing tests, some of which may be due to the testing utilities (that all relied on mixin behaviour previously), and others because I started moving the handlers into props.

e.g. the two main failures are this:

Warning: React.createElement: type is invalid -- expected a string (for built-in components) or a class/function (for composite components) but got: object.

That's likely to be a render method in one of the test components. The stack trace reveals nothing, though, so it's difficult to pinpoint.

TypeError: Cannot read property 'setFormPristine' of undefined
    at submit (/home/travis/build/christianalfoni/formsy-react/lib/main.js:237:7)

That one's pretty easy to debug. :)

[leemeichin]

Okay, apart from the documentation and examples (simple find and replace changes I guess) the last question is how to deal with all of the handlers that were once mixed in. Since they're all props now they have to be extracted before the main elements are rendered, otherwise React will complain.

The Mixin and HOC have to be renamed too, because they don't make sense as they are (and it's kind of confusing). The mixin looks more like the base component (the Formsy.Form component) because everything appears to rely on it. Some opinions on tidying that up would be welcome.

Those are the final two problems.

[th3fallen]

any updates here? since atm this lib will not work with fiber due to the mixin i suspect

[samuv]

@leemachin @aesopwolf
Do you guys need some help? I would love to help you out to release this version.
Maybe we can create a dedicated branch and not do all in one pull request.
What do you think?

[leemeichin]

@samuv that would help, I think. It feels more and more like a rewrite as I try to make changes; it's too much to do all at once.

[samuv]

@leemachin Yep, it feels like a rewrite to me, if we move everything on the specific branch we can focus on each problem with dedicated pull request and isolate all the things that we need.

How can we move forward with this approach?

[maccuaa]

@leemachin @samuv - I've created a PR to @leemachin's branch that fixes the remaining tests.

[samuv]

Thank you @st-andrew, the only thing I don't understand if there is an active maintainer in this thread that can help us to create a doable workflow for this release.

It would nice to have the possibility to create a project here in this repo for understanding the effort and creating some tasks for what we need.
In the case, that something has breaking changes on the API we need to document the changes and help in the transition from one version to the latest.

FYI @christianalfoni @aesopwolf

[maccuaa]

Yea because of the size of these changes, a beta release would be good to catch any breaking behavior.

I hope there's still a maintainer kicking around to review these changes, otherwise might have to fork and republish under a different name to carry on (but i'd rather contribute to this repo).

[th3fallen]

even if this is forked it'd be awesome because currently this lib is preventing me from updating to 15.5.4 :( these updates should allow for me to update

[brandondurham]

I’d be more than happy to point to a forked version, even if it was temporary.

[th3fallen]

@brandondurham please/thanks

[bigblind]

@brandondurham Have you had a chance to fork this? I'm reviewing other form libraries, but I haven't found any that are as easy to use as Formsy.

[maccuaa]

@bigblind - I'm almost done my fork. Hoping to have it published this week.

[maccuaa]

I've published a new fork to npm called formsy-react-2. Make sure you read the README, there are definitely areas for improvement and I will gladly accept PRs.

I've also published a new package based on formsy-material-ui called formsy-mui that makes use of formsy-react-2 if you're looking for examples.

Eventually I'll update the documentation and add examples, for now I just wanted to get these released.