[rewrite] consideration for component initial state config

[cavemansspa]

One thought after first v1 test drive. Would there be a consideration for an explicit state configuration block? Currently, it looks like all of the component's properties are copied to vnode.state on initialization.

An alternative would be to provide a "config" block that the component writer can explicitly define the object they want copied to vnode.state. Similar to the manual steps that are done in this fiddle: https://jsfiddle.net/4xazqtjd/8/

If the "config" block contains a function, mithril, will scope it with vnode.state and vnode arg, effectively giving you a user-defined lifecycle like function on the component.

This suggestion would enable a mechanism that let's the component writer implement component level and instance level functions.

[lhorie]

On one hand, I think it makes phylosophical sense to have a distinction between static and instance methods. On the other, from an architectural perspective, I've been leaning heavily towards lean component. Somehow every time I make fat components, it comes back to bite me

[dead-claudia]

Edit: s/render/view/g

@lhorie

Somehow every time I make fat components, it comes back to bite me

I agree with this sentiment, and that is also why I've grown an avoidance of static methods on classes altogether in favor of additional named exports.

Edit 2: Ignore everything from here down. I don't know what I'm talking about... :smile:

But I do feel too much responsibility is being thrust upon the component object. Something doesn't feel right copying everything off of the component itself to the instance, from both a memory and architectural perspective. What's being suggested is instead of this:

var component = {
    counter: 0,
    increment: function () { this.counter++ },

    onupdate: function () {
        this.increment()
    },

    view: function () {
        return m("div", ["updates", this.counter])
    },
}

doing this instead:

var component = {
    config: {
        counter: 0,
        increment: function () { this.counter++ },
    },

    onupdate: function () {
        this.increment()
    },

    view: function () {
        return m("div", ["updates", this.counter])
    },
}

The main benefit I see is that you're not unnecessarily copying all the hook methods to the vnode's state every time, wasting way too much memory and precious CPU cycles in the process. Also, you can just not copy anything in the common case of no config.

IMHO, though, the state should prototypically inherit from the component instead of just starting out as a shallow clone. In this case, 90% of my concerns are moot, and it's just an aesthetics question again. (Oh, and that would offer a very nice chance to simplify the renderer substantially.)

[dead-claudia]

Edit: Continuation of my second edit above.

Oh, and using inheritance to simplify the renderer would also make it easy to fix #1483. Leaving a note there.

[dead-claudia]

Actually, @cavemansspa, just out of curiousity, is oninit what you're looking for? My examples above would directly translate into this, currently possible in the rewrite:

var component = {
    oninit: function (vnode) {
        this.counter = 0
    },

    increment: function () {
        this.counter++
    },

    onupdate: function () {
        this.increment()
    },

    view: function () {
        return m("div", ["updates", this.counter])
    },
}

[cavemansspa]

@lhorie @isiahmeadows -- thanks for having a look. I would agree that a fat component is not desirable.

This suggestion is stemming from some test components that i experimented with to retro-fit my v0.2 apps / patterns. Which btw v1 makes things much easier overall.

I recognized that the lifecycle methods have pretty much everything you'd need (i.e. a scope of vnode.state and a param of vnode) in terms of accessing a components structure. So i experimented with that jsfiddle with a config block and then thought that would likely be a useful approach in general if mithril wired your component defined functions with the same signature as the lifecycle methods. And you get the additional benefit of being able to bifurcate component level functions if so desired.

You could have the option to provide a config block where you get the proposed behavior, otherwise state will work as it's currently implemented.

I figured it would be worth suggesting to see what you and others thought.

[dead-claudia]

For what it's worth, creating a wrapper for what you want is trivial anyways (hence why I'm hesitant):

function withConfig(C) {
    var oninit = C.oninit || function () {}
    C.oninit = function () {
        var state = vnode.state = {}
        for (var key in C.config) {
            state[key] = C.config[key]
        }
        return oninit.apply(state, arguments)
    }
    return C
}

On Sun, Dec 18, 2016, 19:59 cavemansspa notifications@github.com wrote:

@lhorie https://github.com/lhorie @isiahmeadows https://github.com/isiahmeadows -- thanks for having a look. I would agree that a fat component is not desirable.

This suggestion is stemming from some test components that i experimented with to retro-fit my v0.2 apps / patterns. Which btw v1 makes things much easier overall.

I recognized that the lifecycle methods have pretty much everything you'd need (i.e. a scope of vnode.state and a param of vnode) in terms of accessing a components structure. So i experimented with that jsfiddle with a config block and then thought that would likely be a useful approach in general if mithril wired your component defined functions with the same signature as the lifecycle methods. And you get the additional benefit of being able to bifurcate component level functions if so desired.

You could have the option to provide a config block where you get the proposed behavior, otherwise state will work as it's currently implemented.

I figured it would be worth suggesting to see what you and others thought.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/lhorie/mithril.js/issues/1484#issuecomment-267860717, or mute the thread https://github.com/notifications/unsubscribe-auth/AERrBO-BSgrdmtTXkYug9oh6eWyPlJjwks5rJdbxgaJpZM4LP6aP .

[barneycarroll]

I've spent a lot of time experimenting with various component declaration & initialisation patterns off the back of this, which have revealed what I believe to be a serious gotcha in the component API.

the state of this

My interest was piqued by the fact that @cavemansspa's fiddle reveals surprising behaviour when logging vnode.state: my console showed an object which didn't match up with my expectations based on looking at the component declaration:

▶ constructor {
  someProperty : "hello world",
  toggle : ( Function )
}

This turns out to be because vnode.state creates a temporary constructor in order to bind the component definition (vnode.tag) as the prototype of vnode.state (render.js, lines 199-102. This means each component instance will have its own private constructor type, and any properties declared on the component directly are interfaces (I've tried to demystify the implementation a bit in this patch).

Here's a bin I wrote to get to the bottom of what is and isn't true of component vnode instances which illustrates the prototype behaviour at runtime (the buttons are there to illustrate the vagaries of binding functions, but I won't go into that).

My initial conclusion to the findings above are that they're exotic and unintuitive. Emulating classes is of no practical use for the purpose of the supposed functionality AFAICT.

defining stuff in oninit makes everything simpler

I was glad @isiahmeadows brought up oninit - by sidestepping ambiguous framework internal logic and letting you define state explicitly. Consider the following:

const SomeComponent = {
  oninit( vnode ){
    vnode.state.flag = false
    vnode.state.string = ''
    vnode.state.toggle = () => vnode.state.flag = !vnode.state.flag
    vnode.state.write = function( e ){
      e.preventDefault()

      vnode.state.string = this.input
    }
  },

  view : vnode => [
    m( 'p', 'vnode.state.flag = ', vnode.state.flag.toString() ),
    m( 'p', 'vnode.state.string = ', vnode.state.string ),

    m( 'button', { onclick : vnode.state.toggle }, 'Toggle flag' ),
    m( 'input', { oninput : vnode.state.write, value : vnode.state.string } )
  ]
}

The advantages of writing components this way:

1. No dependency on exotic framework mechanisms - state is manually defined and assigned
2. Component declaration shape is always consistent, all state internals are defined in one place
3. You can define initial state dynamically - eg based on vnode.attrs
4. Thanks to the closure, you get access to vnode, vnode.state & whatever this and arguments are supplied to state methods

Those last 2 clinch it for me: rather than belly aching as to where and how you should define your initial state & methods, define them here and you don't have to choose whether its more important to have access to vnode.state or the calling context - you can have both. You only ever need refactor the function definition itself, rather than the structure it was declared in.

gotcha: oninit is not a panacea

The irritating thing about even providing the option of using the tag as a prototype is that what @cavemansspa is suggesting feels totally intuitive - there's a nominal convenience in having consistent signatures for methods declared on the component tag, and we know that a lot of people dislike declaring functions in other functions - especially in views. So if we have a component whose only moving parts are a view and a method, you can see the appeal of a component of shape { customMethod, view } for brevity.

I wrote a wrapper to enforce consistent signatures across component tag methods such that you can always get at the vnode regardless of calling context, which enables statically defined functions to deal with state and up to date input attrs. I wouldn't recommend implementing this: for everything I just said, it's still weird to have these functions that can't receive anything from the calling context.

However it does reveal a big gotcha in the fat oninit pattern - vnodes are disposable entities that become stale after redraw, meaning anything defined in oninit will only have access to the vnode.attrs, of the initial invocation. See this naive implementation, where changes to input are never acknowledged.

The idiomatic solution is to declare functions depending on dynamic values at call site, which is incidentally shorter than both of the above. This is good functional programming practice IMO because it steers clear of the ambiguous stateful properties of instance-sensitive methods and deals with plain, pure functions instead of relying on passing things between contexts.

Does this warrant a component anti-pattern documentation entry?

[lhorie]

It occurs to me that you can do static methods by simply creating a nested object

var MyComp = {
  instanceMethod: function() {/*...*/},
  static: {
    staticMethod: function() {/*...*/}
  },
  view: function() {/**/}
}

MyComp.static.staticMethod()

But again, considering the usage pattern, they might not belong in a component in the first place

[dead-claudia]

@barneycarroll

This means each component instance will have its own private constructor type

Nope. It's actually functionally equivalent to Object.create(component). Here's roughly how new works with ES5-style constructors: *

function Construct(C, ...args) {
    var object = Object.create(C.prototype)
    var result = C.apply(object, args)

    if (result != null && typeof result === "object") {
        return result
    } else {
        return object
    }
}

If C is a no-op, then result is always undefined, and so object is always returned unmodified, hence equivalent to Object.create(C.prototype). And if you set C.prototype right before calling new C(...args), that's why the library code is effectively equivalent to vnode.state = Object.create(vnode.tag).

Edit: late to the party. :smile:

The only nit I have about it is that Object.create should've been used by default, as it's supported in all versions, has much clearer intent, and is actually much faster than even a shared no-op constructor (no prototype changes).

* This ignores the check for non-object, non-null prototypes, which are implicitly coerced to Object.prototype, but that's not relevant here.

[dead-claudia]

@lhorie

But again, considering the usage pattern, they might not belong in a component in the first place

Why I prefer named exports any time I'm exporting more than a single function or class. Way easier to scale IMHO, since I'm not coupling types with instance-independent functions.

[RobertAKARobin]

Old news, but here's my approach:

var Project = (function(){

    // Everything is private, unless attached to $Class or $Instance
    var $Class = {};
    var $Instance = {};
    var $Instance_Constructor;
    var $DOM_Constructor;

    $Class.all = [];
    // Class methods
    $Class.new = function(){
        var project = Object.create($Instance);
        $Instance_Constructor.apply(project, arguments);
        $DOM_Constructor.apply(project);
        $Class.all.push(project);
        return project;
    }

    $Instance_Constructor = function(title){
        var project = this;
        project.title = title;
    }
    // Instance methods
    $Instance.mountTo = function(element){
        var project = this;
        return m.mount(element, project.DOM);
    }

    // Creates a unique Mithril component for each instance
    $DOM_Constructor = function(){
        var project = this;
        project.DOM = {
            view: function(){
                return [
                    m('h1', project.title)
                ]
            }
        }
    }

    return $Class;

})();

document.addEventListener('DOMContentLoaded', function(){
    var project = Project.new('My sweet project');
    project.mountTo(document.getElementById('project'));
});