Skip to content

Declarative hyperHTML.Component #202

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 27, 2018
Merged

Declarative hyperHTML.Component #202

merged 2 commits into from
Feb 27, 2018

Conversation

WebReflection
Copy link
Owner

@WebReflection WebReflection commented Feb 23, 2018

It is clear at this point basic hyper.Component is too basic and not so friendly when it comes to compose the layout.

Few developers already mentioned this, and I've been thinking about a way to get closer to JSX simplicity.

Through HTML: why not

One possible solution was to somehow give components a way to define themselves.

// similar in concept to HyperHTMLElement.define('custom-element')
MyComponent.define('MyComponent');

The idea behind was to parse the HTML and somehow transform all <MyComponent /> into some abstraction but the cons overreach the pros:

  • it's not standard at all. Beside self-closing tags, hyperHTML would like to still promote standard practices as much as possible.
  • it's a poor imitation of React, with possible components name clashing between libraries, hence less portable than React components themselves
  • it's still string parsing, which is already a lot for the prime time per each template

Current Proposal

Using @liming idea, I've played with a declarative implementation that moves the main logic to the render (or any other defined method, really).

Instead of passing props like values through attributes, we can pass props directly to the render.

To prevent harakiri situations with the state, declarative components won't render automatically while non declarative components can decide to not render now on demand (unfortunately these shipped already so I don't want to break backward compatibility).

Examples

Following a couple of examples, starting with the @liming Menu and MenuItem case:

const {hyper} = hyperHTML;

class Menu extends hyper.Component {
  render(props) { return this.html`
    <div>A simple menu</div>
    <ul>
      ${props.items.map(
        (item, i) => MenuItem.for(this, i).render(item)
      )}
    </ul>`;
    // if needed, this.setState(props, false)
    // can be invoked too
  }
}

class MenuItem extends hyper.Component {
  render(props) { return this.html`
    <li>${props.name}</li>`;
    // if needed, this.setState(props, false)
    // can be invoked too
  }
}

hyper(document.body)`${
  Menu.for(document.body).render({
    items: [
      {name: 'item 1'},
      {name: 'item 2'},
      {name: 'item 3'}
    ]
  })
}`;

The following one is a basic React example reproduced through components:

class Avatar extends hyperHTML.Component {
  render(props) { return this.html`
    <img class=avatar
      src=${props.avatarUrl}
      alt=${props.name}>`;
  }
}

class UserInfo extends hyperHTML.Component {
  render(props) { return this.html`
    <div class="user-info">
      ${Avatar.for(this).render(props)}
      <div class="user-info-name">
        ${props.name}
      </div>
    </div>`;
  }
}

class Comment extends hyperHTML.Component {
  render(props) { return this.html`
    <div class="comment">
      ${UserInfo.for(this).render(props.author)}
      <div class="comment-text">
        ${props.text}
      </div>
      <div class="comment-date">
        ${formatDate(props.date)}
      </div>
    </div>`;
  }
}

const node = document.body;
hyperHTML.bind(node)`${
  Comment.for(node).render({
    date: new Date(),
    text: 'I hope you enjoy learning hyperHTML!',
    author: {
      name: 'Hello Kitty',
      avatarUrl: 'http://placekitten.com/g/64/64'
    }
  })
}`;

function formatDate(date) {
  return date.toLocaleDateString();
}

I am quite happy with the approach/result but I'd love to hear from users their thoughts before it lands.

Thank You!

@coveralls
Copy link

Coverage Status

Coverage remained the same at ?% when pulling 6fb33b4 on better-component into caa5f2e on master.

@liming
Copy link
Contributor

liming commented Feb 26, 2018

Hi @WebReflection ,

I like this implementation. Some thoughts:

  1. Are we able to simplify the interface? This change only allow us using render(). Are we able to use something like Avatar.for(this, avatarProps).
  2. Sometimes developer would like to access the props within the class. can we keep the props inside Component for convenience?
class UserInfo extends hyperHTML.Component {
 onclick(e) {
   if (this.props.editUserProfile) this.props.editUserProfile();
 }
  render(props) { return this.html`
    <div onclick=${this} class="user-info">
      ${Avatar.for(this).render(props)}
      <div class="user-info-name">
        ${props.name}
      </div>
    </div>`;
  }
}
  1. During the development. I found sometimes we need use current html element or the nested component instance. How to get these informations from Components?
class UserInfo extends hyperHTML.Component {
 onclick(e) {
   // how to access the element ref? At the moment I have to write a lot of onconnected callback to save the element inside class.
   this.ref.className = "after-click";

   // how to use the nested component instance?
   this.avartar.changeStyle();
 }
  render(props) { return this.html`
    <div onclick=${this} class="user-info">
      ${Avatar.for(this).render(props)}
      <div class="user-info-name">
        ${props.name}
      </div>
    </div>`;
  }
}
  1. Just curious: why use for?

@WebReflection
Copy link
Owner Author

WebReflection commented Feb 26, 2018

Hi @liming, thanks for coming back with questions.

To start with, after thinking a lot about this, I've realized comp.render(props) is not really a good pattern, for the simple reason that internally the engine must call .render() when necessary and it won't ever pass props around.

Accordingly, while I am sure having a setState(state, false):self that won't trigger automatically render and returns the instance itself is handy, render(props) falls easily as anti pattern.

basic render(), always!

This is a pattern that would never fail. Keep the render clean and specific to render things only.

using update(props) instead

It's a developer choice, really, not something I can enforce, but as a pattern it looks way cleaner.

class MyComp extends hyper.Component {
  constructor(props = {name: 'anonymous'}) {
    this.update(props);
  }
  update(props) {
    this.props = props;
    this.setState({active: !!props.active}, false);
    return this.render();
  }
  render() {
    return this.html`<p>I am ${this.props.name}</p>`;
  }
}

This way a component can always be rendered, directly or indirectly, with or without props.

hyper(document.body)`${
  MyComp.for(document).update({name: document.title})
}`;

Thoughts ?


Answers

Following answers to your questions.

  1. the Avatar.for(this, avatarProps) assumes per each component there is only one Avatar child. The current for(ref, id?) signature gives you the ability to have multiple children when needed so that props should be explicitly passed along a method.
  2. props are extra info not always needed because these could always be simplified through a state. When, where, why props are used are, I believe, a component case-by-case situation so as long as there are good patterns around, I don't see the need for making every component heavier by default with extra props.
  3. nested components instance are what everyone using light components until now should've have kept as reference. Many went for the this.html`<p>${new Child}</p>`; anti pattern approach while the good way for light components is to have a reference this.child = new Child in the constructor and then use this.html`<p>${this.child}</p>`; in the render. This Child.for(this) proposal improves/fixes the anti pattern but it doesn't give you a reference through the DOM (the concept is that the DOM is driven and fully unaware of hyperHTML). However, with .for(...) you can always retrieve a component back, as long as you know how you are rendering one. Child.for(this) used inside any other method will give you the exact same child used within the render. If you pass along also an ID (primitive/object) for multiple children, then you need to know how to reproduce/get that ID. Child.for(this, 0) and Child.for(this, 1) when you have two children and you render both like that would work too. Would this be enough?
  4. when there are already methods in the ECMAScript specification used to obtain a similar result I need, I usually pick that name to make it developer friendly. Symbol.for(uid) was my pick, this time, where per same uid you get back the same Symbol. Do you have better suggestions ?

@liming
Copy link
Contributor

liming commented Feb 26, 2018

This pattern is quite close to what I'm building now. It's quite useful for building complex component.

class MyComp extends hyper.Component {
  constructor(props = {name: 'anonymous'}) {
    this.update(props);
  }
  update(props) {
    this.props = props;
    this.setState({active: !!props.active}, false);
    return this.render();
  }
  render() {
    return this.html`<p>I am ${this.props.name}</p>`;
  }
}

It should be fine not to include the pattern into Component as long as hyperHTML can provide good samples to follow.

About get the reference of nested component, I think it should be a must-have feature for Component. It can be something like this:

// declare the Child component:
Child.for(this, 1)
Child.for(this, 2)

// find the Child component:
const children = Child.search(this)  // return 2 children
const child = Child.search(this, 1) // return 1 child

How cool it is! Borrow Symbol.search() concept.

@WebReflection
Copy link
Owner Author

How cool it is! Borrow Symbol.search() concept.

well, Symbol.search is a symbol primitive, not a function, so it'd be confusing, IMO.

About .search(this) it's also not ideal for the following reasons:

  • it exposes all sub components even if the used key/id was private: not good in terms of security
  • it doesn't guarantee any order. I've used two integers but IDs can be anything. It's either related as weakmap object key, if primitive, or as weakmap wakmap, if it's an object

Moreover, if you declare children you can just assign them and skip the Comp.for(...). I'd like to underline the .for is a simplification of in-template sub components assignment, not the only way to create components.

this.children = [
  new Child,
  new Child
];

Above snippet is way cleaner on the constructor, as example, and gives you the ability to render children like this:

this.html`<ul>${this.children}</ul>`;

where if you want to update them

this.html`<ul>${list.map((data, i) => this.children[i].update(data))}</ul>`;

Last, but not least, you can pass a data attribute right away to any node in the template and put there, eventually, the relationship with its Component/owner. It exposes components this way but if you really need a dual relationship that could be the way.

@liming
Copy link
Contributor

liming commented Feb 26, 2018

Yes there are so many patterns to make things work, but they are not easy to use, especially for developers playing with hyperHTML only a few weeks. And we have to write same patrerns in each Component again and again to achieve same thing. It eventually will result in either someone rebuild a new Component based on hyper Component, or choose other Component based solution.

I guess the problem is you want to keep it as small as possible, because the Component stays INSIDE hyperHTML. Maybe you should separate it out, so we can contribute as much as possible. This way you can build a full ecosystem revolving around this Component, instead of only limited features to make it just work.

@WebReflection
Copy link
Owner Author

WebReflection commented Feb 26, 2018

uhm ... unexpected answer ... I'm not making it just work, I'm telling you what are the limits and issues.

I don't have control of the scope around hyperHTML, but I can simplify composition and what I've proposed seems already simpler than what you had to write 'till now.

Specially with the JSX transformation to hyperHTML prototype, the .for(...) pattern seems to be ideal, but it's true that you need some contract with either .render() or, eventually, .update(...).

However, having a super.update(props) or a super.render() here makes no sense because props are not really essentials and state might or might not be needed (which is why everything is a lazy property in here),

I was hoping for some more hint on how to improve, and by no mean I want to block components development, indeed same way I've created the HyperHTMLElement class everyone else can create their own preferred pattern.

hyperHTML is a primitive to drive layouts and nothing else, really, it has its strengths but also its limits. As example, tell me how would I reference child nodes to an owner which doesn't exist at DOM creation.

It's not as trivial as it looks, and picking the right pattern is key, IMO.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants