WebReflection / uce

ยตhtml based Custom Elements
ISC License
198 stars 15 forks source link

[Help] How to re-render a custom-element using props? #3

Closed djalmajr closed 4 years ago

djalmajr commented 4 years ago

Hi! A silly question: how to re-render a custom-element using props? Only with attributeChanged?

My attempt was that way:

image

And how to use the "props" attribute?

Pen for this: https://codepen.io/djalmajr/pen/qBZyeQO

Thanks in advance!

WebReflection commented 4 years ago

This works https://codepen.io/WebReflection/pen/GRZXgKp?editors=0010

I had to push 1.7 to provide a hook within the constructor, as init was running too late and it wasn't suitable.

Basically, you can always define getters/setters on the class and set properties directly.

In this case I have a setter for the clicked property, and you set that via .clicked=${value}.

I've used private properties too, setup once in the constructor.

Everything works great now, apologies for not making this easier before.

djalmajr commented 4 years ago

Awesome!!!

One last thing: can you give me a proper example of how to use the "props" attribute?! I don't get it from the docs (I'm new in WebComponent ๐Ÿ˜…)

image

WebReflection commented 4 years ago

this.props is an attributes resolution / shortcut ... if you have an attribute such as clicked=${this.clicked} then this.props.clicked will give you that value. However, since attributes only accept / convert strings, your boolean check would' have failed in my-msg, as you had to check something like this.props.clicked === "true" instead of just this.props.clicked

WebReflection commented 4 years ago

P.S. using observedAttributes and this.props.clicked was another way to go/deal with this issue, but without checking against the string "true" it couldn't have worked.

My previous change/suggestion is to deal directly with .clicked as a setter/special boolean attribute instead, but this also works, from your pen:


define('my-msg', {
  observedAttributes: ['clicked'],
  init() {
    this.render();
  },
  attributeChanged() {
    this.render();
  },
  render() {
    this.html`
      <p>${(this.props.clicked === 'true') ? 'Hi!!' : ''}</p>
    `;
  }
});
djalmajr commented 4 years ago

OK. I made this simple example but my goal is to use props to pass a more complex data type (e.g. an array of posts)... I think your first suggestion is more appropriate. Thanks again.

WebReflection commented 4 years ago

Dare I say when props is defined, and it's not null, I might provide above dance out of the box, and make DX awesome.

define('my-component', {
  // define all props via {name: defaultValue} pairs
  props: [{any: []}, {value: {}}],
  render() {
    const {any, value} = this.props;
    // do the render dance
  }
});

This is actually great, I think I'm up for this.

djalmajr commented 4 years ago

It would be nice!

But why not?

define('my-component', {
  // define all props via {name: defaultValue} pairs
  props: {
    any: [], 
    value: {}
  },
  render() {
    const {any, value} = this.props;
    // do the render dance
  }
});
WebReflection commented 4 years ago

no reason, I'm just too down with jdes these days ๐Ÿ˜…

nice suggestion, thanks ๐Ÿ‘‹

WebReflection commented 4 years ago

@djalmajr it's up and running, still covered 100% ... I've updated the readme too.

The new example is here: https://codepen.io/WebReflection/pen/gOrdaRN?editors=0010

WebReflection commented 4 years ago

never mind my last comment, making props something that you can set via el.prop = value but you can get via el.props.value is no good for DX .... so there it is, props now define direct accessors to the node, triggering render when available, and making el.prop = value and el.prop working as expected.

WebReflection commented 4 years ago

so that's it, beside unpkg failing in CodePen, current version would simplify your use case in this way:

define('my-msg', {
  props: {clicked: false},
  init() { this.render(); },
  render() {
    this.html`
      <p>${this.clicked ? 'Hi!!' : ''}</p>
    `;
  }
});
define('my-app', {
  style: el => css`
    ${el} button {
      border: 1px solid #ccc;
      border-radius: 3px;
      padding: 5px 10px;
    }
  `,
  props: {clicked: false},
  init() {
    this.render();
  },
  onClick() {
    this.clicked = !this.clicked;
    this.render();
  },
  render() {
    this.html`
      <button>Click Me!</button>
      <my-msg .clicked=${this.clicked} />
    `;
  }
});
djalmajr commented 4 years ago

Great! It's been more enjoyable working that way...

I would like to make some suggestions like add setState (as hyperHTML), auto-bind methods e some others goodies (like on/off/emit) ๐Ÿ˜…

I've done that but having this built-in would be nice too: https://codepen.io/djalmajr/pen/vYGjWrG

By the way, thanks again! Looking forwarding for more works/posts from you... I've learning a lot.

WebReflection commented 4 years ago

Methods are all self bound by default here .... handleEvent is already used. https://medium.com/@WebReflection/dom-handleevent-a-cross-platform-standard-since-year-2000-5bf17287fd38

djalmajr commented 4 years ago

It would be nice if we could be less verbose:

image

This is more "natural" than this:

image

But it's ok... Thanks for the post!

djalmajr commented 4 years ago

On second thought, I think it is worth exchanging verbosity for a better memory consumption. ๐Ÿ˜†

WebReflection commented 4 years ago

what about this?

import {define} from '//unpkg.com/uce?module';
define('my-counter', {
  attachShadow: {mode: 'open'},
  count: 0,
  init() { this.render(); },
  inc() { this.count++; this.render(); },
  dec() { this.count--; this.render(); },
  onclick(e) {
    const [target] = e.composedPath();
    const {handler} = target.dataset;
    this[handler](e);
  },
  render() {
    this.html`
      <button data-handler="dec" onlick=${this}>-</button>
      <span>${this.count}</span>
      <button data-handler="inc" onlick=${this}>+</button>
    `;
  }
});

edit although, closed shadowDom does not exposes .path[0] as button, or currentTarget ... bummer!

WebReflection commented 4 years ago

Anyway, if handleEvent is not ideal, for this specific case, the following will work well too:

import {define} from '//unpkg.com/uce?module';
define('my-counter', {
  count: 0,
  init() { this.render(); },
  inc() { this.count++; this.render(); },
  dec() { this.count--; this.render(); },
  render() {
    this.html`
      <button onclick=${() => this.dec()}>-</button>
      <span>${this.count}</span>
      <button onclick=${() => this.inc()}>+</button>
    `;
  }
});

This is both performant and memory consumption OK ... after all, assign a new listener each click isn't exactly what's killing the Web these days ๐Ÿ˜…

WebReflection commented 4 years ago

another alternative I've just published is bound-once

I could expose this directly through uce but the issue/pattern is so common that I've thought it was worth a module a part.

Above example can now be written as such, and no issues whatsoever will happen, as the method is always the same, no matter how many renders are called.

import bound from '//unpkg.com/bound-once?module';
import {define} from '//unpkg.com/uce?module';
define('my-counter', {
  count: 0,
  init() { this.render(); },
  inc() { this.count++; this.render(); },
  dec() { this.count--; this.render(); },
  render() {
    this.html`
      <button onclick=${bound(this, 'dec')}>-</button>
      <span>${this.count}</span>
      <button onclick=${bound(this, 'inc')}>+</button>
    `;
  }
});
WebReflection commented 4 years ago

fuck it ... I've just brought bound-once in, the code is now this one:

define('my-counter', {
  count: 0,
  init() { this.render(); },
  inc() { this.count++; this.render(); },
  dec() { this.count--; this.render(); },
  render() {
    this.html`
      <button onclick=${this.bound('dec')}>-</button>
      <span>${this.count}</span>
      <button onclick=${this.bound('inc')}>+</button>
    `;
  }
});

I've also removed the constructor as it was ugly ... the init is anyway now granted to run before anything else could happen, including accessing or setting props.

The 1.9.1 is the best uce version yet, so thanks a lot for suggesting few things, happy to listen to more.

P.S. the on(...) off(...) is not needed in uce, as any method called onThing is automatically added once on each element bootstrap ... however, if you really want these, you can bring these in via:

define(name, {
  on(...args) { this.addEventListener(...args); },
  off(...args) { this.removeEventListener(...args); }
})
WebReflection commented 4 years ago

last change for today ... if there is a render but not an init defined, this will be called automatically.

to summarize all recent changes, this is the last revisited example, this time granting the component the ability to receive a prop and start with a different count:

import {define, render, html} from '//unpkg.com/uce?module';
define('my-counter', {
  attachShadow: {mode: 'closed'},
  props: {count: 0},
  inc() { this.count++; this.render(); },
  dec() { this.count--; this.render(); },
  render() {
    this.html`
      <button onclick=${this.bound('dec')}>-</button>
      <span>${this.count}</span>
      <button onclick=${this.bound('inc')}>+</button>
    `;
  }
});

render(document.body, html`<my-counter .count=${1} />`);

Code pen updated

djalmajr commented 4 years ago

last change for today ... if there is a render but not an init defined, this will be called automatically.

Awesome!!!

... I've just brought bound-once in, the code is now this one

Perhaps you can take a "backbone.js approach" to bind methods and make the code cleaner:

import {define, render, html} from '//unpkg.com/uce?module';

define('my-counter', {
  attachShadow: {mode: 'closed'},
  props: {count: 0},
  bound: ['dec', 'inc'], // or bindAll: [...]
  inc() { this.count++; this.render(); },
  dec() { this.count--; this.render(); },
  render() {
    this.html`
      <button onclick=${this.dec}>-</button>
      <span>${this.count}</span>
      <button onclick=${this.inc}>+</button>
    `;
  }
});

render(document.body, html`<my-counter .count=${1} />`);
WebReflection commented 4 years ago

I think I'm OK with the lazy bound approach, also consider I've never needed it to date, as I always add listeners as this ... yet there are cases this could be handy, but writing these in the proto is just as boring as writing these in the init, imho.

djalmajr commented 4 years ago

I would like to make a case to setState again and avoid calling this.render() all the time:

import {define, render, html} from '//unpkg.com/uce?module';

define('my-counter', {
  init() {
    this.state = { count: this.count };
    this.render();
  },
  async setState(data, callback) {
    const validTypes = ['function', 'object'];

    if (!validTypes.includes(typeof data) || Array.isArray(data)) {
      throw new Error("Invalid data type!");
    }

    this.state = typeof data === "function"
      ? data(this.state)
      : Object.assign(this.state, data);

    // An attempt to make the "caller" method 
    // terminate (specially if we face a async code)
    // before call the "render" again.
    await new Promise(setTimeout);

    this.render();
    callback && callback(this.state);
  },
  syncCount(s) {
    this.count = s.count;
  },
  inc() { this.setState(s => ({ count: s.count + 1 }), this.syncCount) },
  dec() { this.setState(s => ({ count: s.count - 1 }), this.syncCount) },
  render() {
    const { count } = this.state;
    this.html`
      <button onclick=${this.bound('dec')}>-</button>
      <span>${count}</span>
      <button onclick=${this.bound('inc')}>+</button>
    `;
  }
});

render(document.body, html`<my-counter .count=${10} />`);
djalmajr commented 4 years ago

P.S. the on(...) off(...) is not needed in uce, as any method called onThing is automatically added once on each element bootstrap ... however, if you really want these, you can bring these in via:

define(name, {
  on(...args) { this.addEventListener(...args); },
  off(...args) { this.removeEventListener(...args); }
})

Indeed, the on(...) off(...) is unnecessary, but do you think that make sense an emit alias to dispatchEvent?

define('my-counter', {
  // ...
  emit(name, data) {
    this.dispatchEvent(
      new CustomEvent(name, {
        bubbles: true,
        composed: true,
        detail: data,
      })
    );
  },
  // ...
  render() {
    this.html`
      <button onclick=${() => this.emit('dec')}>-</button>
      <span>${count}</span>
      <button onclick=${() => this.emit('inc')}>+</button>
    `;
  }
}

// ...

render(document.body, html`<my-counter .count=${10} oninc=${inc} ondec=${dec} />`);
WebReflection commented 4 years ago

the goal of this project is to provide everything you need, in terms of primitives, to ship Custom Elements + builtin extends out there, and the name is micro Custom Elements, so I'm not too keen to add stuff already available in HyperHTMLElement, as bloating this library would defeat its name and goal.

However, nothing prevents you from creating a uce-state module which either re-export a different define adding state the way you like, or simply exports an object that can be mixed in with definitions (see FAQ)

States are a complicated matter and also async and await and stuff like this bloats a lot once transpiled, but this library targets by default IE11 too so ... why not trying to write your state manager?

WebReflection commented 4 years ago

P.S. when you set props, render is invoked automatically ... the render is also super cheap, so calling it once or 10 times won't likely affect performance

djalmajr commented 4 years ago

the goal of this project is to provide everything you need, in terms of primitives, to ship Custom Elements + builtin extends out there, and the name is micro Custom Elements, so I'm not too keep to add stuff already available in HyperHTMLElement, as bloating this library would defeat its name and goal.

Indeed

However, nothing prevents you from creating a uce-state module which either re-export a different define adding state the way you like, or simply exports an object that can be mixed in with definitions (see FAQ)

States are a complicated matter and also async and await and stuff like this bloats a lot once transpiled, but this library targets by default IE11 too so ... why not trying to write your state manager?

Good idea!

Thanks for the adjustments ... ๐Ÿ‘‹

WebReflection commented 4 years ago

btw ...

bound: ['dec', 'inc']

this might be doable without much bloat, will think about it.

WebReflection commented 4 years ago

On a second thought ... having an explicit intent such as this.bound('name') leads to least surprises, as example, when you this.method but you forgot to put method in the list ... still not sure this extra thing is needed/worth it

djalmajr commented 4 years ago

The same problem can occurr if an developer forgot to write this.bound('method') ๐Ÿ˜… since is more natural to write onclick=${this.method} than onclick=${this.bound('method')} (given the mental model that people have from react, vue, etc)... And expliciting than in bound: ['method1', 'method2'] will make the bind process less "pain" (I think).

But the handleEvent method is always a good choice too... I always make a todo-app to feel how it would be in a large-scale project:

image

The only "problem" with handleEvent is to differentiate two click event types without a data-attr, like:

define('my-counter', {
  // ...
  handleEvent(evt) {
    if (evt.type === 'click') {
      // ... which click? Dec or Inc
    }
  },
  // ...
  render() {
    this.html`
      <button onclick=${this}>-</button>
      <span>${count}</span>
      <button onclick=${this}>+</button>
    `;
  }
}

But, so far, uce is becoming the best choice to write WC without a lot of tools in the process and with small footprint as well (IMO). ๐Ÿ‘

WebReflection commented 4 years ago

so ... should I remove bound-once already and follow your suggestion instead?

djalmajr commented 4 years ago

Yes... I think it's a good way to go.

WebReflection commented 4 years ago

ok, done

WebReflection commented 4 years ago

it's up and running https://codepen.io/WebReflection/pen/qBZMRxy?editors=0010

WebReflection commented 4 years ago

btw, to clarify the pattern ... having onMethod() at the definition level is for events that target the component, where using this is fine, while having bound methods works well for inner elements

djalmajr commented 4 years ago

it's up and running https://codepen.io/WebReflection/pen/qBZMRxy?editors=0010

Nice and clean! :)

btw, to clarify the pattern ... having onMethod() at the definition level is for events that target the component, where using this is fine, while having bound methods works well for inner elements

Understood.

WebReflection commented 4 years ago

the last optimization I'm considering now is to optionally debounce the render ... or simplify a way to do that ... I have a module called fnbouncer that could help

import bouncer from 'fnbouncer';
define('my-element', {
  init() {
    this.render = bouncer.bind(null, this.render, this);
    this.times = 0;
    // test
    let i = 100;
    while (i--) this.render();
  }
  render() {
    html`rendered ${++this.times} times`;
  }
});

As the bouncer "ticks away" repeated calls, expensive renders will be visualized ASAP but multiple props access or state changes at once won't render everything each time ... which is still super cheap, but it's cheaper if it doesn't happen.

Although I think I'm overly-engineering already this, as I've never had a performance issue with uhtml (underneath)

WebReflection commented 4 years ago

now that I think about it, bouncer could use requestAnimationFrame instead ... as that's more coupled with DOM world ... will think about it, don't want to rush any decision as I've done already few times in here ๐Ÿ˜…

djalmajr commented 4 years ago

Maybe, what you can do next is a way to help developers extend the lib in a form that they can call super for "reserved" methods (init, connect, etc)...

For example, if I want to build a uce-events mixin, I could do this:

export default {
  init() {
    super.init && super.init(); // if other mixins has init too.

    if (this.events) {
      Object.keys(this.events).forEach(name => {
        this[this.events[name]] = this[this.events[name]].bind(this);
        this.addEventListener(name, this[this.events[name]]);
      });
    }
  },
  disconnected() {
    super.disconnected && super.disconnected(); // if other mixins has disconnected too.

    if (this.events) {
      Object.keys(this.events).forEach(name => {
        this.removeEventListener(name, this[this.events[name]]);
      });
    }
  },
  emit(name, data) {
    this.dispatchEvent(
      new CustomEvent(name, {
        bubbles: true,
        composed: true,
        detail: data,
      })
    );
  },
};

// ...

const TodoApp = {
  // ...
  events: {
    "todos:add": "handleAdd",
    "todos:edit": "handleEdit",
    "todos:remove": "handleRemove",
    "todos:toggle": "handleToggle",
  },
  init() {
    super.init();
    // other stuff
  },
  // ...
};

define("todo-app", mixins(TodoApp, eventsMixin))

I don't know if your assign-properties can do this, but I'll think how to resolve that...

WebReflection commented 4 years ago
import eventsMixin from 'events-mixin';
import otherMixin from 'other-mixin';

import $ from 'assign-properties';
const mixin = (...components) => $({}, ...components);

define('my-comp', mixin(eventsMixin, otherMixin, {
  init() {
    eventsMixin.init.call(this);
    otherMixin.init.call(this);
    // do your thing
  }
}));

super can't be used in literals, and mixins should know what these are extending ... as they need those dependencies ... so, I wouldn't bother with that, you need to trust whoever use your mixin remembers to invoke its init.

however, with assign-properties you have all it's needed to play around with this, and merge all conflicting methods automatically via that list of mixins ...components

djalmajr commented 4 years ago

Nice! Works great!

WebReflection commented 4 years ago

Something like this (not tried) might work well too

const assign = require('assign-properties');

const mixin = (...C) => {
  const init = group(C, 'init');
  const connected = group(C, 'connected');
  const disconnected = group(C, 'disconnected');
  const attributeChanged = group(C, 'attributeChanged');
  const observed = new Set(C.reduce((p, c) => p.concat(c.observedAttributes || []), []));
  const bound = new Set(C.reduce((p, c) => p.concat(c.bound || []), []));
  const props = C.filter(by, 'props').reduce((p, c) => assign(p, c.props), {});
  return assign({}, ...C, {
    init() { init.forEach(invoke, this); },
    connected() { connected.forEach(invoke, this); },
    disconnected() { disconnected.forEach(invoke, this); },
    attributeChanged() { attributeChanged.forEach(invoke, this); },
    observedAttributes: [...observed],
    bound: [...bound],
    props
  });
};

function group(c, key) { return c.filter(by, key).map(by, key); }
function by(c) { return c[this]; }
function invoke(fn) { fn.call(this); }
djalmajr commented 4 years ago

"Flawless victory!" ๐ŸŽ‰

Now it's time to play with some cool project!

Thanks again!

WebReflection commented 4 years ago
this.constructor.observedAttributes

is the answer, but I don't see anymore the question.

P.S. this is not uce making it up, it's actually how Custom Elements work

WebReflection commented 4 years ago

some uce reactive state change pattern here:

https://codepen.io/WebReflection/pen/LYNJwoV?editors=0010

๐Ÿ‘‹