Erector Design Issue: Widget Class Hierarchy

21 views
Skip to first unread message

Alex Chaffee

unread,
Aug 3, 2011, 3:42:06 PM8/3/11
to erector, Viktor TrĂ³n
The Widget class and module hierarchy is confusing. There are at least
three separate reasons why. (Not all the reasons are *good* reasons,
but here they are.)

Reason 1:

We want to put different widget functionality into different modules
so widget.rb doesn't become thousands of lines long. But if we just
mixed in all the modules into Widget, then calling "super" from a
widget subclass wouldn't always get the method you expect. So we (and
by "we" I mean John F, since he did this nice refactoring a year or so
ago) instead mix them into a *superclass* called AbstractWidget, then
Widget < AbstractWidget and the super semantics are clear again.

There's a blog post by Yehuda that explains this problem and solution
but I can't find it right now.

Reason 2:

I used Erector on a project [1] that needs to create XML documents,
not HTML documents. But some of the methods conflicted, and it needed
a different DOCTYPE, and it was just too icky to use regular widgets.
So I extracted an XMLWidget that does most of the regular Erector
stuff but without predefining tag methods.

I also aliased Widget to HTMLWidget but for backwards compatibility
left Widget meaning HTMLWidget.

So now we have

HTMLWidget < XMLWidget < AbstractWidget

and lots of modules.

[1] https://github.com/alexch/rturk/blob/master/lib/rturk/builders/question_form_builder.rb

Reason 3:

This one's a bug.

While doing the above, I noticed that some of the modules -- esp.
Needs and Convenience, and maybe Caching too -- didn't work right
unless I included them in every class along the hierarchy. I haven't
tracked down the reason. When I do I should be able to pull them back
out into AbstractWidget only, or wherever they're needed.

I figure it has to do with either class instance variable scoping, or
include vs. extend vs. included, or some other weird Ruby metadetail.


--
Alex Chaffee - al...@stinky.com - http://alexch.github.com
Stalk me: http://friendfeed.com/alexch | http://twitter.com/alexch |
http://alexch.tumblr.com

Viktor Tron

unread,
Aug 5, 2011, 2:53:19 PM8/5/11
to erector, Alex Chaffee
a bit of brainstorming about a possible refactor:

it is great to have XML widget but I am not sure if html is a kind of xml
so first I would go for a class/module
which would define that tag/element related generic stuff like
self.tag, self.tag_named, self.full_tags, self.self_closing_tags,
newliney?, and maybe comment, but not instruct or sort_for_xml_declaration
i would call it MLWidget
HTMLWidget and XMLWidget could be subclasses of MLWidget
with their respective specifications, we could also drop Widget from the
names (especially if they were in, say,
a Builder module)

Now for more of a headache there is an other dimension of abstraction:
integration.
rails integration is basically functionality that allows widgets to be
used as view templates.
for Widget class-internally integration delegates view helper methods etc,
form helpers,
assign needs via implicit controller instance variables, binds embedded
rendering to
ActionView render, etc.

So, there are 2 orthogonal dimensions: markup/builder format and
integration.
i am not sure what is the best practice for design in such cases but there
is a million ways to do
it, so we gotta find one which is explicit and simple to understand, use,
and implement.

some assumptions:
1) even in a rails app you may want to use a non-rails widget
2) erector widgets should be easy to use to generate xml format as well as
html in rails as well-behaving templates
see
http://groups.google.com/group/erector/browse_thread/thread/b0f18e0d26b5b4a2?hl=en
3) abstract widget should be totally agnostic to output format ( i mean
the actual markup language)


Practically if you want to use an xml scheme you probably wanna subclass
XMLWidget to define your fave tags
and maybe there are more variables like HTML5 vs XHTML etc.
So this dimension is better candidate for class hierarchy, not to mention
that it is already similar in the codebase :)

Now, we could offer a widget class method (functor to dynamically create a
class, let's call it make), which will expect
an argument for integration and optional modules to include.

module Erector
class AbstractWidget

def self.make(integration, *modules)
Class.new(self) do
include integtation
# here one can include stuff that are dependent on integration
and builder format, like (just illustration)
include Externals if rails_is_included &&
widget_class.new.is_a?(HTML)
include Caching unless rails_is_included
modules.each { |m| include m }
end
end

end

for backward compatibility, we would do:

Erector::Widget = HTMLWidget.make(Erector::Rails if defined?(::Rails))

if you think about it the Inline module is also another 'integration'
InlineWidget = HTMLWidget.make(Erector::Inline)

Now, if you wanna do, say, xml views in rails, you would do
#
class Application::Xml < Erector::XmlWidget
# your tag defs here a la html.rb
end

class Application::XmlWidget < Application::Xml.make(Erector::Rails)
# here you could redefine all kinds of widget internals
# and you can expect super to work correctly here (!)
end

or if no overrides needed, then just
Application::XmlWidget = Application::Xml.make(Erector::Rails)

and then (now ignoring the class naming issue i posted about):
# app/views/posts/index.xml.rb:
class Views::Posts::Index < Application::XMLWidget
needs :love
def content
text "i am content, ", love
end
end

or one could just use

and everything would work even with an xml layout automatically.

In sum, the refactor could be roughly (all within module Erector):

class AbstractWidget
include Text # or Emit
include AfterInitialize
include Convenience
include Needs # now it is incld in xml_widget
# should also include output handling (any delayed emission scheme
abstracted out)
end

class MLWidget
include Element # this may need refactor depending on promise etc
# content of xml_widget,rb
end

class HtmlWidget < MLWidget
include Attributes
include JQuery # should be optional ?
include Sass # should be optional ?
# content of html_widget.rb
end

class XmlWidget < MLWidget
# some xml specific stuff in xml_widget
end

(i am not sure about Externals and Caching, they seem to be integration
components.)

hth
vik

Alex Chaffee

unread,
Aug 5, 2011, 6:48:29 PM8/5/11
to Viktor Tron, erector
On Fri, Aug 5, 2011 at 11:53 AM, Viktor Tron <vikto...@gmail.com> wrote:
> a bit of brainstorming about a possible refactor:
>
> it is great to have XML widget but I am not sure if html is a kind of xml

Technically there are subtle differences between HTML and XML but
they're too small to matter. I like that HTMLWidget descends from
XMLWidget because it reinforces one of Erector's selling points: that
Erector always emits valid documents.

And there are some HTML documents that *are* XML (e.g. XHTML) and
those *may* want to have an XML instruction. They would also want to
support XML Namespaces if and when we support those. (We almost
certainly won't.)

> so first I would go for a class/module
> which would define that tag/element related generic stuff like
> self.tag, self.tag_named, self.full_tags, self.self_closing_tags,
> newliney?, and maybe comment, but not
> instruct or sort_for_xml_declaration
> i would call it MLWidget
> HTMLWidget and XMLWidget could be subclasses of MLWidget
> with their respective specifications, we could also drop Widget from the
> names (especially if they were in, say,
> a Builder module)

I really think our priority should be making stuff that works for
people rather than being precise and perfect about definitions like
XML vs. HTML. Right now an XMLWidget doesn't define any tags but an
HTMLWidget does; that's good enough for me. I think your MLWidget
would not "pull its own weight" as a standalone class.

While we're talking about object names, I've also been thinking about
pulling out the "widgets" module into a separate top-level namespace,
ErectorSet. It's kind of a joke, but also a good idea, since it gives
a clearer distinction between core erector and widgets built on top of
it that not everyone will want.

> Now for more of a headache there is an other dimension of abstraction:
> integration.
> rails integration is basically functionality that allows widgets to be used
> as view templates.
> for Widget class-internally integration delegates view helper methods etc,
> form helpers,
> assign needs via implicit controller instance variables, binds embedded
> rendering to
> ActionView render, etc.
>
> So, there are 2 orthogonal dimensions: markup/builder format and
> integration.
> i am not sure what is the best practice for design in such cases but there
> is a million ways to do
> it, so we gotta find one which is explicit and simple to understand, use,
> and implement.
>
> some assumptions:
> 1) even in a rails app you may want to use a non-rails widget

What's a non-rails widget?

> 2) erector widgets should be easy to use to generate xml format as well as
> html in rails as well-behaving templates

Currently the Rails integration is in a module; including that module
in AbstractWidget would allow both XMLWidgets and HTMLWidgets inside
Rails.

> see
> http://groups.google.com/group/erector/browse_thread/thread/b0f18e0d26b5b4a2?hl=en

Yep, that's a sticky namespace problem. We have a similar problem with partials.

> 3) abstract widget should be totally agnostic to output format ( i mean the
> actual markup language)

Wouldn't it be fun if we could emit JSON and s-expressions from Erector too?

> Practically if you want to use an xml scheme you probably wanna subclass
> XMLWidget to define your fave tags
> and maybe there are more variables like HTML5 vs XHTML etc.
> So this dimension is better candidate for class hierarchy, not to mention
> that it is already similar in the codebase :)

I agree.

> Now, we could offer a widget class method (functor to dynamically create a
> class, let's call it make), which will expect
> an argument for integration and optional modules to include.

You just blew my mind.

Please let me re-read your meta-factory proposal after I stuff my
brains back into my head.

Viktor Tron

unread,
Aug 6, 2011, 2:19:36 PM8/6/11
to Alex Chaffee, erector

sure

> While we're talking about object names, I've also been thinking about
> pulling out the "widgets" module into a separate top-level namespace,
> ErectorSet. It's kind of a joke, but also a good idea, since it gives
> a clearer distinction between core erector and widgets built on top of
> it that not everyone will want.

sure

>> Now for more of a headache there is an other dimension of abstraction:
>> integration.
>> rails integration is basically functionality that allows widgets to be
>> used
>> as view templates.
>> for Widget class-internally integration delegates view helper methods
>> etc,
>> form helpers,
>> assign needs via implicit controller instance variables, binds embedded
>> rendering to
>> ActionView render, etc.
>>
>> So, there are 2 orthogonal dimensions: markup/builder format and
>> integration.
>> i am not sure what is the best practice for design in such cases but
>> there
>> is a million ways to do
>> it, so we gotta find one which is explicit and simple to understand,
>> use,
>> and implement.
>>
>> some assumptions:
>> 1) even in a rails app you may want to use a non-rails widget
>
> What's a non-rails widget?

a widget without rails integration.
Currently, Erector::Widget forces rails integration if you are in rails.
It is delegating to helper methods to the rails view context via
'helpers'), but helpers is only ever non-nil if
the widget it initialized with Erector::Rails.render. This means that
if you call an widget.to_html method and you got rails helpers, you get
wierd errors
NoMethodError: undefined method `link_to' for nil:NilClass
I think it is better if we catch this at the initialization level.
So if I want to use a builder outside rails now i gotta use HTMLWidget.

An inline widget is also a non-rails widget, which delegates
to parent (surely in the context of a parent rails widget it can use
rails helpers as well). However InlineWidget is now a subclass of
Erector::Widget
which is by default rails-ed :)

>> 2) erector widgets should be easy to use to generate xml format as well
>> as
>> html in rails as well-behaving templates
>
> Currently the Rails integration is in a module;

it is and it isn't, see this line
https://github.com/pivotal/erector/blob/master/lib/erector/rails3.rb#L206
it's a self-imposing module :)

> including that module
> in AbstractWidget would allow both XMLWidgets and HTMLWidgets inside
> Rails.

it would, but it would not allow non-rails widgets unless it is included
on demand
and this is exactly the point of my proposal.

>> see
>> http://groups.google.com/group/erector/browse_thread/thread/b0f18e0d26b5b4a2?hl=en
>
> Yep, that's a sticky namespace problem. We have a similar problem with
> partials.
>
>> 3) abstract widget should be totally agnostic to output format ( i mean
>> the
>> actual markup language)
>
> Wouldn't it be fun if we could emit JSON and s-expressions from Erector
> too?

tell us more. usecase?

>> Practically if you want to use an xml scheme you probably wanna subclass
>> XMLWidget to define your fave tags
>> and maybe there are more variables like HTML5 vs XHTML etc.
>> So this dimension is better candidate for class hierarchy, not to
>> mention
>> that it is already similar in the codebase :)
>
> I agree.
>
>> Now, we could offer a widget class method (functor to dynamically
>> create a
>> class, let's call it make), which will expect
>> an argument for integration and optional modules to include.
>
> You just blew my mind.

lol, scary. sorry about that

> Please let me re-read your meta-factory proposal after I stuff my
> brains back into my head.
>

let me have a second go at it now knowing you wanna keep HTML < XML
first about how to refactor the hierarchy and most of all the module
inclusions.

class AbstractWidget
include Text # or Emit

include Needs # now it is incld in xml_widget
# should also include output handling (any delayed emission scheme
abstracted out)
end

class XMLWidget


include Element # this may need refactor depending on promise etc
# content of xml_widget,rb
end

class HtmlWidget < XMLWidget
include Attributes # optional
include JQuery # optional
include Sass # optional


# content of html_widget.rb
end

Now the modules:

AfterInitialize
is this not too much untested feature for something that you get with this?
def initialize(*args)
super(*args)
# extra stuff
end
plus calling super and including it in AbstractWidget before it defines
its own initialize has no effect.

Caching
I think this module could be modified so that there is a class-method
cachable!
which initializes the cache and overwrites _render to consult the cache.
With this I think it can safely be included in AbstractWidget (tho it
should use alias method chain, not super...)
shall I?

Externals
This is a lot of functionality, which i do not really understand and never
used
how about making it an optional include?

Convenience
This module lumps together very different stuff
* to_pretty: is this really needed?
* to_text: there are ways to render html as text, does this really need to
be feature of erector, or at least use an external gem that does the job
fully
* join (soon to deprecate by emit no? :)
* url, css, javascript: these are def only relevant to HTML and not needed
if you got rails helpers, i suggest refactoring them in an optional
html_helpers module
* dom_id: !!! this is in fact a kinda bug I forgot to mention yet, dom_id
is a first-class rails helper method (which i use a lot) but because of
this convenience method, i had to re-re-define it as helpers.dom_id :)

Attributes, JQuery, Sass could also be optional modules to include for HTML

And now recap the pros of my 'metafactory' proposal :)
* it gives the user a very direct API to include any of these optional
modules.
* direct API to make integration both explicit and optional (in line with
point 1 above)
* allowing a widget class for XML with rails integration without losing a
non-rails htmlwidget

Reply all
Reply to author
Forward
0 new messages