Add skip element to hook object AND allow root DOM with children in patch #363
Conversation
Allow the root DOM element to have children when used as original patch target: `patch(document.getElementById('container'), vtree)`. Without this patch the original child would be ignored and therefore duplicated if included in the `vtree`. This problem can also be overcome with `toVNode` but not always preferable to use it.
|
you can solve this today by doing: patch(toVNode(document.getElementById('container')), vtree)no changes needed. |
|
@caridy, do you see any problems adding a Some benefits from expanding the |
|
@bloodyKnuckles yes, that's part of #359 (which is implemented by #362) in which such ambiguity is not longer a problem since you're forced to always use |
Add as "skip flagged element" test. Proposing adding "skip" flag to `hook` object to prevent `patch` function from overwriting the corresponding flagged DOM element.
Add a "skip element" flag to vnode `hook` object to prevent `patch` from overwriting a DOM element identified by a flagged vnode, e.g., `h('div#skipme', {hook: {skip: true}})`, will prevent `patch` from patching `<div id="skipme">preserve</div>`.
bloodyKnuckles
changed the title
bloodyKnuckles
changed the title
|
I have updated my Snabbdom fork by adding a "skip element" flag to the hook object. This allows preparing of a particular DOM element prior to Snabbdom |
|
@bloodyKnuckles you don't need any of these to just skip the element. You can create a simple vnode without data and/or children, and a unique key, and snabbdom will not touch it. |
|
@caridy how does the key in the vnode correspond with the DOM element to be skipped? I'm using Snabbdom through CycleJS so access to the DOM element and Snabbdom are limited. For example, here's a code example: https://codesandbox.io/s/01qjkpwv5v How can the |
this is not really required! you can probably go without the key if there is no ambiguity with the siblings, that's all.
that's probably a question for @staltz. I'm just explaining how snabbdom works internally. If two vnodes are the same (via key, or selector), and there is no data, and no children specified in those vnodes, they modules will not attempt to mutate the element, which seems to be what you want. That means that you can have another process that makes changes to it (via a different patch routine, or just direct mutations on the DOM). |
|
@caridy I appreciate you looking at it. I think you've summed up my problem with "If two vnodes are the same...". The target DOM element is not the same as the new vnode. So my goal is to be able to skip patching an element regardless of vnode similarity. The simplest solution I can think of is a |
|
@bloodyKnuckles maybe I don't understand that. Snabbdom is very simple, you have two children collection, the oldVnode.children and the newVnode.children, and you have to compare them, independently of what is really in the DOM (snabbdom assumes that oldVnode.children is an accurate representation of what is in the DOM at the moment of the patching). now, if you want to maintain an element corresponding to a vnode from oldVnode.children, it must exists as a vnode in newVnode.children, and it must be distinguishable from the rest. attempting to just skip things from newVnode.children as if it wasn't part of the children collection in the first place can have undesirable results because the diffing algo may decide to add things before or after, so the final result is a non-deterministic order of childNodes. That's why we are suggesting to use the vnode identify in old and new vnode to produce the exact output that you want. |
Add more tests to core tests to determine the effect the skip flag has on node order.
@caridy I agree, this is important. I added 5 more tests that reorder, add, and remove nodes—all pass. I'm trying to think of more potential pitfalls. Can you think of any more situations that need testing? |
|
It is hard to tell because I haven't understand what are you trying to do. Those changes are definitely not something that we can do because it will impact the perf by adding those extra checks and string computations during the critical path, so there must be another way to do what you want/need, but someone has to understand those requirements so we can help you. |
|
yes, performance is another important consideration. Regarding the "skip element" flag, if there is no
The performance hit boils down to: How is Snabbdom performance tested? That'd be nice to know the performance hit these checks bring. Regarding purpose, simply put, I want to create a "black hole" in the VDOM that corresponds to a DOM element. The reason for this is to allow an element to be manipulated by a separate process other than the one that manipulates the DOM surrounding the "black hole". If I was only using Snabbdom |
|
@bloodyKnuckles I think what you want to achieve can be done with thunks (see the thunk docs in snabbdom). They basically exist to enable a hook into "patch". |
|
@staltz that was one of the first things that came to mind but didn't
figure out how. I can take another look.
|
|
@staltz The problem I have with For example, plain Snabbdom: https://codepen.io/anon/pen/WMxzQe?editors=1010 I need a way to tell Snabbdom to skip the element entirely, even on initial vtree render. I've also looked at the various hooks, init, prepatch, create, etc. but have not found a way with Snabbdom as-is to create this "black hole" in the VDOM, (other than |
|
@bloodyKnuckles You're getting close. In snabbdom, a patch is avoided when the next vnode is the same object as prev vnode. The initial patch, as you said, occurs by default because the prev vnode in this case is actually a so-called emptyVNode, and To fix that, you need to use toVNode so that the first comparison is not with var patch = snabbdom.init([]), h = snabbdom.h, p1;
var tovnode = tovnode.default
var thunk = snabbdom.thunk
var contentTree = tovnode(document.getElementById('container'))
var middleEl = document.getElementById('middle')
var vmiddle = tovnode(middleEl)
console.log(vmiddle);
var vtree = h('div#container', [
h('div#top', 'a'),
thunk('div#middle', (a) => vmiddle, ['donotwritethis']),
h('div#bottom', 'c'),
])
p1 = patch(contentTree, vtree)I tested this in the codesanbox and it worked. :) |
|
@staltz I have experimented with |
|
Curious about the performance hit, I ran this test with and without the proposed I ran the test 10 times, switched bundles, ran 10, switched bundles, ran 10, switched bundles, ran 10. And here's the results I got: And here's the test bundles DIFF: |
Allow the root DOM element to have children when used as original patch target:
patch(document.getElementById('container'), vtree). Without this patch the original child would be ignored and therefore duplicated if included in thevtree. This problem can also be overcome withtoVNodebut not always preferable to use it.