Matt-Esch / virtual-dom

A Virtual DOM and diffing algorithm
MIT License
11.67k stars 777 forks source link

Access to the vnode path from the hook #253

Open Gozala opened 9 years ago

Gozala commented 9 years ago

I've being wanting to store a unique address of each vnode on a actual dom node in order to be able to use it from an event listener. @Raynos said:

open an issue about wanting to the the vdom index in a hook

I'm not totally sure how I would be able to use index in a hook to compute a full path to a node, but I would really like to:

  1. Be able to compute such a path for individual vnode without having to walk the whole tree every time.
  2. Be able to compute such a path during vtree diffing vs patching as former will happen in the web worker while later will block a UI thread.
Matt-Esch commented 9 years ago

Each vnode contains the number of descendants it has. This way we can do fast walking of the tree whilst containing repeated subtrees. The main issue is ts that the vnode -> dom node mapping is one to many. You can have repeated nodes in the dom rendered more than once. We get around this by using descendant count (which remains unique on an immutable subtree).

When we are searching for the DOM nodes to patch, we use the sibling count to do a fast skip over the nodes that we are not interested in.So if I'm looking for index 20 I can skip the first child node if it contains 12 descendants, because 20 can't possibly lie in that subtree. I suggest you implement a search based on a fast descent given the DOM node index.

Aside from using this information, there aren't many other options beside tacking the data onto the dom node itself.

So 1. You don't have to walk the whole tree ever to find a node from an index.

  1. The node index is provided in the patch object itself. Perhaps this can be exposed if this is what you are interested in, but be sure to take into consideration how this breaks subtree re-rendering.

I'm interested in what kind of path you would like access to and where you would like it to be stored or derived given that it can't live on the vnode.

Gozala commented 9 years ago

Each vnode contains the number of descendants it has. This way we can do fast walking of the tree whilst containing repeated subtrees. The main issue is ts that the vnode -> dom node mapping is one to many. You can have repeated nodes in the dom rendered more than once. We get around this by using descendant count (which remains unique on an immutable subtree).

I do understand that specific subtree vnode instance may occur in several branches of the tree & given that vnode is immutable it's descedent count maybe used by a parent node to calculate number of it's own descedents.

When we are searching for the DOM nodes to patch, we use the sibling count to do a fast skip over the nodes that we are not interested in. So if I'm looking for index 20 I can skip the first child node if it contains 12 descendants, because 20 can't possibly lie in that subtree.

So if I understand this comment correctly this is how node tree is indexed ?

    0
   /\
  1  4
 /\   \
2  3   5

Still I'm not entirely sure where that node index information is stored, my guess is it not computed during diffing and stored in the patch, is it ?

I suggest you implement a search based on a fast descent given the DOM node index.

I am afraid I don't follow, what search are we talking about here ? Probably I have not explained problem precisely enough so we got little out of sync here.

Aside from using this information, there aren't many other options beside tacking the data onto the dom node itself.

Assuming I understand the indexing correctly, string that index on corresponding DOM node is exactly what I'm looking for.

So 1. You don't have to walk the whole tree ever to find a node from an index.

I see what you mean, if I have a vnode index I could find a corresponding vnode without traversing branches that do not have corresponding vnode. Never the less react style path (.0.1.1.0) still seems to have an advantage as it directly encodes index of child with in it's siblings and there for it is no longer necessary to walk lower indexed siblings to check number of descedents it has. Either way this is not a big deal for my use case.

  1. The node index is provided in the patch object itself. Perhaps this can be exposed if this is what you are interested in, but be sure to take into consideration how this breaks subtree re-rendering.

To be precise what I want is to store that index on a DOM node so I can read it back on DOM event in order to find corresponding vnode. Not sure I understand why would this break a subtree re-rendering though, sure two difffenent DOM nodes may point to the same vnode instance but that's fine. I do believe that all I really need that hooks were passed an index so it could be stored on DOM node. Or maybe other type of hook can be defined for that purpose.

I'm interested in what kind of path you would like access to and where you would like it to be stored or derived given that it can't live on the vnode.

To be specific I'm running application that generating virtual-dom in the web worker. diffs between generated trees are also calculated in the web worker and then patches are send to a main thread for rendering.

Now when DOM events like say click occur on DOM elements need to be serialized to be dispatched on the correspondig vnode that live in the web worker. In order to do that without walking up the DOM tree of the event.target to generate a node path I need to store an index / path on the DOM elements so that it can be simply serialized with an event and passed to web worker so it could seach for the corresponding vnode in the tree.

While I would rather have react like path a.k.a data-reactid for corresponding vnode discovery, described index would work just fine if I had a way of storing it on an actual DOM nodes.

Gozala commented 9 years ago

Another reason I'd prefer a path over an index is that tree in the DOM and worker may get out of sync there for index stored in DOM may no longer point to a vnode that corresponds to that DOM node. Since path a.k.a data-reactid encodes key of the node instead of index when node has one (like .$browser.$tabstrip.$tab-xxbsoteuth) it's still possible to find correspondig node to event.target even if trees are slightly out of sync (like children are reordered or previous child is removed).

In fact ideally each vnode would be hashed during diffing so that hash to vnode map could be updated from generated patches, which in turn would allow finding vnode corresponding to event.target node by a simple map lookup regardless of changes that may have happened to the tree.

While actually hashing would be too expensive I imagine virtual-dom diff could simply generate an id for each vnode and carry it across instances until there is a change in which case new id could be generated.

Matt-Esch commented 9 years ago

The vnode tree is not indexed. We can't index it because vnodes do not have a unique position in any given tree. They are to be reused an arbitrary number of times in an arbitrary number of trees.

The "path" is a quality of the DOM node. You can probably figure out a path for each node when it is created, assuming that a path is just "parent DOM node index + child index". But I believe you were trying to avoid this computation?

For the purpose of patching, the DOM is indexed using the pre-order index you describe.

   0
   /\
  1  4
 /\   \
2  3   5

To find VNode of DOM index 5 we should look a the VNode structure:

   5
   /\
  2  1
 /\   \
0  0   0

Each VNode simply stores how many descendants it has. To find 5, we recursively descend the tree applying the algorithm:

if currentIndex + descendantCount < searchIndex ? 
    add descendant count to index and try next sibling : 
    add 1 to the index and descend down the current node

So in the example case we check the first child of the root (where our index is 1) and we add the sibling count (2) to see that the max node index in that subtree is 3, so we skip over to the next sibling at index 4. We see that 4 + 1 is within range and descend down that tree, finding index 5.


But ok, the index isn't satisfying enough.

Unfortunately hooks don't have access to the vnode right now so it doesn't really work, but we could modify the code so that the vnode is passed in as an argument to the hook. You could then do something like:

'use strict';

var cuid = require('cuid');
var defaultKey = 'some-key-on-dom-node';

module.exports = IndexHook;

function IndexHook(indexObject, domKey) {
    if (!(this instanceof IndexHook)) {
        return new IndexHook(indexObject, domKey);
    }

    this.vnodeIndex = indexObject || {};
    this.domKey = domKey || defaultKey;
}

IndexHook.prototype.hook = function hookIndex(node, propertyName, prev, vnode) {
    var index = node[this.domKey] || cuid();
    this.vnodeIndex[index] = vnode;
    node[this.domKey] = index;
};

IndexHook.prototype.unhook = function unhookIndex(node, propertyName) {
    var index = node[this.domKey];
    if (index) {
        delete nodeIndex[index];
        node[this.domKey] = null;
    }
};

Which means you are creating a vnode index using pseudo uuids (I chose cuid for speed) and putting that uuid on the DOM node so you can access it later. Then you can fetch the vnode from the index when the event is triggered, and this should not leak.

Of course it would be better if your event hook transparently created this mapping instead of having to add a separate index hook, but I think this works for illustration purposes.

Gozala commented 9 years ago

The vnode tree is not indexed. We can't index it because vnodes do not have a unique position in any given tree. They are to be reused an arbitrary number of times in an arbitrary number of trees.

Sorry for confusion by indexed I meant the index that is generated for the purpose of patching.

So in the example case we check the first child of the root (where our index is 1) and we add the sibling count (2) to see that the max node index in that subtree is 3, so we skip over to the next sibling at index 4. We see that 4 + 1 is within range and descend down that tree, finding index 5.

Yes that was also my understanding of how seach would worked.

But ok, the index isn't satisfying enough.

Index isn't ideal, but it colud be used to solve the main problem I'm having. Maybe we could expose index first and see if something better colud be exposed in separate issue ?

Unfortunately hooks don't have access to the vnode right now so it doesn't really work, but we could modify the code so that the vnode is passed in as an argument to the hook. You could then do something like:

Proposed solution is not going to work for me actually, because in my case vnodes live in worker thread while dom nodes live in the UI thread. Something like this would do though, where passed index is an index generated for patches.

class AddressHook {
  constructor() {
    this.addressBook = Object.create(null)
  }
  hook(node, propertyName, previous, index) {
    const address = node.dataset.address || ++AddressHook.id;
    this.addressBook[address] = index
  }
  unhook(node, propertyName) {
    const address = node.dataset.address
    if (address) {
      delete this.addressBook[address]
    }
  }
}
AddressHook.id = 0

This way in the main thread when event happens I can lookup an index of the corresponding vnode and forward that and some event data to the web worker, where vnode will be resolved by index as you described earlier.

In fact this brings up another issue for which I have submitted #261 at the moment I wrap vnode constructor so that I could serialize observed event listeners via plain attributes like data-listener-click: true while all events are registered separately on main thread and data-listener-* attributes are used to find a relevant event target whose dom path is built and send to web worker so that corresponding listener could be invoked. I would much rather use hooks than do this such an ad-hock way.