search

AppliedMathematicsANU / plexus-form

A dynamic form component for react using JSON-Schema.
MIT License
134 stars 31 forks source link

Components shouldn't try to use this.props.key #34

Closed martijnvermaat closed 9 years ago

martijnvermaat commented 9 years ago

From React 0.12 (I think), key and ref are now reserved prop names since passing them has a separate meaning. Hence, a component shouldn't try to read this.props.key.

This is done in, for example, InputField and InputWrapper where it indeed yields undefined.

I think the original idea behind using key here is still useful (e.g., to properly set id and htmlFor), but a different property name should be used.

odf commented 9 years ago

Ah yes, looks like it is used all over the place. Thanks for catching this one!

martijnvermaat commented 9 years ago

A simple renaming will probably do, but there might be places where you also still want to have a key. I didn't look closely at that, but the key for <option> in SelectionField should stay for example.

odf commented 9 years ago

I had a quick look through the code and it seems that we'll want to duplicate the key prop that is constructed in fields/make into one that can be read within a render method. I think label would fit nicely. Then every use of this.props.key will have to be replaced by this.props.label. We still want to keep setting the key prop to disambiguate fields, so I won't be renaming key when it's in the receiving position.

I'll get to this right away and see if anything breaks.

odf commented 9 years ago

Fixed in d81ee5d.