Differed init + update + lifecycle 'hooks'

[ajjahn]

I spent some time thinking about the approaches and discussion in #70 and came up with this. It still suffers from some tradeoffs (possibly two places dealing with passed in args), but for me feels a bit closer to the "right" abstraction.

In this implementation of StickyComponent:

• initialize is called only once per instance with the arguments given to new.
• There is an update method that receives args for subsequent renders. This is the formal place to operate on passed in properties.
• By default, initialize delegates to update to avoid duplication, but can easily be overridden.
• The will_ and did_ methods are replaced with 'hooks'. I don't see any reason for these to be passed arguments since they'll be the same values until the next render. I do, however, find it useful to be able to hook into various stages of the lifecycle. For example, before_update would be to save current state before it is replaced instead of adding additional logic to the update method.

[jgaskins]

I really like how clean this is.

[jgaskins]

I like the idea of passing the block here, so Wrapper doesn't need to assume it's calling klass._new(*args). We may be able to skip taking *arguments in the block, though. They should be the same as *args if I understand the flow here.

[ajjahn]

Yeah, it should be able to just use *args, but I had issues when I didn't explicitly define *arguments. Opal on occasion does some mysterious things with passing blocks, and when I see something weird happening, I default to just being verbose.

[jgaskins]

If we go with the Thunk approach, we may only be able to provide unmount hooks with a virtual-dom unhook, but I'm not 100% sure if that'll work. I'm reasonably sure that, unless you provide the same instance every time, unhook will be called on every render.

The good news is that unhook takes a next value, which will probably be null or undefined when the component is actually unmounted.

[ajjahn]

Ah, good catch. I completely forgot about before_unmount. I'll tack on an unhook and see what happens.

[jgaskins]

Crap, it looks like hooks don't work on thunks — only vnodes. I wrote a quick UnmountObserver class for Wrapper to use (specifically, Wrapper set one up as an instance variable for virtual-dom to read as one of the properties of the thunk) and hooks weren't firing. When I replaced …

return #{component.render}`

… with …

return h('div', { observer: #@unmount_observer }, [#{component.render}]);

… they began firing.

[ajjahn]

Interesting. Well, that's unfortunate, but honestly, I personally could survive without before_unmount. The extra hooks are nice to have, but for me, these StickyComponent PRs are about components keeping state.

[jgaskins]

This is a great point. It's a good idea to stick with the simplest thing that works, which means foregoing callbacks until a need presents itself. If the only way to do certain things is via callbacks, we can revisit it then.

I guess I just got too focused on making them work if we're going to provide them at all. :-)

[ajjahn]

Okay, I dropped the callbacks; should probably figure out how to properly test this.

[jgaskins]

One of the reasons I used .render instead of .new was so you could choose use a StickyComponent instance as a regular component. I don't know if that's a reasonable thing to do, though. What are your thoughts on that?

[ajjahn]

I think I land on the side that if you define a StickyComponent, then you have a StickyComponent. If you need a component to behave both sticky and non-sticky, just make a sticky wrapper component for a non-sticky component. Well defined responsibilities for each type of component will encourage smaller composable classes.

[jgaskins]

Well said.

[jgaskins]

It looks pretty good so I went ahead and merged it into my PR. I'm still a little iffy on overriding .new. It feels awkward for it not to return an instance of the component. It caused a bit of a disconnect for me when playing around with it.

[ajjahn]

Yeah, overriding new definitely has pros and cons. I like that I can use a StickyComponent exactly the same as any other component, and have a custom initializer like I'm used to. Just call MyComponent.new(my, args) and, from the outside, I don't have to care that it is 'sticky'. On the flip side, I can't really interact with a StickyComponent instance other than just declaring it and letting it do its thing.

Okay, this might go against the grain of my earlier comment, "if you define a StickyComponent, then you have a StickyComponent", but what if the sticky facility became less about the specific 'type' of component and more about telling any component how you want it to behave in a given context.

Add this to the component module:

module Clearwater
  module Component
    def self.sticky(*args)
      StickyWrapper.new(*args) { |*arguments| new(*arguments) }
    end
  end

And just have a StickyWrapper, not a StickyComponent class:

  class StickyWrapper
      attr_reader :component

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

      %x{
        Opal.defn(self, 'type', 'Thunk');
        Opal.defn(self, 'render', function Component$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)}) {
              #{component.update(*@args) if component.respond_to?(:update)};
              return #{component.render};
            }

            return previous.vnode;
          } else {
            self.component = #{@block.call(*@args)};
            return #{component.render};
          }
        });
      }
    end
end

When invoking MyComponent.new(some, args), you get an ephemeral instance of MyComponent.
When invoking MyComponent.sticky(some, args), you get an long living instance wrapped in a StickyWrapper.

What do you know, another option on how to implement this.