Support camelCased properties
dantman opened this issue · 5 comments
Polymer defines properties as camel case and converts between that and dash separated notation in attributes. React has a similar pattern, using the camel cased property forms inside of JSX code instead of the attribute form.
If possible it would feel a little more like react if we could write:
<iron-icon icon='settings' itemIcon />
<paper-icon-button icon='menu' paperDrawerToggle />
- etc...
I don't think this is necessary. You already use dash-case for the element names, so why not for the attributes too?
Also, it would have a perf cost to implement this.
If it really matters to you, you can build react wrappers for all your polymer elements, with PropType validation and camelCase properties.
- Polymer defines element names in dash case, there is no other form.
- React defines elements as the lowercase forms (incl. dash case).
- Only React components which are explicitly not elements get special cased element names (ie: If it's UpperCamelCase it's a JS variable referring to a React component instead of an element name).
- React defines props even in JSX syntax not as lowercase/dash-separated DOM attributes but as camelCased names based on camelCased DOM properties.
- This definition applies equally to React components and native html elements:
<Foo barBaz={4} />
,<div tabIndex='0' />
, and<input readOnly />
are correct;bar-baz
,tabindex
, andreadonly
are not. - Polymer defines attributes in lower dash-case (attribute form being used in html and setAttribute) and properties in camelCase like DOM properties are; In fact api documentation of all properties/attributes is done in camelCase, only html examples use the dash-case.
React/JSX is written with camelCased DOM properties, not attributes; Polymer's properties are camelCased; and all documentation documents Polymer props/attrs primarily in their camelCased property forms. So I think it makes sense to use camelCase when using Polymer elements through React.
Where would the perf. cost come from?
ReactPolymer already requires explicit registration of custom properties/attributes. ReactPolymer seems to be using ReactInjection.DOMProperty injection to handle custom properties. And examining this interface it seems to already explicitly support defining properties along with their camelCased property name, lower/dash-case attribute name, and other behaviours the property has.
With perf cost I mean converting from camelCase to dash-case. It could however be cached.
The explicit registration is only for custom properties on standard html elements (<div drawer />
), which have an attribute white-list. Maybe the readme should be more clear about that.
I see what you mean with properties vs attributes, but I built react-polymer for compatibility issues, not stylistic ones. It is already very complicated, I don't need more complexity.
That said, I might merge a PR from you if it is a simple change and has no downsides.
String conversion wouldn't be an issue, I was thinking about something like an optional reactPolymer.registerProperties
that would inject a series of property definitions instead of whitelisting custom attributes. The prop -> attr name conversion would be cheap and done only at registration time.
ReactInjection.DOMProperty.injectDOMPropertyConfig({
Properties: {
itemIcon: ReactInjection.DOMProperty.HAS_BOOLEAN_VALUE
},
DOMAttributeNames: {
itemIcon: 'item-icon',
}
});
Sadly after doing a deep dive into the React internals it doesn't look like this idea is going to work out.
The property definition API works well, unfortunately when ReactDOMComponent is setting properties right before it gets to the block that would use setValueForProperty
to set a property in a way that includes the injection api, it does an isCustomComponent
that short circuits directly to a setValueForAttribute
call that sets an attribute with no injection instead of a property.
This means that all parts of the injection api are completely ignored on custom components. So, that itemIcon
would work on a <div>
but not an <iron-icon>
.
Maybe in the future when they make props context aware (I saw something about them wanting to be able to differentiate between html and svg for some props) the injection api might be contextual enough for us to ask for something that'll let us define properties for a custom element.
I think you could create a separate module that monkey-patches DOMPropertyOperations.setValueForAttribute
to do what you want.