WebReflection / hyperHTML

A Fast & Light Virtual DOM Alternative
ISC License
3.06k stars 112 forks source link

Compare function only compares actual DOM nodes #364

Closed atirip closed 5 years ago

atirip commented 5 years ago

There is option in the code to allow perhaps some day to pass different compare function to domdiff.

      var compare = options.compare || eqeq;

I needed it and I hacked it in to my forked version. Only to wire(), as I couldnt exactly wrap my head around how to add it to bind() and as I did not needed it for bind(), that's ok for me. Is there a plans to implement that in some day or should I perhaps create a PR?

This external compare function, as 3rd argument to wire() solves for me the following use cases:

        var props = {}
    var text = 'Just A text';

    hyperHTML.define('localized', function(str) {
        if ( str == 'Hello') return 'Ciao';
    });

    function render() {
        return hyperHTML.wire(props, ':page', {
                compare: function(a, b) {
                    if ( 'outerHTML' in a ) {
                        return a.outerHTML == b.outerHTML;
                    } else if ( 'textContent' in a ) {
                        return a.textContent == b.textContent;
                    } else {
                        return a == b;
                    }                       
                }
            })`
            <div>
                <!-- on default this updates on every call, while no changes -->
                <h1>${{localized: 'Hello'}}</h1>

                <!-- on default this updates on every call, while no changes -->
                ${{html: `<h1>HTML</h1>`}}

                <!-- this is not updated on every call as expected -->
                ${hyperHTML.wire(props, ':h1')`<h1>Wire</h1>`}

                <!-- this is not updated on every call -->
                <h1>${text}</h1>

                <!-- on default this updates on every call, while no changes -->
                <h1>${{text:'Object with text property'}}</h1>

                <!-- this updates on every call as expected -->
                <h2>It is ${new Date().toLocaleTimeString()}.</h2>
            </div>
        `;
    }

    hyperHTML.bind(document.body)`${render()}`;
    setInterval(render, 1000);  
WebReflection commented 5 years ago

the domdiff compare is not a boolean matter https://github.com/WebReflection/domdiff#a-node-generic-info--node-callback-for-complex-data

but I wonder what issue exactly you are trying to solve.

as it is presented now, this looks like a premature optimization for cases that are fast already on the DOM (i.e. setting the textContent of a node)

do you have data that supports your concerns?

atirip commented 5 years ago

First I tried to do localizer() as a define function and then when literally the whole page is text, it felt (I underline, felt, have not measured) too much updates. I do extreme rerendering and therefore I am pedantic, my application flow is I catch every input and change event and call render() after each (lets say I have huge form of various stuff). This works extremely well in hyperHTML, but only if I have all data in props ready. One can edit in textarea for example without noticing any delay, while the whole page is rerendered between keystrokes. If I need to pull some data with defined function, not so much. I changed that so I now pull all static data into static props when I create my views and forms. But It is kinda nuisance.

The second case and the case why I indeed did this is more unique. We have some of HTML stored as a strings, this is user generated content, that user include and edit (we use Quill for that, but it doesn't matter). The problem here is that everything is generated in hyperHTML and the text editor does its stuff there, which I know is strongly prohibited :-), but I just have no choice. So - I create DOM by passing html string to hyperHTML and then while user edits stuff and I need to call render, the same string is compared to the same, not with DOM, and the editors HTML is not messed with at all. After edit is done, I read the DOM, convert to string, pass to hyperHTML and rerender. The issue is with default compare function, I just cant't do that.

EDIT: Just read it again and my explanation could be messy - effectively in the last case I need to be able to rerender, but I also need to leave some DOM untouched by hyperHTML, so I use external comparator to effectively lie to domdiff.

WebReflection commented 5 years ago

why don't you use an attribute instead?

hyperHTML.define('raw-html', (node, value) => {
  if (node.innerHTML !== value) {
    node.innerHTML = value;
  }
  return '';
});

In that way you can pass raw-html as such:

wire(ref, id)`<div raw-html=${someHTML} />`;

and you'll have that div content injected again only if something changed.

wouldn't be this much easier for you?

exposing the domdiff compare requires a lot of effort in terms of:

and so on and so fort, so I don't think is a good idea in general, but I also don't have time for this, specially 'cause I think you don't need it at all.

exposing it might also cause troubles with the internal logic that expects comparison to be exactly what it is, which is never a boolean result, for example, like you showed before.

that's my first alert about not wanting to expose that part of the internal logic.

WebReflection commented 5 years ago

P.S. the same concept applies for text content

hyperHTML.define('raw-text', (node, value) => {
  if (node.textContent !== value) {
    node.textContent = value;
  }
  return '';
});
atirip commented 5 years ago

That attribute trick is pretty clever and it would be much easier indeed. You should document that. Thanks, I will try that as soon as I can (meaning that part works and I have other work to do, but if that works, I am changing the code).

WebReflection commented 5 years ago

It is documented: https://viperhtml.js.org/hyperhtml/documentation/#api-3

Screenshot from 2019-05-24 14-12-49

WebReflection commented 5 years ago

Closing, as there's apparently nothing else to do for me here.

atirip commented 4 years ago

Better late than never - that solution works wonderfully. Thank you.