simerplaha/html-to-scala-converter

No need to separate attrs from children

japgolly opened this issue · 4 comments

It's a little more efficient to put all modifiers in the same parameter list.

So instead of

<.ul(^.className:="dropdown-menu")(
<.li("List item 1"))

this results is slightly better performance:

<.ul(^.className:="dropdown-menu",
<.li("List item 1"))

No worries. I will change this. Is the performance increase during runtime or compile time ? Because if it's just compile time then I will add a checkbox to select a preferred output. If it affects runtime performance then I will have to go back and make the change to a scalajs-react project that is currently running in production.

The reason I thought it wouldn't matter because I thought Scalatags code gets compiled down to HTML anyway. But I am probably wrong and there is some runtime Vdom magic that I don't know about.

It's during runtime but it's minuscule, negligible really, so I wouldn't worry about going back to change all your stuff. It's just that for new stuff, you may as well do it the more performant way.
The attr/children separation is hangover from HTML and while at first I used to model my scala the same way, IMO it's must more readable to do it like this.

Also while we're talking, FYI you can also just use <.br instead of <.br(). 😄

Done and Done :)

I've added the option to Add attributes on new line which somewhat follows the formatting style in the code you linked me to.

I think the next problem to work on would be to camel case the Event functions (onClick, onDrag etc). But if you think of other issues that can be worked on please let me know.

Nice work! And will do. Cheers mate!