jorgebucaran / superfine

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

2.0 API #94

Closed jorgebucaran closed 6 years ago

jorgebucaran commented 6 years ago

Picodom@2.0.0 is coming to you very soon. The signature of patch needs to change, and I need your feedback to get it right.

Closes

Bitter Tea

Bring back the old pre-1.0 signature. This API is more flexible and easier to implement at the expense of a slightly worse developer experience (is it?). The justification is that Picodom is intended to be used to create tiny Hyperapp-like view frameworks instead of directly consumed. Still, this is IMO much better than similar popular alternatives.

import { h, patch } from "picodom"

// First and every other patch.
let element = patch(
  container,
  element,
  oldNode,
  newNode
)

Double Espresso Steamed Soy Milk Foam

This proposal arguably improves the surface API providing at the expense of "slightly" less flexibility. This also has out of the box SSR DOM rehydration. This API makes consuming picodom directly more fun, but a bit awkward to integrate into your own library because you need to be aware there would be essentially two ways to call patch, the first time you call it, and every other time.

SSR recycling is enabled by providing a recycleElement to patch that works the same way as https://github.com/hyperapp/hyperapp/pull/590.

import { h, patch } from "picodom"

// First patch.
let element = patch(vnode, containerElement)

// or first patch with server side rendered rehydration

let element = patch(vnode, containerElement, recycleElement)

// Every other patch.
patch(vnode, element)

/cc @mindplay-dk @JSteunou @estrattonbailey @pspeter3 @maxholman @dancespiele

pspeter3 commented 6 years ago

I wonder if it is worth making initial patch and update patch two different methods. Right now the name could be confusing.

jorgebucaran commented 6 years ago

@pspeter3 That might be a way to make "Double Espresso Steamed Soy Milk Foam" easier to document. But I am swaying in favor of "Bitter Tea" now.

estrattonbailey commented 6 years ago

Yeah, I personally agree: the API doesn't need to be overly concerned with developer experience.

When I was poking around a month or so ago, the main thing I thought would be helpful was a way to patch sub-trees so that state updates could be localized and not have to walk the entire tree. Would this be possible given the Bitter Tea approach?

jorgebucaran commented 6 years ago

@estrattonbailey What does patching a sub-tree entail? Replacing/updating elements instead of prepending/appending to a container? If so, then yes, I guess. Do you have some pseudocode?

mindplay-dk commented 6 years ago

Did you ever look at this? I've given up and have settled for Preact, but I'm positive this rewrite was headed in the right direction - I just can't dedicate the time to complete it. For one, this is fully type-hinted, so it should be much easier to work with - the test-suite is also much faster, fewer dependencies, type-hinted test-suite, inline comments, and so on.

Bring back the old pre-1.0 signature. This API is more flexible and easier to implement at the expense of a slightly worse developer experience (is it?). The justification is that Picodom is intended to be used to create tiny Preact/Hyperapp-like view frameworks instead of directly consumed.

IMO, you're bringing back at least two major problems.

First, I don't believe it's actually possible to build something Preact-like - e.g. something that supports constructors and stateful components, because the only way that happens, is if the framework is in charge of creating/destroying component instances, the same way it manages the life-cycle of DOM elements. It has to be deeply integrated. At least that's the conclusion I came to.

Secondly, the long-debated problem of deferred removals - the signature, where someone has to pass in the old state and new state, makes it very difficult "cheat" on this part, e.g. somehow "faking" a prior VDom node state of something that has already been removed. Since you need the past information to make this happen, it's much easier and simpler to start with a design where you manage the VDom state internally: don't require someone to pass in the old and new state. There are only two things you can do with that feature of the API: either correctly or incorrectly pass in the prior state - you can limit that to "correctly" by simply maintaining that state internally, and I wouldn't recommend injecting it into the VDom model (like e.g. Snabbdom) so that leaves either the DOM, or a state object of some sort, perhaps one that gets created by a mount() function; or something like jQuery's .data()... either way, putting that on the consumer of the API is asking for trouble.

Perhaps most importantly, look at the internal signature of my patch-function:

function patchTarget(
    target: TargetNode | undefined,
    parent: Element | undefined,
    nextSibling: Node | undefined,
    vNode: VChildNode,
    callbacks: Function[]): Node)

Yes, that's a lot of arguments - but they're all extremely necessary: you want to be able to either patch an existing DOM node, or have the framework create the correct node for you and position that node correctly, so there's no way around it, you need at least the target element (which may be undefined), as well as it's parent and it's next sibling for positioning. You can't have fewer arguments than that, or it will be impossible to position a created element.

It might even make sense to make the public function very similar to this, which would provide the most flexibility - e.g. same signature minus the callbacks which are an internal detail.

I can answer any question you might have about that code, I just can't dedicate the time to complete it myself. But I think, if you just start over, you're going to end up wasting a lot of time on issues I already spent wayyyy to much time on. Totally up to you. Just trying to help :-)

jorgebucaran commented 6 years ago

My diff/patch rewrite will fix the deferred problem, this proposal is just about API. And I think you are right, I was not trying to build something like Preact here, this is just Hyperapp's VDOM stripped so that people can play with it, customize or depend on to build their own tiny "functional" view frameworks. I think you always wanted to build something else, unfortunately. I've updated the original issue.

jorgebucaran commented 6 years ago

@mindplay-dk You can't have fewer arguments than that, or it will be impossible to position a created element.

Haha, sure I can. Fingers crossed. 🤞

mindplay-dk commented 6 years ago

My diff/patch rewrite will fix the deferred problem

I concluded it wasn't possible, so I really look forward to seeing your solution ;-)

I think you always wanted to build something else, unfortunately.

I don't think so? You just said yourself above that you wanted people to be able to build something like Preact with this - it isn't possible without some means of maintaining state.

jorgebucaran commented 6 years ago

Yes, but I definitely had something more like Hyperapp in mind. I updated the original post to reflect this.

estrattonbailey commented 6 years ago

Replacing/updating elements instead of prepending/appending to a container?

Yep! I just wanted to contain state to a component and update the component and it's children starting from its position in the DOM, instead of having to traverse the entire tree from the top. I made a demo using my fork a while back.

I'm not sure if it would be compatible with your rewrite, but that's okay! AFAICT that's the main blocker to stateful components.

rbiggs commented 6 years ago

I'm not sure why some of you think a return to the older call signature would make Preact-style stateful components impossible. I used the old call signature in version 0.2.0 to do just that. That call signature was:

function(oldNode, node, element, parent, cb)

Inside my component this becomes:

// Patch DOM with component update.
this.element = patch(
  this.oldNode,
  (this.oldNode = this.render(__data)),
  this.element,
  this.container
)

My component class keeps track of oldNode, element and parent (container). node is created when the component render function is called, passing in the new state. Because the component instance keeps track of oldNode, element and parent, it passes these to patch on each update. This means, like Preact, you can create multiple stateful components in the same parent (container). Picodom updates the correct tree for that component because each component has a reference to that tree thru the element property that it gets from patch. This is not the case when patch assumes the tree to update is always parent.children[0]. Here's a Codepen example of what I'm talking about: https://codepen.io/rbiggs/pen/ppZRWE?editors=0010

After the call signature was changed, I was never able to replicate this behavior. I tried many times. The newer call signature works fine for functional components, as long as you don't need to change the scope in which they're rendered or have only one in its own parent. With the old call signature, the component instance always has a reference to its oldNode, element and parent. This means you can define and instantiate them in one file, export them and import them into another file, where, when state changes, they will update as expected. With the simplified call signature there's no proper reference to these and so Picodom with the newer call signature winds up either recreating the entire tree from scratch, or creates a totally new tree prepended to the parent.

By the way, here's the Preact version of my Picodom multiple list example, same markup, data, etc.: https://codepen.io/rbiggs/pen/BYdQGa?editors=1111

So, I think the call signature change proposal for 2.0 is good.

By the way, I don't think anyone should try to recreate Preact with Picodom other than for fun and learning. After all, Preact is only 3KB. By the time you add everything to make Picodom work exactly like Preact, you'll probably have something much larger. In my example I made something that was Preactish/Reactish without completely replicating the paradigm. My component classes are real classes that you have to, gasp!, instantiate with the new keyword. But that's what I wanted, so I would have a reference to the component instance that I could pass around, export, import or whatever.

Maybe I'm completely misunderstanding what everyone is saying here and I need to down a couple of cups of double espresso soy milk foams. ☕️☕️

jorgebucaran commented 6 years ago

This proposal only affects how we mount stuff to the DOM, it doesn't (shouldn't) affect whether you can make stateful components using Picodom or not, or am I wrong?

@estrattonbailey I just wanted to contain state to a component and update the component and it's children starting from its position in the DOM...

Hmm, and where did you want to contain the state?

mindplay-dk commented 6 years ago

I don't think anyone should try to recreate Preact with Picodom other than for fun and learning. After all, Preact is only 3KB. By the time you add everything to make Picodom work exactly like Preact, you'll probably have something much larger.

For the record, neither am I.

The main reason I took an interest in Picodom, was because of element-level life-cycle hooks, which seems much simpler and more elegant than having to write classes to achieve this control.

For this to work out in practice though, I concluded, you'd need some kind of "transparent" element - something like DocumentFragment, which represents a subtree with no common ancestor, for scenarios when you need to control the life-cycle of multiple elements that don't have a common parent. Preact has Component to address the need for life-cycle events for subtrees. JSX recently added the <> empty element, which might be a possible approach, though I'm not sure if it's possible to add attributes to an empty element? < oncreate={...}>. Hmm. Probably not.

The other major problem is the lacking ability to abort rendering of a subtree - if you have 10 date-pickers with hundreds of elements, you'll need a hook that lets you check if the value has changed, and skip rendering the entire sub-tree, without losing that subtree. Preact has shouldComponentUpdate. If this were a thing in Picodom, likely it should be implemented as another element-level life-cycle hook, in order to stay the course and maximize consistency.

These are the two hard problems that a library needs to solve, in my opinion: rendering (with full control, esp. the ability to abort) and reuse (some kind of stateful component life-cycle.)

jameswilddev commented 6 years ago

In the case of the calendar example, it feels to me that the calendar should be its own VDOM as it is re-rendered at a different time to its surroundings.

If you can (or could?) include a raw DOM element in your VDOM, this would be quite easy to do from the API consumer end.

jorgebucaran commented 6 years ago

If this were a thing in Picodom, likely it should be implemented as another element-level life-cycle hook, in order to stay the course and maximize consistency.

We don't need one in Hyperapp as this can be solved in userland. Same for Picodom. We don't need shouldComponentUpdate, because we don't have stateful (class & this) components. But of course, this was not clear when I said: "use Picodom to build your own React-like view framework".

mindplay-dk commented 6 years ago

We don't need one in Hyperapp as this can be solved in userland

I have seen too many wonky patterns to rely on userland to get this right.

I'm of course fine with a library not handling this - that's a matter of sticking to a defined scope, but if you expect a framework to be able to do this, the library must make that possible.

If it doesn't, then, yeah, you were probably right, maybe what I wanted was always something different, I just didn't know it yet ;-)

rbiggs commented 6 years ago

Just to clarify, the new JSX syntax <></> is not an empty element. It's just a shorthand convenience for using <React.Fragment></React.Fragment>, which in turn creates a documentFragment. Since documentFragments only exist in memory and are thrown away when they are inserted into the DOM, they don't have properties. They do have a few attributes, such as childNodes, etc., and methods such as appendChild.

You can't have a documentFragment that would fire an oncreate since they never get created in the DOM. Same problem with onupdate, etc. In React, lifecycle hooks are defined at the component level, not on the elements, so this isn't a problem for them.

Introducing support for documentFragment in React was not trivial. It took a lot of work from their team. Both Preact and Inferno are looking at supporting it but have been delaying it due to the amount of rewriting of the patch and render algos. Mithril does now currently support this. Personally I don't think this is worth implementing in Hyperapp and Picodom since it would require quite a bit of extra code for a trivial feature that we can live without. My opinion. The React crowd got along fine without it for years. I understand the support for it in Babel and Buble is still kind of flaky.

Maybe after other micro-libraries have managed to implement support for it you might want to take a look at how they did it.

jorgebucaran commented 6 years ago

@rbiggs You nailed it! 🙇 👍

rbiggs commented 6 years ago

The more a look at it, the more I like the Double Espresso Steamed Soy Milk Foam proposal. Funny how that just rolls off the tongue. I like the support for SSR too. It basically gives those who want them a mount and a render/update. They can easily write a wrapper function for each. Besides, I've never been a big fan of bitter tea. A double espresso, no problem.

mindplay-dk commented 6 years ago

Just to clarify, the new JSX syntax <></> is not an empty element. It's just a shorthand convenience for using <React.Fragment></React.Fragment>, which in turn creates a documentFragment

You're describing React - not JSX, which merely defines it as a zero-or-more list of children. The JSX spec itself defines the syntax only, not the meaning.

You can't have a documentFragment that would fire an oncreate since they never get created in the DOM. Same problem with onupdate, etc. In React, lifecycle hooks are defined at the component level, not on the elements, so this isn't a problem for them.

That's what I was trying to get at. React provides a component concept, which provides a means of controlling the life-cycle of zero-or-more children. We currently don't have something like that in Picodom, and I personally found this to be quite problematic, as there are common use-cases both for entire components that toggle on and off, which requires support for zero elements; as well as for components that require more than one element, where, presently, you need to introduce a useless wrapper <div> to get the life-cycle events.

Clearly JSX fragements are not the way to solve this problem - not if we want to consistently have element-level life-cycle-events and avoid the introduction of a component concept.

The other option then is something like <React.Fragment>, a document fragment with a life-cycle.

It requires a different design for sure, but it's not necessarily substantially more complex - it's a matter of coming up with a design that uses a homogeneous model, which of course isn't easy. React wasn't able to - components and elements are something very different. That's what attracted me to Picodom - the idea of a single concept that is general enough to support all use-cases.

We didn't get there in the first attempt. In order to truly map from the JSX model to the HTML model, we have to support zero and multiple children, consistently, everywhere - because HTML does. I think that means at least some kind of fragment element with life-cycle events is required, since that's the only way to group more than one node with JSX syntax. (other than e.g. inline arrays, which look terrible.)

A concrete use-case I ran into on friday, was a color sample swatch that optionally displays a darker and lighter version of the color sample - the only way to accomplish that, was to duplicate the test:

const Swatch = (color, variations) => (
    <div class={`swatch--${color}`}>
        <span class="swatch--sample"></span>
        {variations && <span class="swatch--darker-sample"></span>}
        {variations && <span class="swatch--lighter-sample"></span>}
    </div>
);

Here, a color swatch has one sample unless variations={true}, otherwise it has three.

An extra wrapper element around the optional two darker/lighter samples is useless - it's generally unacceptable that the framework or syntax should dictate the markup you use; you should be able to use the markup you need.

With document fragments, you can do this:

const Swatch = (color, variations) => (
    <div class={`swatch--${color}`}>
        <span class="swatch--sample"></span>
        {variations && <>
            <span class="swatch--darker-sample"></span>
            <span class="swatch--lighter-sample"></span>
        </>}
    </div>
);

If the color swatches all had to float left, you might not even want a wrapper div around that component at all:

const Swatch = (color, variations) => (
    <>
        <span class={`swatch--sample color-${color}`}></span>
        {variations && <>
            <span class="swatch--darker-sample"></span>
            <span class="swatch--lighter-sample"></span>
        </>}
    </>
);

If this component had a life-cycle, you could use Component and it would provide the life-cycle events for that fragment.

Now, React/Preact is far from ideal for me, on account of having two completely different concepts of elements and components, but it works - Picodom, presently, dictates the markup I have to use, like adding extra wrappers, etc.

In my opinion, a good HTML library must work equally well for all types of markup, which is why I've resigned myself to using Preact for the time being.

I'd love to see Picodom provide a markup-agnostic, homogeneous model with element-level life-cycle events, but I don't see any way of doing that without some kind of fragment concept to deal with cases of zero or multiple elements with no common ancestor?

jorgebucaran commented 6 years ago

You can return an array from a component, what's your point?

BTW I am closing here since I am ready to merge #96.

jorgebucaran commented 6 years ago

@mindplay-dk IIRC Preact components can return arrays in the same way Hyperapp components can, but there's no fragment support like in React.