connected/disconnected events cannot dispatch to nested components

[liming]

This problem happened when the the parent of nested components has already bind connected/disconnected events.

class ChildComponent extends hyper.Component
{
  onconnected(e) {
  }

  render() {
    return this.html`
      <div onconnected=${this}
         "I am a child"
      </div>
    `;
  }
}

class ParentComponent extends hyper.Component
{
  onconnected(e) {
  }

  render(){
    return this.html`
      <div onconnected=${this}
         ${new ChildComponent()}
      </div>
    `;
  }
}

In this circumstance, the child component won't receive the onconnected event.

The cause of this is this piece of code. The parent component dispatches onconnect and skips the dispatch of child component.

[WebReflection]

are you suggesting the order should be inverted ? good catch btw

[WebReflection]

btw, beside the possible typo in the example, the non closing <div> opening tag, I'd like to highlight a little (unfortunately common) anti pattern there.

class ParentComponent extends hyper.Component
{
  render(){
    return this.html`
      <div>
         ${
           /* a new component every single render */
           new ChildComponent()
         }
      </div>
    `;
  }
}

If the parent gets its render method invoked twice, you'll have two ChildComponent and one of them probably lost somewhere.

As you can see in this Code Pen, you can find ways to setup your instances once instead:

const {Component} = hyperHTML;

class ChildComponent extends Component {
  constructor() {
    super().times = 0;
  }
  render() {
    return this.html`<p>child rendered ${++this.times}</p>`;
  }
}

class ParentComponent extends Component {
  get hyper() {
    // setup some property
    this.writable = 123;
    // or setup via descriptors ...
    return Object.defineProperties(this, {
      // ... more own properties
      childComponent: {value: new ChildComponent},
      // ... and then ...
      // shadow hyper getter for the next render
      hyper: {get() { return this.html; }}
    }).html;
  }
  render() {
    // childComponent is always same one => force its rendering
    return this.hyper`<div>${this.childComponent.render()}</div>`;
  }
}

const parent = new ParentComponent;
const render = hyperHTML(document.body);

// parent is always same one => force its rendering
setInterval(() => render`${parent.render()}`, 1000);

[liming]

Hi @WebReflection,

The example code I write is just dummy code. What I really use is something like this:

import {Component} from 'hyperhtml/esm';
import diff from 'deep-diff';

class BaseComponent extends Component {
  constructor(props) {
    super(props);

    this.props = props;

    this.children = new WeakMap();

    this.innerState = this.toState(props);
  }

  get defaultState() {
    return this.innerState;
  }

  apply(props) {
    // convert the props to states
    const state = this.toState(props);

    // diff the original state and the new state
    const changes = diff.diff(this.innerState, state);

    if (changes) {
      // apply the change to the state object. This would keep the state as the same
      changes.forEach(c => diff.applyChange(this.innerState, state, c));
    }

    this.setState(this.innerState);
  }

  /**
   * toState is the method you can map the properties to inner state
   */
  toState(props) {
    return typeof props === 'function' ? props : Object.assign({}, props);
  }

  child(ChildComponent, identity, props_) {
    let childComponent = this.children.get(identity);

    const props = props_ ? props_ : identity;

    if (!childComponent) {
      childComponent = new ChildComponent(props);

      this.children.set(identity, childComponent);
    } else {
      childComponent.update(props);
    }

    return childComponent;
  }

  /**
   * update component properties. It would trigger rerender
   */
  update(props) {
    if (props) this.props = props;

    this.apply(this.props);
  }
};

class MenuItem extends BaseComponent {
  constructor(props) {
    super(props);
  }

  render() {
    return this.html`
      <li>${this.state.name}</li>
    `;
  }
}

class Menu extends BaseComponent {
  constructor(props) {
    super(props);
  }

  render() {
    return this.html`
      <div>A simple menu</div>
      <ul>
       ${this.state.items.map(item => this.child(MenuItem, item))}
      </ul>
    `;
  }
}

const menu = new Menu({items: [{name: 'item 1'}, {name: 'item 2'}, {name: 'item 3'}]});

render(document.querySelector('.simple'))`${menu}`;

setTimeout(() => {
  menu.update({items: [{name: 'item 4'}, {name: 'item 5'}, {name: 'item 6'}]});
}, 5000);

The BaseComponent extended from hyper Component is trying to solve 2 problems:

1. Difficult to use nested component
2. this.state is always changed so the nested children are always rerender. In this BaseComponent I tried to patch the this.state instead of change it. (Instead diff the DOM, we can diff the state!)

Do you think it's a good practice?

[WebReflection]

Difficult to use nested component

Unfortunately that's true. I'm still trying to figure out how/what would be the best way to solve the issue.

Instead diff the DOM, we can diff the state!

I think you're entering in reversed vDOM world there 😄 Theoretically setting state will trigger an update but updates in hyperHTML are cheap. Try to be sure your state diffing is actually faster than just passing a state because you might be surprised.

---

Room for improvements

I'm trying to ship .adopt method but to be honest these not-so-friendly light components are the most important issue I'd love to solve on top of everything.

I want to solve two issues:

1. make nested components easy to use / declarative and compatible with viperHTML too
2. provide a way to scope/style them (or to scope/style in general without abusing the style attribute)

Even if unrelated, both seems to be the biggest weakness of hyperHTML.Component.

If you have any suggestion I'd be happy to listen and, to be honest, I like a lot your this.child(ClassName, item, props) approach.

[WebReflection]

@liming I just had some weird thoughts on this ... what if ...

const components = new WeakMap;

const setChildren = component => {
  const children = new WeakMap;
  components.set(component, children);
  return children;
};

const setChild = (Class, children, identity) => {
  const child = new Class;
  children.set(identity, child);
  return child;
}

// hyper.Component plus a .for(...) static method
class Component {
  static for(component, identity) {
    const children = components.get(component) || setChildren(component);
    return children.get(identity) || setChild(this, children, identity);
  }
}

So that you could do the following

class MenuItem extends Component {
  update(props) {
    this.setState(props);
    return this;
  }
  render() {
    return this.html`
      <li>${this.state.name}</li>
    `;
  }
}

class Menu extends Component {
  update(props) {
    this.setState(props);
    return this;
  }
  render() {
    return this.html`
      <div>A simple menu</div>
      <ul>
        ${this.state.items.map(
          item => MenuItem.for(this, item).update(item)
        )}
      </ul>
    `;
  }
}

That would allow you to do something like:

const menu = new Menu().update({
  items: [{name: 'item 1'}, {name: 'item 2'}, {name: 'item 3'}]
});

render(document.querySelector('.simple'))`${menu}`;

setTimeout(() => {
  menu.update({items: [{name: 'item 4'}, {name: 'item 5'}, {name: 'item 6'}]});
}, 5000);

What do you think? Would a Component.for static method be a good compromise/solution ?