[WIP]: Using builder pattern for increased readability in the dsl.

[johnsusi]

I'll put this up here for discussion. #22 has some good background for the design of the dsl.

What I propose is the ability to build nodes using a builder.

Motivating example:

  def render

    form do |form|

      form.action = ''
      form.method = 'post'

      form << fieldset do |fieldset|

        fieldset << legend('Title') << input do |input|
          input.name = 'radio'
          input.type = :radio
        end

        fieldset << label do |label|
          label.htmlFor = 'radio'
          label.style[:color] = 'red'
          label << 'Click me'
        end

      end

      form << button do |button|
        button.type = :submit
        button.class_list << 'btn' << 'btn-default'
        button.onclick do |event|
          event.prevent
          puts 'submit'
        end
        button << "Submit me!"
      end

    end

  end

The builder has domain knowledge about what html attributes and events are available so typo's can be caught without inspecting the dom. If you miss a name it's easy to extend in client code.

This PR adds the following to the DSL.

1. Construct your node using a block that receives a builder objekt.

  div do |builder|
    builder.class_name = 'bold'
  end

1. Construct content using << instead of arrays

  div do |builder|
    builder << h1('Hello')
  end

1. An optional syntax for events (this is just a side effect, not intentional)

  button do |builder|
    button.onclick do |event|
      puts 'clicked'
    end
  end

Of course, button.onclick = -> { |event| } works as well

Initially I considered using the return value from the block as 'content' but that has to many edge cases. Using << on the builder is much cleaner imho.

Also there is no magic going on here. Ive looked at a few different types of dsl and some use method_missing to hide the builder object but this is very hard to get right for every edge case.

This syntax would be opt-in so no changes are needed to old code.

The code attached is functional but needs more work.

So, suggestsions, yays, nays etc

[jgaskins]

This is interesting. For those playing along at home, this wouldn't replace the current syntax, but would instead add to it.

I think my favorite part is the event-handling aspect:

button 'Click me' do |button|
  button.onclick { |event| puts 'clicked' }
end

Because this isn't great:

button({
  onclick: ->(event) { puts 'clicked' },
}, 'Click me')

This gets especially bad when you're building a video player, for example. Depending on how powerful you want your video element to be, you may need to use over a dozen event handlers:

video({
  ref: @container,
  onclick: method(:toggle),
  oncanplay: method(:on_can_play),
  onpause: method(:on_pause),
  onplay: method(:on_play),
  ontimeupdate: method(:on_time_update),
  onloadstart: method(:on_load_start),
  onloadeddata: method(:on_loaded_data),
  onloadedmetadata: method(:on_loaded_metadata),
  onload: method(:on_load),
  onseeking: method(:on_seeking),
  onseeked: method(:on_seeked),
  onwaiting: method(:on_waiting),
  onstalled: method(:on_stalled),
  onvolumechange: method(:on_volume_change),
}, [
  source(src: webm_source),
  source(src: mp4_source),
])

Realistically, most of those would simply be re-rendering the app because a video element manages its own state, but we'd still have to tell it to do that for every event. With this PR, this becomes a lot cleaner:

video({ ref: @container }, [source(src: webm), source(src: mp4)]) do |video|
  video.onclick { toggle }
  video.oncanplay { on_can_play }
  video.onplay { on_play }
  video.onpause { on_pause }
  # ...
end

I think this goes along with the spirit of what blocks are for (passing functionality rather than values) much more than the idea of putting child elements into them.

Food for thought: I don't know what the performance implications are. I'm usually hesitant to add much functionality to the element DSL methods primarily because they can get called thousands of times in a single render — the first render is a bit more sensitive to this because the methods have not been optimized until partway through it. I'm not sure what the performance difference is between block.call(builder) if block and yield builder if block_given?, but that'd be an interesting experiment.

[jgaskins]

More food for thought: Block handling has a bug in the released versions of Opal (I think there's a fix but it's not backported to a release yet) in examples like what we might be invoking here.

For example, instantiating a GrandCentral::Store and its initial state in the same operation triggers this bug:

store = GrandCentral::Store.new(AppState.new(attrs)) do |state, action|
  # ...
end

In the above example, the block gets assigned to the call to AppState.new instead of Store.new. This means that, currently, it has to be two separate operations. I don't know if that bug affects this PR, but I think it will if we do something like this:

div([div('something')]) do |dubious_div|
  # Which div does this get run as?
end

It'll work fine if the inner element isn't the same as the outer element (since it will use a different method), but if both calls go to div like this, it may cause issues in the current version of Opal. If it is affected, we'll need to wait to merge this in until Opal 0.11 is released (or the fix gets backported to 0.10) and update our opal dependency accordingly.

[johnsusi]

Very good feedback. I did not know about the block bug.

Agree on the event-handling. And it was completely unintentional ;-)

I haven't looked at the generated code yet but I'm sure there is room for improvement.

We don't actually need a ruby hash to back the builder, instead we could just set the values directly on the js-object that will get passed to virtual-dom. Perhaps bypass the sanitize-part as well?

What's your take on using predefined attributes vs using method_missing?

Perhaps provide an escape hatch like builder['non-standard-attribute'] = ....

Are there anything special that needs to be done for server-side rendering? Pure-ruby version on the server-side path and an js-optimized in the opal path?