WebReflection / lighterhtml

The hyperHTML strength & experience without its complexity 🎉
https://medium.com/@WebReflection/lit-html-vs-hyperhtml-vs-lighterhtml-c084abfe1285
ISC License
735 stars 20 forks source link

arguments to `render` #44

Closed vilarfg closed 5 years ago

vilarfg commented 5 years ago

Loving LighterHTML, thanks!

Would you consider modifying the signature of render from render(node, callback) to render(node, callback, ...args) ? (I've noticed it would also change the signature of internal update). And then pass args to the callback.

I ask because with the current setup, I'm to keeping some local state in my components, see:

// I feed this to `wickedElements.define` (after decorating it)
class {
    init(e) {
      const t = (this.el = e.target);
      this._render = render.bind(t, t, this._render);
    }
    _render() {
      const state = this._localState;
      return html`
        <p>I am <span class="fancy">fancy!</span></p>
        <code>${JSON.stringify(state)}</code>
      `;
    }
  }

But I would prefer not to, and simply go with:

class {
    init(e) {
      const t = (this.el = e.target);
      this._render = render.bind(t, t, this._render);
    }
    _render(state) { // <-- different signature
      // no local state referenced
      return html`
        <p>I am <span class="fancy">fancy!</span></p>
        <code>${JSON.stringify(state)}</code>
      `;
    }
  }

NOTE: this._localState gets populated in a decorator somewhere, not included here just to keep things simple

Thoughts? Am I missing something ? :slightly_smiling_face:

keep up the amazing work! thanks

WebReflection commented 5 years ago

what's wrong with this approach ?

class {
    init(e) {
      this.el = e.target;
      this._render = render.bind(null, this.el, this._render.bind(this));
    }
    _render() {
      const state = this._localState;
      return html`
        <p>I am <span class="fancy">fancy!</span></p>
        <code>${JSON.stringify(state)}</code>
      `;
    }
  }

the render concept in both here and hyperHTML is suggested to be kept without arguments. What you really want in a render is the right context, so that you can call thing.render() if needed without ever needing to know what argument, if any, you need to pass.

vilarfg commented 5 years ago

What I am trying to avoid is to keep a (potentially big) computed state for this component locally.

Maybe it makes more sense with a bit of back story.

If I understood you correctly: I could just bind the render function to the computed state. It would work! But isn't binding expensive?

Maybe I am missing the point, you said:

here and hyperHTML is suggested to be kept without arguments

Why is that the suggestion? :thinking:

Thanks!

WebReflection commented 5 years ago

If I understood you correctly: I could just bind the render function to the computed state. It would work!

Yup

But isn't binding expensive?

not really https://benediktmeurer.de/2015/12/25/a-new-approach-to-function-prototype-bind/

also, the majority of react users (but also other frameworks) bind tons of methods per each instance because they don't know better, and this has never been a real-world issue.

Why is that the suggestion?

one of the reasons is here: https://github.com/WebReflection/hyperHTML/issues/104

vilarfg commented 5 years ago

Oh, thank you so much! I got it now

I think the signature render(state = this) => void is pretty neat.

I'm going to try combining it with this decorator to connect to store (it's not Redux btw :slightly_smiling_face: )


export default store => w =>
  class extends w {
    onconnected(e) {
      if (super.onconnected) super.onconnected(e);
      this._storeUnsubscribe = store.subscribe(
        state => {
          if (!this._render) return;
          this._render.call(state);
        },
        null,
        true
      );
    }

    ondisconnected(e) {
      this._storeUnsubscribe();
      if (super.ondisconnected) super.ondisconnected(e);
    }
  };

Thank you for all your help !