Add property support of "rendered" and React classSet.

[cain-wang]

Add property "rendered". If set to false, React will skip the rendering of the element. Add support for React classSet. If the className property value is an object, wrap the property value with React.addons.classSet().

[mlmorg]

Can we add some documentation on these added features? Otherwise, :+1: lgtm

[cain-wang]

Sure, will do :)

Cain

On Mon, Dec 22, 2014 at 5:18 PM, Matt Morgan notifications@github.com wrote:

Can we add some documentation on these added features? Otherwise, [image: :+1:] lgtm

— Reply to this email directly or view it on GitHub https://github.com/uber/r-dom/pull/2#issuecomment-67912083.

[cain-wang]

Hi Matt,

Just updated the README file, please take a look when you got a chance:

https://github.com/cain-uber/r-dom/blob/master/README.md

Thanks,

Cain

[cain-wang]

Hi Matt,

I created this simple js perf test cases to try out different ways of calling React APIs. http://jsperf.com/jw-react-api

Surprisingly, our current pattern passing all children as an array (3rd test case) is bit slower than apply pattern (4th test case). I'm still improving the test case, just wanna let you know some early results.

Thanks,

Cain

On Mon, Dec 22, 2014 at 5:37 PM, Jialiang (Cain) Wang cain@uber.com wrote:

Hi Matt,

Just updated the README file, please take a look when you got a chance:

https://github.com/cain-uber/r-dom/blob/master/README.md

Thanks,

Cain

[mlmorg]

The array pattern was fastest for me lol...

[mlmorg]

:+1: looks good. Squash before merging please to keep this to one or two commits!

[cain-wang]

Definitely. I won't have that many minor commits polluting the history :)

On Thu, Dec 25, 2014 at 2:19 PM, Matt Morgan notifications@github.com wrote:

[image: :+1:] looks good. Squash before merging please to keep this to one or two commits!

— Reply to this email directly or view it on GitHub https://github.com/uber/r-dom/pull/2#issuecomment-68114262.

[cain-wang]

The pull request is just merged in.

Thanks.

Cain

On Thu, Dec 25, 2014 at 2:20 PM, Jialiang (Cain) Wang cain@uber.com wrote:

Definitely. I won't have that many minor commits polluting the history :)

On Thu, Dec 25, 2014 at 2:19 PM, Matt Morgan notifications@github.com wrote:

[image: :+1:] looks good. Squash before merging please to keep this to one or two commits!

— Reply to this email directly or view it on GitHub https://github.com/uber/r-dom/pull/2#issuecomment-68114262.

[cain-wang]

Hi Matt,

I find an issue with how we handle the React key on the each element. Since almost all elements have the same key 'customComponent', React will only render the first child if they are off the same type (removing the key part in our lib will fix that).

I read through various articles, and it seems key is very important for React to identify the elements and the keys should be bound to the data model. So we need to double think of assigning general keys to all elements .

In this case, we might have to avoid to call createElement() with children as an array, and use createElement.apply() instead.

My updated benchmark shows using Varargs with apply() doesn't cause that much performance overhead (and faster than React DOM API).

http://jsperf.com/jw-react-api/3

Thanks,

Cain

[mlmorg]

Nice catch. It's annoying that React warns when it isn't set since passing an array would be a performance gain. Maybe I will open up a ticket on React, but yes, in the meantime, we can remove the whole key code and use .apply() like react-hyperscript does.