View layer overhaul

[igorbernstein]

While working on #2679, I started thinking about the view system as whole & wanted to start a discussion about a possible overhaul. This ticket describes a lot of ideas that can & should be split up into separate tasks, but I wanted to discuss these changes in a broader context of ActiveAdmin's view layer road map and whatever changes that get a 👍 can be spun off into separate issues.

It seems to me that a lot of the complexity in FormBuilder stems from the fact that it mixes 2 unreleated concerns: adding dynamic fieldset functionality to formtastic and trying to support Arbre's dsl that magicly concatenates html behind the scenes. Which needlessly couples the ActiveAdmin FormBuilder to Arbre, complicates if not prevents the FormBuilder's use in plain erb templates and creates weird edge cases like https://github.com/gregbell/active_admin/blob/master/spec/unit/form_builder_spec.rb#L440. It also forces context sensitive behavior into the FormBuilder's implementation...for example if input is called in an inputs block then we use the form buffer otherwise just return the html. In my experience the delegator/decorator pattern would be a better fit to support the buffering behavior: expose the decorator in the dsl to buffer the returned html, but leave the FormBuilder's internal behavior unmodified. Looking at arbre's implementation for standard rails forms, it seems like thats the intended usage: https://github.com/gregbell/arbre/blob/master/lib/arbre/rails/forms.rb#L33. I think it would be cleaner to extend FormBuilderProxy for the Formtastic & ActiveAdmin's FormBuilder's apis. This would allow forms just to use Arbre's buffers and address #2542

Arbre is cool, but seems to accomplish way more than is needed in this project. Arbre's goal seems to be construct DOM in memory and allow the developer to manipulate it in an object oriented manner. However, from what I've seen in the project the only part that's really needed of Arbre is the html building DSL that allows a developer to quickly define the html structure inline with the rest of admin definition. The unused in-memory DOM representation more than doubles the memory usage required to service a request and complicates the implementation to having a seperate buffer per element. I think a lot can be gained my dropping the in memory DOM representation: a single ActionView buffer can be passed from parent to child, lowering memory usage and allow us to use streaming responses.

While thinking about the Arbre changes, I was trying to figure out a good upgrade path for people who might've depended on the old behavior. And I thought that there is no technical reason why we can't support multiple template engines in the view blocks:

form(engine: :erector) do |f|
  h1.heading! "First Name"
  ....
end

form(engine: :erb) do |f|
<<-EOS
  <%= f.inputs :first_name, :last_name %>
EOS
end

I also ran across erector, which seems to very similar to Arbre but seems an older projector and supports streaming: http://erector.rubyforge.org/faq.html#whyuseerector. Any idea why this project was ruled out 2 years ago?

So to summarize:
Do you:
a. agree with:
b. be interested accepting PRs for:

the following changes:

1. decoupling of form buffers from FormBuilder
2. using a single buffer streaming architecture for the view api
3. switching out Arbre to something seemingly more mature like erector
4. configureable view engine

[seanlinsley]

The form builder is currently incompatible with Arbre, hence #2542. The complexity instead stems from this syntax:

form do |f|
  f.inputs do
    f.input :name
  end
end

Whereas normally each f.input call would be wrapped in Haml or ERB that stored its return value, the AA form builder abstracts that so you don't have to worry about it and can just write Ruby.

If you look at the commit history, the AA form builder predates the creation of Arbre by quite a bit. The problems we're having is that they never went to the trouble of integrating Arbre into the form builder.

I'm open to consider anything that you're willing to write. But on the subject of supporting different template engines, I think that's overkill because at that point you might was well use a partial:

form partial: 'form'

[igorbernstein]

1. decoupling of form buffers from FormBuilder
I was thinking that the decorator would implement the inputs method and pass a inputs decorator...something along the lines of the following pseudo code:

def inputs(*args)
  html = if block_given?
    proxy_call_to_form(*args) do |form|
      yield InputsProxy(form, buffer: self.buffer)
    end
  else
    buffer << proxy_call_to_form(*args)
  end
end

class InputsProxy
  def initialize(form, opts)
     @buffer = opts.fetch :buffer
  end
  def input(*args)
  @buffer << proxy_call_to_form(*args)
  end
end

2. using a single buffer streaming architecture for the view api
3. switching out Arbre to something seemingly more mature like erector
These are fairly large changes that I would be willing to implement but would like to hear your thoughts on where they would they would fit in this projects roadmap. Would you actually be ok w. with replacing arbre w. erector before 1.0? Given your understanding of your user base, would they be ok with this?

4. multiple engines
If you are declining multiple engines, does that imply you are ok with breaking backwards compatibility? or that core arbre changes are off the table?

[seanlinsley]

Would you actually be ok w. with replacing arbre w. erector before 1.0? Given your understanding of your user base, would they be ok with this?

My clients don't care because they don't write code, but I rather like Arbre's syntax so it would take some convincing. Since performance is such a big topic, I'd love to see benchmarks.

If you are declining multiple engines, does that imply you are ok with breaking backwards compatibility? or that core arbre changes are off the table?

I declined the multiple engines idea because IMO the inline form syntax should only support one syntax for simplicity's sake. But an AA form should definitely work with those engines. Right now it really bugs me that it's so hard to use the AA form builder with ERB or Arbre partials because of all the buffering problems.

I want the best decision to be made. If that means making breaking changes, fine. If there are improvements to be made to how Arbre works, then please make them.

[seanlinsley]

I haven't studied the Rails form builder code too closely, but there are probably some things there we could emulate to make our code simpler.

[pietschy]

Thanks for the pointer to Erector @igorbernstein. It does look pretty nice, I'm going to trial it in my next project (although not in the AA bits obviously). Having written a few basic Arbre components I did find them pretty confusing at times and quite hard to test (e.g. setting up the testing environment, having them break other tests etc).

@seanlinsley, They seem to claim it's ~ 2x faster than ERB templates, but don't know when this was measured. http://erector.github.io/erector/faq.html#howfastiserectorcomparedtoerbhamletc

Anyway, food for thought.

[pietschy]

Having read a bit Erector seems to have not kept up. See https://github.com/ageweke/fortitude/blob/master/README-erector.md

[varyonic]

I think it is time to wrap up this great discussion.

1. For better or worse FormBuilder has been integrated with Arbre using proxies, and form_buffers has been removed. There are still issues because of the way Formtastic buffers its own output, and has_many has never been proxied. There have been discussions of simple_form and cocoon but no follow up. I think this item has been addressed as far as can be expected.
2. I think streaming responses for Arbre is a great idea and I added an issue referring here. I started working on something a while ago but it has not been a priority.
3. Erector (now Fortitude?) is interesting, though a major change, and introducing new dependencies is always a concern. I think a separate issue should be opened if anyone is serious about going further.
4. I agree with Sean on multiple engines being overkill beyond partials and not a priority.