Implement Component / Widget

[Raynos]

A widget is a primitive similar to Diffable<T>

It allows for implementing sub applications, components, sub tree re-rendering, stateful components, thunks and other concepts outside of virtual-dom

A widget consists of a init, update and destroy methods

type Widget := {
  init: () => DOMElement,
  update: (previous: Widget) => void,
  destroy: () => void
}

When virtual-dom encounters a widget it will init() it if it hasnt seen it before and replace the DOMElement with the created DOMElement.

If virtual-dom encounters a widget in both the left and the right tree it will call right.update(left) to update the right widget with the previous state.

If the virtual-dom sees that a widget has been removed i.e. left is a widget and right is some other kind of node it will call destroy() in which widget can do cleanup.

See an example stateful-widget here

[Matt-Esch]

I am not sure what you consider left and right trees, but suppose that you patch the right tree on top of the left, you ought to pass the new widget structure into the old one, in case there are any stateful concerns, ie it should be right = left.update(right), or for clarity, current = previous.update(current)

[Matt-Esch]

update: (previous: Widget) => void should be update: (previous: Widget) => DOMElement so that you can replace the root node of the widget.

[Matt-Esch]

There should be a way of checking type equality of the widgets

[Raynos]

so you can do either

right = left.update(right)

or

right.update(left)

either one works and you can convert between the two.

[Raynos]

update: (previous: Widget) => DOMElement

The only use case for this is swapping out the top level dom element. I say that use case needs to be demonstrated.

most widgets will probably just use a wrapper div who's tagName they never change.

[Raynos]

There should be a way of checking type equality of the widgets

compare the init and update functions to each other. Or introduce a type string key.

[Matt-Esch]

Right is not initialised. right.update(left) requires intialisation of right in some way, so it's not the same thing. Some manual cloning or something is required. Preserving all state that left is in.

Type checking using a type string would make this worse, if update is 2 different functions with unreadable closure state. You can only update the initialised component safely.

right = left.update(right) is problematic when right is the root of a tree, since it cannot just be replaced.

the problem is that the right tree must contain the updated left node, and that's only possible by replacing the right node with the left node and passing the right node into the update function of the left

[Matt-Esch]

I disagree that the use case for replacing the root node needs to be demonstrated. It's not problematic, it's simply for completeness. We can make it Maybe if you would prefer that. We currently have support for the root node in the entire tree.

[Raynos]

Correct, right is not initialised, this means update() will have similar logic to init(). It will need to copy properties from the left

[Matt-Esch]

That's not really acceptable

[Raynos]

Turns out update() does need to return a DOMElement

[Raynos]

function update(previous) {
  // previous is a different type of widget
  if (previous.type !== this.type) {
    return this.init(); // use the init() path instead
  }
}

Unless you want virtual-dom to do that check instead

[Matt-Esch]

vdom treats the widget like any other node. It has to be able to sort, index and match node types to produce the correct patch. Patch will support removing, thus calling destroy() on the widget, updating (passing the right into the left) and initialising, inserting a new node

[Matt-Esch]

A widget should not expect its existence in a virtual tree, nor should it expect update to be a widget itself. You just pass the data to update, the fact that it is a widget when vdom calls update is irrelevant. That's why when you call update on the left, you are simply passing data to a potentially stateful thing. The existence of the widget functions in the prototype makes no difference and is not expected. There is no expectation of type being defined. Updating the right with the left is a pretty coupled and boilerplate prone option to consider.

Outside of vdom, a widget is still usable with a reasonable interface. I would not want to update a widget on the page by instancing a new widget with the new data and passing the old widget into it. I want to pass the new data into the update function.

[Raynos]

https://gist.github.com/Raynos/9590818

So there's an implementation of thunk widget.

The most important thing is that given 4 thunk widgets [a, b, c, d] from 4 trees and 4 diff calls

You must either do

a.update(b)
b.update(c)
c.update(d)

or

b.update(a)
c.update(b)
d.update(c)

NOT

a.update(b)
a.update(c)
a.update(d)

The last one will break and fail. To write a pure widget it must have a contract that a widget is an object with an init & update function and one of those and ONLY one of those will ever get called.

To make that more obvious I went for .update(previous)

[Matt-Esch]

a,update(b)
a.update(c)
a.update(d)

I think this is correct and what I have opted for. It doesn't force impurity in terms of having to store and mutate state other than the dom related state (which has to be stored somewhere), and it allows for a wider range of stateful things, plus allowing for a natural interface for usage outside of vdom. You can think of it as a sub-application context, which is a triplet of rootNode, vdomTree and state

A default widget would allow you to define a pure render function

function init() {
    var vdom = this.tree = render(this)
    var rootNode = this.rootNode = dom(vdom)
    this.state = null // see it's gone
    return rootNode    
}

function update(state) {
    var vdom = render(state)
    var patches = diff(this.tree, vdom)
    this.rootNode = patch(this.rootNode, patches)
    this.tree = vdom
}

function destroy() {
    this.rootNode = null
    this.tree = null
}

[neonstalwart]

i've been trying to use widgets as they are currently implemented and something isn't quite right about it (maybe i'm misunderstanding).

i have a widget like this:

var render = require('virtual-dom/render'),
    diff = require('virtual-dom/diff'),
    patch = require('virtual-dom/patch');

module.exports = Widget;

function Widget(value, render) {
    this.render = render;
    this.value = value;
}

Widget.prototype.init = function () {
    this.vdom = this.render(this.value);
    this.dom = render(this.vdom);
    return this.dom;
};

Widget.prototype.update = function (update) {
    this.value = update.value;

    var vdom = this.render(this.value);

    this.dom = patch(this.dom, diff(this.vdom, vdom));
    this.vdom = vdom;
};

Widget.prototype.destroy = function () {
    this.dom = null;
};

and using this pattern to render, diff, patch a view that contains widgets

var vdom, dom;

function renderDom() {
    // view returns the vdom which includes some widgets
    var newVdom = view(list);

    dom = dom ? patch(dom, diff(vdom, newVdom)) : render(newVdom);

    if (!dom.parentNode) {
        document.body.appendChild(dom);
    }

    vdom = newVdom;
}

after the first call to renderDom, vdom contains an initial list of widgets (that have been initialized) and dom contains the elements associated with those widgets. during the 2nd call to renderDom, the initial set of widgets have update called and dom still contains the elements from the initial set of widgets but the problem is that vdom has a different set of widgets now (which have not been initialized) and when renderDom is called a third time, update is called on the 2nd set of widgets.

should the initial set of widgets be replacing the 2nd set of widgets in vdom OR when update is called am i supposed to transfer the element from the current widget to the next one (rather than updating the value of the current one) OR ... ?

from the discussion so far i'm uncertain about the intent of the current implementation - i get the feeling that @Matt-Esch intends for the initial set of widgets to replace the 2nd set of widgets in the vdom tree but the code doesn't currently do that so maybe i'm misunderstanding.

[Matt-Esch]

You are correct that this was what I was originally planning. I.e. right = left.update(right). Nice example by the way. It does show that what I was expecting is easier to understand. I am personally surprised it worked at all, as I just sat down and wrote it and have never tried it myself :P

There has been some disagreement as to how widgets should update. The question comes down to should we have

right.update(left) or right = left.update(right)

Your understanding has followed my preference so far, that is right = left.update(right). Currently the right = part has been removed, only very recently, during a refactor addressing immutability concerns.

@Raynos primary concern was about immutability, i.e. he was concerned that several aspects of the diff patch cycle made changes to the trees, voiding certain assumptions. Patching into another virtual tree seems wrong.

In response to this I decided that it's actually quite possible to build immutable trees. The benefits of such are that caching trees in places, reusing them etc is less problematic. Indexing the complete tree was limiting, it required duplication of trees if you used them more than once. Now instead of writing the tree index on diff, we simply place a count of the number of descendants on each node on creation, so the tree-order index can be computed very quickly when we descend the tree recursively. So now we can call Object.freeze on the nodes on creation (but we won't do that for performance). Unfortunately widget hasn't been updated to reflect these changes yet.

If you take a look in the history you will see that widget was in fact updating at one point in the way that you expected, i.e, the widget in the right tree was being replaced by the widget in the left tree, and the left widget being called with update(rightWidget), just as you expected.

With the immutability concerns however, this is in the process of being reverted back to what we originally planned, which was, given a patch b, call b.update(a). So instead of passing new state into the old widget, we are passing the old state into the new update function, if that makes sense.

I figured that this was better overall, but I think the interface of new.update(old) is really cumbersome and unintuitive. There will be a higher order function to wrap widgets though, allowing you to write widgets with an interface of old.update(new).

So to summarise, the immutability concerns of indexing have been addressed, but the widget behaviour hasn't been corrected in line with this. The feature is currently untested, as with hooks, so expect it to change a bit.

[Matt-Esch]

Just to give you an example of how you might write this when the new update ordering is implemented:

var render = require('virtual-dom/render'),
    diff = require('virtual-dom/diff'),
    patch = require('virtual-dom/patch');

module.exports = Widget;

function Widget(value, render) {
    this.render = render;
    this.value = value;
}

Widget.prototype.init = function () {
    this.vdom = this.render(this.value);
    this.dom = render(this.vdom);
    return this.dom;
};

Widget.prototype.update = function (previous) {
    var vdom = this.vdom = this.render(this.value);
    this.dom = patch(previous.dom, diff(previous.vdom, vdom));
    previous.destroy(); // optionally kill the previous dom reference
};

Widget.prototype.destroy = function () {
    this.dom = null;
};

I'll use this example in the tests

[neonstalwart]

so i liked right = left.update(right) because it meant that right.init() wasn't called just to form the patch but i disliked it because left does not have access to the same closure as right - for example

function view(data) {
  return h('ul', data.things.map(function (thing) {
    // right and left could be referencing a different `data` that might influence what happens here
    return h('li', data.logic ? thing.one : thing.two);
  });
}

i also like that a widget should have no awareness of being part of vdom.

where you're going with what you've described, it is looking like there is less and less need for both init and update and at one point in playing with this i even went as far as something like this:

Widget.prototype.init = Widget.prototype.update = function (widget) {
  if (widget) {
    this.value = widget.value;
  }

  var vdom = this.render(this.value);
  this.dom = this.dom ? patch(this.dom, diff(this.vdom, vdom)) : render(vdom);
  this.vdom = vdom;

  return this.dom;
}

with what i'm understanding about where you're heading, we might just need one method and it could be implemented like this (it probably shouldn't be called init or update though, maybe render?):

Widget.prototype.update = function (previous) {
  if (previous) {
    this.dom = previous.dom;
    previous.destroy(); // ??? should the patch do this for us?
  }

  var vdom = this.render(this.value);
  this.dom = this.dom ? patch(this.dom, diff(this.vdom, vdom)) : render(vdom);
  this.vdom = vdom;

  return this.dom;
}

[neonstalwart]

oh... i only just saw your last change to the code. i still think you could reduce it to one function so, something like this is probably what i'll try later with what you have now.

Widget.prototype.init = Widget.prototype.update = function (state, dom) {
    var vdom = this.render(this.value);
    dom = dom || render(vdom);

    this.dom = patch(dom, diff(this.vdom, vdom));
    this.vdom = vdom;

    return this.dom;
}

given that widgets should not be aware of being part of a vdom tree

• they shouldn't be responsible for destroying the previous widget
• they should be able to update themselves - i.e. widget.update(widget, widget.dom);

[Raynos]

@neonstalwart I agree that init() and update() can be collapsed into a single function.

I like the idea of passing in a dom node to a widget so that a single widget instance can be re-used in multiple locations in the tree.

However a widget should still need a .destroy() (which can be optional).

Returning a dom node is indeed important.

[Raynos]

@neonstalwart

function view(data) {
  return h('ul', data.things.map(function (thing) {
    // right and left could be referencing a different `data` that might influence what happens here
    return h('li', data.logic ? thing.one : thing.two);
  });
}

Your example is flawed as the view() function runs to completion and all the closures have already been called by the time it returns.

but i disliked it because left does not have access to the same closure as right

Please provide a better example for this issue.

[Matt-Esch]

So what was the motivation for an init function? Well we needed something to create a DOM node, because originally only init was returning them.

Now update will optionally return a DOM node. It must return the DOM node on init (or first call to update), although we could insert a placeholder if no dom node is returned. When it returns a DOM node, the referencing node is patched.

Update came about from a preference (of @Raynos) to have update actually look like:

function update(previous, current) {

}

The motivation for the inclusion of an init function was to mirror the destroy function in having a stateful setup function. Of course this can all be implemented as just a function in the tree, but it was designed to provide a sensible separation of the logical operations being performed on the node, whilst allowing patch to safely initialise and to safely destroy nodes. You can collapse functions into a single function with if statements in it. I don't think that it's inherently better.

With swapping this call around though, we do see that the destroy function is less powerful. Destroy really makes sense on the removal of something called with init. Persisting a widget over trees makes sense in this use case. Update should not have to call destroy on the previous, and the only reason I did so was because it was leaking references to DOM nodes. This has been solved by passing the node into the update function, so the previous widget should only contain application state or the previous vnode tree, never DOM nodes.

This does mean that destroy should be called with the node in question.

[Matt-Esch]

Example thunk implementation on the current widget implementation

https://gist.github.com/Matt-Esch/9867761

[neonstalwart]

i think we're getting closer... i'm going to play around with your latest changes to see if i have any more thoughts.

one of the reasons i'm preferring a single function over init and update is that at the moment, right.init() is never called - render calls init but patch calls update. i think that might cause a problem if the expectation is that init should have been called on an instance before any calls to update. when there is only one function, it should be less surprising if it's not called in "init" mode since either mode ("init" or "update") should be handled by the one function.

[Raynos]

I'm going to close this issue.

I think our widget implementation is acceptable for now.