Add ability to specify a function that determines the key for a VNode

[ghost]

We use some attributes to denote our elements, something such as a data-uuid="4423424" for all of our elements. Adding the ability to specify the key on the node allows the patches to detect moves instead of just diffs. That is useful in our case as in addition to just doing the diffing we want to inspect the patches to highlight the changes.

[TimBeyer]

First of all, thank you @dariusriggins for this PR. This could be really useful indeed.

I do have my concerns however regarding the proposed implementation. Currently you'd be leaking the tag object from htmlparser2. That would mean that once people start using this feature, html-to-vdom would forever be coupled to how htmlparser2 represents the tag.

I was thinking about how to fix it, and one thing that would definitely be possible is exposing the attributes object after conversion to the format VNode expects, since this library and VNode are already tightly coupled.

Another thing is that I have the feeling the callback should be specified after the library initialization, maybe as an additional argument to convertHTML, but even there it would look a little bit awkward.

Maybe it could work like this

var htmlToVDom = require('html-to-vdom')({
    VNode: VNode,
    VText: VText
});
var convertHTML = htmlToVDom({
    getVNodeKey: function (attributes) {
        return attributes.id;
    }
});

or

var convertHTML = require('html-to-vdom')({
    VNode: VNode,
    VText: VText
});

// partial application
var convertHTMLWithKey = convertHTML.bind(null, {
    getVNodeKey: function (attributes) {
        return attributes.id;
    }
});

var withKey = convertHTMLWithKey('<div id="foo"></div>');
var alsoWithKey = convertHTML({
    getVNodeKey: function (attributes) {
        return attributes.id;
    }
}, '<div id="foo"></div>');

var withoutKey = convertHTML('<div id="foo"></div>');

[ghost]

Thanks for the response. I think the second option you specified, passing it in as an argument of converHTML makes the most sense and offers a good deal of flexibility. Being able to flip the arguments to allow binding also makes it great for my case and general re-use as well. If you have no issues with it I'll go ahead and correct the PR to match that syntax.

[TimBeyer]

Sounds good to me. We'll also keep backwards compatibility that way and people won't have to touch their imports if they want to use it, or create a new converter function several times if they want to sometimes use it, sometimes not.

[ghost]

Updated.

[TimBeyer]

Let's merge it then! :)

[TimBeyer]

Published as v0.5.0

[TimBeyer]

Now I am wondering if we should have exposed the tag name as well.

[ghost]

Awesome, thanks. Yeah that may of been useful, I was thinking the same but for my personal use case I certainly don't need it and couldn't think of a situation I'd want the tag name over an id or some other value.

[TimBeyer]

@dariusriggins what's your use case if I may ask? I'm kinda curious what people use my library for. :)

[ghost]

Making a visual summary of the changes from one diff to the other. Given two HTML strings, how can we visualize (and eventually patch diffs) between the two. This change was key because I need to show an item moving instead of just being updated.