Sticky components experiment

[jgaskins]

One of the tradeoffs of current Clearwater components is that they're ephemeral. In a lot of ways this is really great because then you can treat them simply as objects. However, it means that you can't store state on them — they're effectively immutable. Any state has to be stored outside of them and passed down the component tree from some component layer that knows about that store.

This PR includes two experiments. Each of them makes it possible to create stateful, React-style components — I've called it a StickyComponent because naming things is hard. Instead of MyComponent.new(...), you write MyComponent.render(prop1: prop1) (it takes props like React components) to put it into the render tree. For example:

class Layout # Typical Clearwater component
  include Clearwater::Component

  def render
    Toggle.render(initially_on: false)
  end
end

class Toggle < Clearwater::StickyComponent
  def initialize(*args)
    super # Ensure we get props and everything setup

    @on = props.initially_on || false
  end

  def render
    button({
      onclick: proc {
        @on = !@on # Toggle the on state
        call       # Re-render the app
      },
    }, [
      @on.to_s,
    ])
  end
end

In this example, we render a button that toggles between true and false. Nothing fancy, but it exhibits how nothing outside of this component needs to know about its "on" state. Note how we did have to get properties that were passed into here from props. It would be nice not to have to do that, but we can't get around that while still guaranteeing that initialize is called only once in the component's lifecycle.

Differences between experiments

I mentioned above that I included two experiments. You can find them in the diff as Wrapper and Wrapper2 because of my incredible skill in naming things.

Wrapper hijacks the diff phase the same way CachedRender does. It calls a few other methods to emulate React lifecycle methods primarily to show off that it can, but also because they can be useful if you have a stateful component. It's not much different internally from CachedRender except for lifecycle hooks, so performance should be close.

Wrapper2 uses a Clearwater::BlackBoxNode to take even more control of the rendering process. It provides the full spectrum of React-style lifecycle hooks.

The downside of this approach is that it's quite a bit slower than normal Clearwater rendering. My benchmarks of the second approach show that it can take about 2x as long to update than Clearwater's traditional rendering algorithm. In light of this, I added an async_render prop, which provides support for something like I described in #57. This only works for BlackBoxNode#update, and takes longer overall, but it gets some content in front of users faster.

I wanted to start a discussion here to see if we want to support any of this with Clearwater. I admit, stateful components are nice to have sometimes, especially for components provided in a gem. Nobody who uses my gemified component should need to worry about how it stores its internal state. However, I wonder if the rest of it is necessary at all.

[jgaskins]

Also, the 2x performance difference I saw with the async_render property was for a high quantity of very small components (about 3-5 DOM nodes each). If you're only rendering a few dozen components with complex innards, this should be much closer to the default components.

[ajjahn]

I definitely think Clearwater should support stateful components. I agree in most cases it is better to manage state outside of the component, but there are cases where it is needed. A re-usable, toggle component, that is ignorant of the Store or state structure is a perfect example. Every time I need a component to behave like this in Clearwater, it is like hitting a wall. It has forced me to roll my own StickyComponent, (I called it LifecycleComponent). It works very similarly to the Wrapper2 version using the BlackBoxNode.

[jgaskins]

Every time I need a component to behave like this in Clearwater, it is like hitting a wall.

Yeah, I've ended up throwing stuff that would ideally be component state into the data store to get around it, which is kinda gross. In addition to the obvious app-state bloat that comes with this approach, the rest of my app shouldn't have access to something that's specific to a single component.

I've even used BlackBoxNode to render a whole Clearwater app to get this kind of functionality (I wrote that code off the top of my head and never ran it, so it might not even work, but hopefully it expresses my intent).

It's hard to find the "right" abstraction here — using quotes here because none of them are really right. They all involve tradeoffs that are terrible for some use cases. I also don't particularly like having to use something like props. I got spoiled by being able to do whatever I want with my constructors in traditional Clearwater components. :-)

I'm a little sensitive to how this works because I have a feeling people will reach for this a lot more than I would — Ruby developers are known for preferring convenience over all else and not having to write constructors and attr_readers is pretty convenient. So if it's something that makes it into Clearwater itself, it needs to feel good without being detrimental to performance, debugging, or overall application-code quality. I don't know if I've found that balance yet. I'll keep experimenting, though.

[ajjahn]

I've even used BlackBoxNode to render a whole Clearwater app to get this kind of functionality

I have done that as well. It worked, and since I only had to do it in a couple isolated cases, I didn't do any performance testing.

Is there a reason for treating children and other props differently? Unless I've overlooked something, why not intercept the arguments passed to the constructor by overriding the new method?

class StickyComponent
  def self.new(*args)
    instance = super
    instance.instance_variable_set("@props", args)
    instance
  end
end

This would remove the need for the Props class and allow developers to implement their initialize methods anyway they choose.

[jgaskins]

since I only had to do it in a couple isolated cases, I didn't do any performance testing.

Same. I've only done it three times, and finally got fed up and decided to come up with this PR. :-)

Is there a reason for treating children and other props differently?

I really wanted to do something besides props because I love having control of my initialize method, but then update would have to call initialize to reassign instance variables. From a lifecycle perspective, it makes sense to call initialize just once when mounted and never again.

And then, if we give people control over their args, it means that will_receive_props would need to be rethought. We could rename will_receive_props to something like receive_args for two reasons:

• "props" isn't a good fit for a list of positional args
• "will"/"did" don't seem to make sense because we're not just letting the component know "we will/did reassign your props", we're saying "these are your new args".

But then initialize and receive_args would be nearly identical in every component, even if they both just ended up calling another method to do their work. :-(

[jgaskins]

The only way around all that that I could think of would be to call initialize on every update. That's not horrible, but it means you can't assume you're setting up state for the first time, which can probably lead to bugs. You'd have to memoize ivars so they get initialized only once:

class DropdownMenu
  def initialize(menu_options)
    @options = menu_options
    @open = false unless defined? @open # Weird example, can't use ||= to memoize booleans
  end
end

I'm not sure this is the worst idea, tbh. Admittedly, I originally abandoned it without much thought for lifecycle reasons alone, but in the larger picture, this feels cleaner than props. I understand why React uses props+state after this whole exercise, but I don't like it any more than I did before. :-)

[jgaskins]

@ajjahn I went back to using the .render class method in c3bf7cd, but added a warning if you use MyStickyComponent.new in your render tree instead of MyStickyComponent.render. What do you think?

Also, tried out the sticky method to wrap a class and its args:

div([
  sticky(Counter, 12) { do_the_thing },
  Counter.new(12) { also_do_the_thing },
])

[ajjahn]

I think it is reasonable to err on the side of the principle of least surprise and avoid overriding .new, but to me, calling the factory method render is a bit convoluting, simply because we use that term in a few other places already, (component instances, application rendering, etc).

Also, I think it is possible I'm over looking something, but I'm not sure I understand the separate very similar, but separateSticky and Wrapper classes. It seems to me sticky could delegate to the Wrapper class:

def sticky(klass, *args, &block)
  Wrapper.new(args, block) { klass.new(*args, &block) }
end

That would get pretty close to #71 (comment). Do you have any thoughts or feedback on that variation?

[jgaskins]

It's very similar. In fact, I copy/pasted the code in at first, but the main difference is that the block in this one is intended to be passed to the component constructor. The reason we take the class is so that we know what the class is of the new one for invalidation purposes. For example:

div([
  logged_in? ?
    sticky(SessionInfo, current_user) :
    sticky(NewSession),
])

vs

div([
  logged_in? ?
    sticky { |current_user| SessionInfo.new(current_user) } :
    sticky { NewSession.new },
])

In the second example, we would end up trying to tell a NewSession to update itself with something intended for a SessionInfo. This isn't a great example, but any time you swap out a sticky component in the render tree for another, they must be the same class in that case or it will try to update the first with arguments intended for the second — in the best case scenario you get a NoMethodError within update, but the most likely scenario is that you set instance variables and any errors are delayed until the following render. In the first example, we detect the change of class and render it as new.

However, now that I mention that, if they were reordered without setting a key, that would cause them to lose their state anyway. This is probably why whenever passing an array with React, it gives a warning if you don't specify a key property for each array element. We aren't even handling key, but we totally should. I don't think we can do it the way we do for CachedRender (define a key method on the component) because we'd need the component instance. We may need to define a keyword arg on sticky (sticky(*args, key: 'zomg')).

calling the factory method render is a bit convoluting

This is excellent feedback. Do you have any other ideas?

Do you have any thoughts or feedback on that variation?

It's what inspired this experiment. I really like it. Composition over inheritance and all that jazz. I'm just playing around with different ways of doing things to see what feels good to use that also works well in some of the weird edge cases that are sure to crop up. :-)

[ajjahn]

Ah, yes, in fact leaving the class argument out of my example on #71 was an accidental omission. It is supposed to be there.

As far as naming the factory method, when reading a typical component's render method, you'll encounter something the resembles MyComponent.new. And each time render is called, you're basically saying, "Hey MyComponent, give me something that's new." Perhaps saying "Hey MyComponent, give me something that persists.", would reveal the intent more clearly?

Communicating about code is hard, so please forgive me if you feel you've addressed this and I'm just missing it, but what I'm still wondering is why have a StickyComponent type at all? All it really is is a plain old component that know's how to wrap itself in a StickyWrapper.

Here's what I'm getting at, using 'Persist[ent]' instead of 'Sticky':

module Clearwater
  module Persistent
    def self.included(klass)
      klass.extend(ClassMethods)
    end

    module ClassMethods
      def persist(*args, &block)
        Wrapper.new(self, *args, &block)
      end
    end

    class Wrapper
      attr_reader :component

      def initialize(klass, *args, &block)
        @klass = klass
        @args = args
        @block = block
      end

      %x{
        Opal.defn(self, 'type', 'Thunk');
        Opal.defn(self, 'render', function Persistent$render(previous) {
          var self = this;
          if(previous &&
             previous.vnode &&
             this.klass === previous.klass &&
             previous.component) {
            self.component = previous.component;
            if(#{!component.respond_to?(:should_update?) || component.should_update?(*@args)}) {
              if(#{component.respond_to?(:update)}) {
                #{component.update(*@args)};
              }
              return #{component.render};
            }
            return previous.vnode;
          } else {
            self.component = #{@klass.new(*@args, &@block)};
            return #{component.render};
          }
        });
      }
    end
  end
end

module Clearwater
  module Component
    def persist(klass, *args, &block)
      Persistent::Wrapper.new(klass, *args, &block)
    end  
  end
end

Make a component persistent:

class MyComponent
  include Clearwater::Component
  include Clearwater::Persistent # Optionally, this could easily be included in all components by default.
  #...
end

Using a persistent component:

div({}, [
  MyComponent.persist(my, args) { and_block },
  persist(SomeComponent, and_some, args) { and_block },
])

Maybe I should have just PR'd that instead of writing all the code in a comment.

[jgaskins]

I agree, "sticky" is not my favorite name for this. I'm not sure about "persistent", but it's probably an improvement, at least. My main friction with it is that, when I think of "persist" in terms of software, I think of either persistent data structures or object persistence into long-term storage — this doesn't really fit either description very well. So, like render, it's an overloaded term already. :-\

what I'm still wondering is why have a StickyComponent type at all? All it really is is a plain old component that know's how to wrap itself in a StickyWrapper.

I think the idea of composition here is interesting and I'm experimenting with it, but a StickyComponent subclass would be designed around stickiness. I haven't come up with a scenario yet where a component that isn't would see much realistic benefit from it, unfortunately. Do you have any ideas?

I haven't come to any conclusions. It's been rare that I've found a real need for stateful components, so I'm still feeling things out.

[ajjahn]

Yeah, "persistent" is overloaded. Others that come to mind: "perpetual", "fixed", "recurrent", and "perennial." Naming is hard.

I haven't come up with a scenario yet where a component that isn't [designed around stickiness] would see much realistic benefit from it, unfortunately. Do you have any ideas?

Not really. It is just as easy to create a sticky component that simply wraps whatever component that wasn't designed for stickiness. So, yeah, I think I'm back to where I started on that one.

[jgaskins]

I'm still really glad you brought it up. I wouldn't have even thought about it otherwise. So many different ways to slice this pizza and I think there's value in at least exploring them.

[jgaskins]

How do you feel about memoize as a name? It implies that we'll recycle a previous one or create a new one.

class MyComponent < Clearwater::MemoizableComponent
end

class Layout
  include Clearwater::Component

  def render
    div([
      # ...
      MyComponent.memoize,
    ])
  end
end

[ajjahn]

I like it!