jorgebucaran commented 6 years ago

Summary

The result is a large Double Espresso Steamed Soy Milk Foam!

<body>
  <div id="app"><h1>This heading element will be recycled by default!</h1></div>
</body>

import { h, patch } from "picodom"

const view = state => (
  <div>
    <h1>{state}</h1>
    <button onclick={() => patch(view(state - 1), root)}>-</button>
    <button onclick={() => patch(view(state + 1), root)}>+</button>
  </div>
)

const root = patch(view(0), document.getElementById("app"))

The DOM element passed to patch represents the root of your application, not a container. The function returns the element and HTML that results from this operation is the same HTML-like markup returned by the view.

<body>
  <div>
    <h1>0</h1>
    <button>-</button>
    <button>+</button>
  </div>
</body>

In addition, Picodom now supports hydration of pre-existing DOM content. This means that instead of throwing away any server-side rendered markup, Picodom will try reuse those elements. For this to work, the markup must be the same markup produced in your view.

2018-02-17 00 18 34

// The delay simulate some SSR HTML + HTTP request.
let root

setTimeout(() => {
  root = patch(view(0), document.getElementById("app"))
}, 3000)

Notice in the GIF how the heading fades into the page, but when it's replaced by the counter, it's value is simply changed. This means that Picodom reused that heading, instead of creating a new one from scratch and only updated the element's nodeValue. Notice how the +/- buttons are animated, however, since their markup was not included (on purpose in order to demonstrate this feature) in the pre-existing HTML.

If you don't flag nodes you created yourself, you can't know which DOM state is relevant to an update - this is what I was trying to address in my implementation by injecting the last known VDom state into the DOM, in the hopes that this would also allow for deferred removals, which I also don't think Picodom 2 will presently? If another update is triggered while a not-yet-removed DOM node is still in the DOM, this would be seen as the current state, and it would either add/remove/move the DOM node, which really should be left where it is, would it not?

My hope with my own implementation was that it would only touch DOM nodes that it was either told directly to touch (the top level target node) or had itself flagged during a previous update so it could recognize the node as being a "managed" node. The idea was that DOM nodes lingering during deferred removals, as well as DOM nodes created and injected during oncreate would be unaffected.

Note that Preact's top-level render function accepts a third argument, the replaceNode flag, which allows you to force it to replace an entire subtree, as you would if the document had already been rendered from the server-side. I understand you're trying to optimize by recycling the DOM nodes that were created already when the document was loaded, but I think you'd have to at least internally know whether you're forcing the DOM into a specific state, or patching existing managed DOM nodes, as it makes a difference when you have unmanaged nodes that shouldn't be touched.

jorgebucaran commented 6 years ago

@mindplay-dk But it will then necessarily also destroy foreign nodes created during oncreate...

You can easily avoid foreign nodes being destroyed if you plan your markup more carefully. Sorry, but your example was only set up to fail.

let update = model => (
  <div>
    <h4>Hello Here</h4>
    <div>
      <img
        src="http://farm4.staticflickr.com/3095/3502308202_80420dda73_z.jpg"
        oncreate={img => new Cropper(img)}
      />
    </div>
    {model.there && <h4>Hello There</h4>}
  </div>
)

@mindplay-dk I understand you're trying to optimize by recycling the DOM nodes that were created already when the document was loaded...

Hydration is a one-time-only optimization. If you want to completely opt-out call patch without a DOM element. By doing so, a new element will be created and returned to you.

const element = patch(<h1>Hello</h1>)
// 
// ...
//
document.body.appendChild(element)

...hopes that this would also allow for deferred removals ...

I am not trying to solve this problem at the moment, but I may try when I actually rewrite the entire diff algo (in time for Hyperapp 2.0).

mindplay-dk commented 6 years ago

You can easily avoid foreign nodes being destroyed if you plan your markup more carefully

You mean wrap things in extra elements, which is what I mean when I say that HTML support should be universal - I don't want the framework to dictate the kind of markup I can/may use.

I don't think I'm trying to make things unnecessarily difficult here. I just want to be sure that, when my HTML/CSS guy delivers a finished UI design, I can move it into JSX and build a render-function easily, without first having to "fix" the markup and adjust the CSS - and possibly break it, and/or break things for other developers working on other parts of the UI that depend on the same CSS.

If you want to completely opt-out call patch without a DOM element. By doing so, a new element will be created and returned to you.

😄👍

jorgebucaran commented 6 years ago

@mindplay-dk You mean wrap things in extra elements, which is what I mean when I say that HTML support should be universal - I don't want the framework to dictate the kind of markup I can/may use.

How does Preact solve this then? It doesn't. I am happy with the current behavior.

dancespiele commented 6 years ago

Remember to change this in the index.d.ts file before to merge the pull request

export function patch(
  parent: Element,
  oldNode: VNode | null,
  newNode: VNode
): Element

to

export function patch(
  node: VNode,
  parent: Element | null
): Element

mindplay-dk commented 6 years ago

Note that the second argument is no longer the parent, it's the target element:

export function patch(
  node: VNode,
  target: Element | null
): Element

codecov-io commented 6 years ago

Codecov Report

Merging #96 into master will decrease coverage by 66.66%. The diff coverage is 33.33%.

@@             Coverage Diff             @@
##           master      #96       +/-   ##
===========================================
- Coverage     100%   33.33%   -66.67%     
===========================================
  Files           2        1        -1     
  Lines         119      117        -2     
  Branches       38       36        -2     
===========================================
- Hits          119       39       -80     
- Misses          0       57       +57     
- Partials        0       21       +21

Impacted Files	Coverage Δ
src/index.js	`33.33% <33.33%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update db16211...b40b5d6. Read the comment docs.

jorgebucaran / superfine

→ Picodom 2.0 → Patchdom #96

Summary

See Also

Codecov Report