default values for newChild and updateChild

[valdrinkoshi]

What about setting these default values for newChild and updateChild?

constructor() {
  super();
  this.newChild = () => document.createElement('div');
  // assuming we have itemsSource API
  this.updateChild = (child, item) => child.innerHTML = item.toString();
}

The user can still override these values to customize the scroller.

A default newChild helps especially those template libraries that need a container to render within, e.g. lit-html and preact. With lit-html, the user would just need to do this to update the nodes:

import {render, html} from './node_modules/lit-html/lib/lit-extended.js';
import './virtual-scroller-element.js';

const itemTemplate = (item) => html`item ${item.someproperty}`;
const updateChild = (child, item) => render(itemTemplate(item), child);

const scrollerTemplate = (totalItems) => html`
   <virtual-scroller updateChild=${updateChild} totalItems=${totalItems}>
   </virtual-scroller>`;
      
render(scrollerTemplate(1000), document.body);

[domenic]

I like the idea of having good defaults. In #81 we discussed another one for newChild, of using a light-DOM child <template> element. I'm unsure which is better. I guess the template seems nicer for people who are prepared to do some configuration of updateChild, whereas the createElement("div") version seems better for framework cases like you list, or for other consumers who really just want to handle everything inside updateChild. We could even do both, e.g. using the <template> if it's present and falling back to <div> if not? Maybe too magic...

For updateChild, I don't think we can get away with setting innerHTML as a default behavior, since that's just too XSS-prone. textContent might work fine though.

In general the less config we can mandate, the more <virtual-scroller> feels like a built-in element, and the happier I am. So the ideas in this thread make me very happy :)

[valdrinkoshi]

Yeah, it should be fine to search for a distributed template and fallback to a generic div. It would make sense to recycle by default to further optimize performance. Maybe something like this?

constructor() {
  super();
  // ...
  let childTemplate = null;
  const nodePool = [];
  this[_newChild] = () => {
    if (!childTemplate) {
      // Use distributed template or a generic div
      childTemplate = this.querySelector('template') || document.createElement('div');
    }
    return nodePool.pop() || childTemplate.cloneNode(true);
  };
  this[_updateChild] = (child, item) => child.textContent = item.toString();
  this[_recycleChild] = (child) => nodePool.push(child);
}

[domenic]

Sorry for the late response, but yes, that seems pretty great to me!