Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Syntax for component DSL #22

Closed
jgaskins opened this issue Sep 10, 2015 · 8 comments
Closed

Syntax for component DSL #22

jgaskins opened this issue Sep 10, 2015 · 8 comments

Comments

@jgaskins
Copy link
Member

Several people have said that the component DSL in Clearwater is "ugly" or they don't like it. These are subjective things and people are more than welcome not to like something, but I wanted to start a discussion about it and see if there was a way we could improve it.

One possibility @krainboltgreene and I discussed was adding a mixin (separate from Component) to enable a block-based DSL. Libraries like paggio and @fazibear's inesita (both of which are far more googleable names than Clearwater and that makes me really jealous) support a block-based DSL that builds up DOM nodes (in the case of paggio) or virtual-DOM nodes (in the case of inesita), so I wanted to start there.

I do like how clean those syntaxes are. Here is a comparison between their block syntaxes and Clearwater's content parameter:

# Block DSL
div do
  p class: 'foo' do
    text 'bar'
  end
end

# Clearwater current
div([
  p({ class: 'foo' }, [
    'bar', # I end lines within hashes/arrays with commas because diffs
  ]),
])

There is no denying that the block version is cleaner. It removes a lot of the required syntax from the current Clearwater DSL. So it's pretty obvious that we should start using that block syntax, right?

The problem I'm having is that it's not without tradeoffs. This is the reason @krainboltgreene and I talked about using a separate mixin for components that want to use syntax like that. Here are a few of the tradeoffs I noticed in order to get code that looks that good, listed in reverse order of their MD5 hashes because I find that hilarious:

  • Declarative/procedural impedance mismatch
  • Block context
  • Performance

Declarative/procedural impedance mismatch

Idiomatically, blocks (and, really, all functions/methods) are used for two reasons: return value or side effects. Sometimes both. For example, we use the return value in nearly all blocks passed to Enumerable methods, and we might use the each method to modify something in our app.

The great thing about the block DSL, to me, is that you use a declarative syntax to specify your DOM tree. It's great because that's exactly what HTML is at a high level: a declarative syntax specifying your DOM tree.

The impedance mismatch for me is something I'm having trouble putting into words. Basically, you're calling DSL methods to build a list of children for the parent node. But where is the ultimate value of that going? You have to assume it's keeping track with an accumulator. Is that accumulator reset if I tell it to render the same component twice? These are the sort of questions that drive me mad while debugging. :-\

Block context

Both of the libraries I mentioned above allow you to call methods with the same name as DOM node types (div, ul, etc), but neither one defines those methods on the component. So how does that work?

Well, the blocks are not executed in the same context in which they are defined, which is always something that bugs me about JavaScript (nobody ever knows what this refers to at any given time in any JS app ever), so I try to stay away from it in my Ruby code. It can be powerful when used in moderation, but defining my entire app UI with it would be way beyond my headspace.

So what if you need to call methods on your component or use instance variables while rendering? Well, both libraries delegate to their parent context with method_missing, which takes care of the method calls, but instance variables are out of the question. There isn't a way, as far as I know, to delegate instance-variable lookup to other objects.

Performance

Anyone who's been following my contributions to Opal lately has probably noticed that they've been almost exclusively performance-related. I'm a bit of a performance nut, mostly because I'm impatient with electronic devices. It's one of the reasons ActiveRecord bugs the shit out of me. :-\

So when the block syntax was first suggested to me, the first thing that came to my mind was "what is the performance impact?" After looking at the code in paggio and inesita, I can tell you quite confidently that the performance impact is very nontrivial. As mentioned in the previous section, their DSLs walk up the tree to the component using method_missing. Now, we don't have to deal with the performance impacts of method_missing that server-side implementations have, but walking that tree makes every method call to your component happen in O(log(n)) time while rendering.

This isn't a problem with their code. It's working as well as the design is letting them. The design isn't bad, either. It's just a tradeoff. This is the same problem Ruby has. Several Ruby implementors have mentioned that the design of the Ruby language actually limits how fast the implementation can go. It's a similar situation here, where you trade performance for developer happiness.

Bearing all this in mind, while I do think developer happiness is very important (I'm doing all this so I can write Ruby in the browser instead of JavaScript, after all), the component DSL is the single hottest code path during rendering at the app level, and probably the hottest in most apps. For apps that generate large DOMs, we need something that scales well.

So I'm not going to say I don't want the block syntax (or I wouldn't be creating this issue), but I wanted to put all my cards on the table and see if there is a way we can work around the performance issue at least. If we can, I think it'd be nice to add a second mixin that enables the block syntax (my first two objections mean I don't want it to replace the default entirely). If we can't, it might be better for the block syntax to be a separate gem.

@jdickey
Copy link

jdickey commented Sep 13, 2015

👍 for the whole discussion, @jgaskins; very well stated.

The thing that raised my eyebrows was the last sentence,

If we can't, it might be better for the block syntax to be a separate gem.

Maintaining two Gems (or equivalent software components in any language) that "do the same thing, just in more-or-less different ways" is an absolute guarantee in practice that over time, the two implementations will diverge. Not just "Gem A uses Classic syntax and Gem B uses block syntax", but "the two Gems implement Feature X differently enough that you need to be aware of the tradeoffs in using one vs the other". Having used that particular style of block syntax before (most recently in Fortitude, essentially a fork/enhanced implementation of Erector), I think it's Great Stuff™. The "classic" syntax will always be "ugly" in comparison. The question before this House appears to be, is that necessarily a bad thing? And if it is, how do we get out in front of that in-practice-inevitable divergence and manage it?

@krainboltgreene
Copy link

an absolute guarantee in practice that over time, the two implementations will diverge

@jdickey: Not if each fluent language style simply "compiles" down to objects the way react expects. Then it doesn't matter what public API they decide, because they still have to adhere to the internal DOM element API.

@jgaskins
Copy link
Member Author

@krainboltgreene I think @jdickey's concerns are warranted, but I also think you're right that if we stick to a simple contract between Clearwater and the component, a separate implementation shouldn't be a big deal.

As it stands right now, the contract between Clearwater and components is:

  • Routed components have router and outlet setters (I don't believe it requires getters).
    • This does not apply to non-routed components (those instantiated directly within other components), but it's provided in the mixin anyway.
    • We can remove this as a requirement by doing runtime checks for component.respond_to?(:outlet), since it only happens once per render and isn't a hot spot.
  • Their render method returns something we can sanitize for the virtual-dom library.
    • Examples: string, number, a vdom node, other components, or an array of any of the above.
    • The only exception is that the root component MUST return a virtual-dom node.

That's it. If we stick to that, we don't have to worry about breaking one implementation in favor of the other. Still, I appreciate your thoughts on that, @jdickey.

@jgaskins
Copy link
Member Author

The "classic" syntax will always be "ugly" in comparison. The question before this House appears to be, is that necessarily a bad thing?

I think this is the most important question on the table. The only answer I have is that I actually like the way it exists currently, so it's not a problem for me. Specifically, even though I think a cleaner syntax would be nice, I prefer the concept of passing the content as a value rather than a block.

Someone brought up at one point that if it does get cluttered more quickly, it's likely to encourage people to keep components small and focused; extracting chunks of the UI into named components is a good idea. Of course, it also means that lazier developers still won't do that and they'll end up with a mess and probably blame Clearwater for being ugly. It's a hard balance to strike.

The performance concerns I have aren't concrete because I haven't benchmarked an Opal implementation of the block syntax. I'd be interested in seeing a performance comparison for a nontrivial render. I'd be happy to provide feedback on an implementation, but since it's not an itch I have, I don't think I'll be the one to scratch it.

@krainboltgreene
Copy link

I think largely the issue was just the readme, which we've fixed.

Once I show developers the implementation it was a lot easier to convince them of the use:

class User
  include Clearwater::Component

  def render
    element(properties, [h1("Hello"), h2("By Kurtis")])
  end  
end

@krainboltgreene
Copy link

Though to be fair that same terseness works for fluent languages:

class User < Clearwater::Component

  def render
    element(properties) { h1("Hello"); h2("By Kurtis") }
  end
end

@jgaskins
Copy link
Member Author

I thought I responded here yesterday. I must not have hit the submit button.

Basically, what I was going to say was that an update to Inesita last week addresses the second and third concerns I had about the block syntax in my original post. That is, it turns out it's possible to do it without instance_eval, freeing you from being in the wrong context to call methods on the component and having to walk up the stack every time.

However, my first point still stands. I've had a lot of time to think about it and I don't think the connotations of Ruby blocks really match what the render method is intended to do. If people really want the block syntax, my recommendation will be for them to use Inesita or implement a similar component to use in Clearwater apps (honestly, I'm not even sure Inesita::Component can't be used in Clearwater). I fully intend to keep the contract between Clearwater and components very simple, so a third-party implementation should realistically be able to keep up with changes.

@jdickey
Copy link

jdickey commented Oct 20, 2015

@jgaskins: Sounds great. Inesita hit my radar last week (independently of your mention at the start of this issue, which I somehow completely blew past, along with paggio). It's great that we've got multiple tools out there now, and absolutely fantastic if they can work together without stomping on each other. There's a lot of wailing and hucksterism pushing the idea that "Ruby isn't 'hot' anymore". Glad to see projects like these as counters to that. Glad to see the README was fixed and we've all got some more to think about. Thanks again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants