choojs / nanocomponent

🚃 - create performant HTML components
https://leaflet.choo.io/
MIT License
366 stars 30 forks source link

Using `placeholder` breaks `onupdate` - unable to update and modify DOM element. #18

Closed eugeneware closed 7 years ago

eugeneware commented 7 years ago

The use of a placeholder because it uses nanomorph under the hood, means that the element that gets passed to the first argument f onupdate is actually NOT the one that gets inserted into the document and returned by the first render. Thus, any attempt to modify that element has no impact on the actual DOM element.

The root cause is that due to using polite-element the actual element that gets inserted is the placeholder element. Then the rendered element is used to diff and update the placeholder element. Thus the placeholder DOM element is the one that needs to be returned and provided to the onupdate element and any other place that the element is passed in.

Work around: Cache the element that gets passed into onload (this seems to be the right one) and use this in onupdate, or don't use a placeholder element.

Here's a test case that shows the behaviour.

var component = require('nanocomponent');
var html = require('bel');
var assert = require('assert');

var el;
var comp = component({
  onupdate: function (e, name) {
    // This e refers to the element that was used by nanomorph in polite-component to diff
    // and update the placeholder element
    e.textContent = 'Updated, ' + name;

    // This line will work and update the element:
    // el.textContent = 'Updated, ' + name;

    // This will throw, as the elements are not equal
    assert.equal(e, el, 'Element should be equal');
  },
  placeholder: function (name) {
    return html`
      <div>Placeholder, ${name}</div>
    `;
  },
  render: function render(name) {
    return html`
      <div>Hello, ${name}</div>
    `
  }
})

// Render "Placeholder, Mike" and then "Hello, Mike"
el = comp('Mike');

// Try to set the component to 'Updated, sarah' after 1 second
setTimeout(() => {
  comp('Sarah');
}, 1000);

document.body.appendChild(el);
yoshuawuyts commented 7 years ago

nanocomponent v3.0.0 is out - don't think this issue is relevant anymore. Also released https://github.com/yoshuawuyts/microcomponent for a more high level take on the API of nanocomponent

yoshuawuyts commented 7 years ago

@eugeneware thanks heaps for reporting though! :grin: - a refactor was due to solve issues like the one you were running into

eugeneware commented 7 years ago

Cool. Will check them out!