ruby-hyperloop / hyper-react

The project has moved to Hyperstack!!
https://hyperstack.org/
MIT License
285 stars 14 forks source link

inconsistent automatic snake case to camel case conversion #108

Open sollycatprint opened 8 years ago

sollycatprint commented 8 years ago

From @catmando on December 22, 2015 13:7

We need to clean this up for example the html attribute default-value should be expressible as :default_value or "default-value". Sometimes things are getting converted, sometimes not.

Also conversions should only apply when passing params to builtin in tags, and (possibly) to native .js components. i.e. when passing a param to a ruby component no conversion should be made.

Related to #99

Copied from original issue: zetachang/react.rb#108

sollycatprint commented 8 years ago

From @catmando on December 23, 2015 6:43

This is worse than I thought. Take param :read_only for example. It doesn't work because read_only gets automatically translated to readOnly. This is done because react.js maps readOnly back to read-only.

The specific list is as follows:

acceptCharset accessKey allowFullScreen allowTransparency autoComplete
 autoPlay cellPadding cellSpacing charSet classID className colSpan 
contentEditable contextMenu  crossOrigin  dateTime  encType  formAction 
formEncType formMethod formNoValidate formTarget frameBorder hrefLang htmlFor 
httpEquiv marginHeight marginWidth maxLength mediaGroup noValidate radioGroup 
readOnly rowSpan spellCheck  srcDoc srcSet tabIndex useMap dangerouslySetInnerHTML

Solutions: (in cases we are only referring to names in the above list)

  1. don't allow params to be declared with the snake case version of these names
  2. only do the translation from snake to camel only if the target is a react.js component
  3. translate camel BACK to snake when picking up params. don't allow react.rb to declare params from the above list (since they will never get passed)
  4. don't do this mapping thing (i.e. if you want "readOnly" type "readOnly")
  5. when passing the params the snake_case becomes camelCased, (like today) but the camelCase becomes snake_case. When reading params we always reverse. thus everything should work out. To keep things consistent we ALWAYS do this for all params regardless if they are in the list.

3 and 1 are similar but 3 seems much better (i.e. you are only disallowing a the above camelCased names as sort of "reserved words." A variation on (3) would be to only disallow both the snake and camel versions to be declared in the same component. I.e. its fine to have allowFullScreen as long as you don't also have allow_full_screen.

A positive to #1 is that it has the least magic. The above list AND their snake_case counter parts all become effectively reserved.

4- is untenable

2- is probably doable, but has a lot of magic.

5- seems like too much magic, and also very hard to deprecate, as people may have code looking directly at props...

catmando commented 8 years ago

@zetachang - have a think about this please, and let me know what you think we can do... it would be nice to have this work very consistently everywhere...