jorgebucaran / superfine

Absolutely minimal view layer for building web interfaces
https://git.io/super
MIT License
1.56k stars 78 forks source link

Changing patch parameter order, prepend issues and updating of mounted components. #149

Closed rbiggs closed 5 years ago

rbiggs commented 5 years ago

I made a branch of Superfine: https://github.com/rbiggs/superfine/tree/mount-improvement This addresses several issues.

  1. The order of parameters in patch Currently it is:
    patch(lastNode, nextNode, container)

    This changes it to:

    patch(nextNode, container, lastNode).

    What this accomplishes is the ability to just do this as a mount function:

    patch(nextNode, container) 

    You're already doing this in the README examples:

    const render = app(view, document.body)

    The README has the current example:

    const app = (view, container, node) => state => {
    node = patch(node, view(state), container)
    }

    Notice the difference between parameter order in the app function compared to the patch function. This type of juggling around parameters can lead to unintended parameter placement. With my changes this example becomes:

    const app = (view, container, node) => state => {
    node = patch(view(state), container, node)
    }

    Now both functions have the same parameter order. Nice.

I also made a change to how patch inserts a component into the DOM the first time. Since the last argument can be left out, I check if it is nullish:

if (!lastNode) {
  var element = createElement(nextNode, lifecycle)
  container.appendChild(element)
  nextNode.element = element
}

I create a new element and append it to the container. Then update the vnode nextNode by attaching the element reference to it. This will be used later for re-rendering. If the nextNode is truthy, then I use it to update the DOM. I do so not by relying on container.children[0], but by the element reference on lastNode:

else {
  patchElement(container, lastNode.element, lastNode, nextNode, lifecycle)
}

This accomplishes two things that have always bugged me about Superfine.

  1. patch will always append a component to the container instead of prepending as it currently does. This reduces the need for containers just to insert containers in the correct order. This makes it a lot easier to reuse html already in the document body or whatever. It's really unintuitive when Superfine prepends a component to existing markup. And no need to pass in null as a parameter value. Just leave it off at the end.
  2. Superfine doesn't have to depend on container.children any more in order to know what element in the DOM it needs to update. lastNode always has that reference anyways. This means you can render multiple components into the same container, and have them update without confusion.

I really prefer this behavior to the current behavior. Curious what everyone else thinks.

mindplay-dk commented 5 years ago

What this accomplishes is the ability to just do this as a mount function:

patch(nextNode, container) 

I'm not sure about the usefulness of this change?

Is it useful beyond the rare case where you're rendering something that's never going to get updated?

I guess server-side rendering is the only use-case I can think of - maybe it's meaningful for that. (?)

I'm not generally big on optional arguments (or any kind of function overload) and, in the case of patch, it seems to imply the operation can be performed without a prior state, which is sort of a "half truth" - for example, it would go very wrong if you omit the lastNode and try to patch a container with pre-existing content.

Having to explicitly say, "my last state is a null state" makes the operation more explicit - leaving this argument optional could imply it's possible to patch a container without a prior state, which isn't quite accurate, or at least omits an important assumption: that the target container is in a null state.

I know it's not very "Javascript" of me, but I generally prefer explicit, uniform call sites ;-)

rbiggs commented 5 years ago

Closing this now.